ref(seer): add viewer_context_scope to post_process_group - #121545
Conversation
Set ViewerContext after project/org resolution in post_process_group, covering all downstream Seer calls in the post-process pipeline (lightweight_rca_cluster, etc.) and any tasks dispatched from it via ViewerContextHook propagation. Part of AIML-3206 (SeerViewerContext retirement Step 2).
| "is_regression": bool(is_regression), | ||
| "is_new_group_environment": is_new_group_environment, | ||
| } | ||
| with viewer_context_scope( |
There was a problem hiding this comment.
can we wrap only the post process jobs that need viewer context?
There was a problem hiding this comment.
The thought process was that setting it at the entrypoint means any Seer call added to the post-process pipeline in the future automatically has ViewerContext. The current issue is that when the addition is needed at the call site it gets very fragmented and poorly maintained because people forget to add it, whereas setting a contextvar for the other pipeline steps is pretty much free. (Also this is mainly for seer though it has uses for security etc within sentry as well)
There was a problem hiding this comment.
there was some discussion here about removing some of these Seer calls from the hot path here: #121462 (comment)
change looks ok as is. i'm a little concerned since this means that any changes to how ViewerContext works or any performance / latency problem there has a chance of breaking post process, so i'd like to see if we can avoid it.
| "is_regression": bool(is_regression), | ||
| "is_new_group_environment": is_new_group_environment, | ||
| } | ||
| with viewer_context_scope( |
There was a problem hiding this comment.
there was some discussion here about removing some of these Seer calls from the hot path here: #121462 (comment)
change looks ok as is. i'm a little concerned since this means that any changes to how ViewerContext works or any performance / latency problem there has a chance of breaking post process, so i'd like to see if we can avoid it.
Summary
viewer_context_scopeinpost_process_groupafter project/org resolution, covering all downstream Seer calls in the post-process pipeline (lightweight_rca_cluster, etc.)post_process_group(liketrigger_lightweight_rca_cluster_task) will also get ViewerContext propagated via taskbroker'sViewerContextHookseer.viewer_context_not_setwarnings that persisted after ref(seer): add viewer_context_scope to ingest and explorer_index paths #121467 — all traced tolightweight_rca_clusterrunning intask-ingest-push-workerPart of AIML-3206 (SeerViewerContext retirement Step 2).
Test plan
seer.viewer_context_not_setwarnings in GCP logs should dropseer.viewer_context_resolutionmetriccontextvar_missingshould drop