Add more information logs for UI logs table #2220
Conversation
Amogh-Bharadwaj
requested review from
heavycrystal,
serprex,
iskakaushik and
iamKunalGupta
| @@ -247,6 +248,7 @@ func (a *FlowableActivity) CreateNormalizedTable( | |||
| }) | |||
| defer shutdown() | |||
|
|
|||
| a.Alerter.LogFlowInfo(ctx, config.FlowName, "Setting up destination tables") | |||
There was a problem hiding this comment.
why do we have this twice?
There was a problem hiding this comment.
think it's meant to say conn.StartupSetupNormalizedTables has created tables, beginning ingestion. Wording needs to be updated
| @@ -265,15 +267,19 @@ func (a *FlowableActivity) CreateNormalizedTable( | |||
| numTablesSetup.Add(1) | |||
| if !existing { | |||
| logger.Info("created table " + tableIdentifier) | |||
| a.Alerter.LogFlowInfo(ctx, config.FlowName, "created table "+tableIdentifier+" in destination") | |||
There was a problem hiding this comment.
Might be too noisy for a large number of tables
| @@ -509,6 +515,8 @@ func (a *FlowableActivity) GetQRepPartitions(ctx context.Context, | |||
| } | |||
| } | |||
|
|
|||
| a.Alerter.LogFlowInfo(ctx, config.FlowJobName, fmt.Sprintf("obtained partitions for table %s", config.WatermarkTable)) | |||
There was a problem hiding this comment.
Maybe just log instead of sending an alert? (too noisy)
There was a problem hiding this comment.
These are not alerts. This is for our logs table in UI
I think LogFlowEvent is for alerts ?
There was a problem hiding this comment.
My bad, but still do we want to do it for all tables?
There was a problem hiding this comment.
yeah because we have scarcity of user facing info during initial load - common feedback from customers cc @iskakaushik
| @@ -58,7 +58,7 @@ func (a *SnapshotActivity) SetupReplication( | |||
| ) (*protos.SetupReplicationOutput, error) { | |||
| ctx = context.WithValue(ctx, shared.FlowNameKey, config.FlowJobName) | |||
| logger := activity.GetLogger(ctx) | |||
|
|
|||
| a.Alerter.LogFlowInfo(ctx, config.FlowJobName, "Setting up replication slot and publication") | |||
| a.Alerter.LogFlowEvent(ctx, config.FlowJobName, "Started Snapshot Flow Job") | |||
There was a problem hiding this comment.
| a.Alerter.LogFlowEvent(ctx, config.FlowJobName, "Started Snapshot Flow Job") |
There was a problem hiding this comment.
This is for alerting, we need this
Amogh-Bharadwaj
changed the title
Currently the only information logs we store in catalog is "pushed x records"
It is important for us to show users more than just that
Note that this PR pertains to the logs table in UI. It has nothing to do with alerting
This PR logs info about some key moments during replication:
Setting up of target tables
Setup of replication slot and publication
Replication of all partitions in QRep/Initial load
Schema changes
Resync completion
Source cleanup of drop flow
Destination cleanup of drop flow
Removal of tables from publication
Addition of tables from publication
Functional test