Repository navigation
Add active runs metric (OTel gauge, Grafana panel, Sentry) - #8283
twpsfirmament wants to merge 2 commits into
Conversation
Signed-off-by: twpsfirmament <twpsfirmament@gmail.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
cloud-verifier · ⚠️ 1 finding, not blocking
🟠 runs.active count scans all RUNNING child actions on every export · runs/repository/impl/action.go:108 · medium · medium likelihood · see inline comment
Reviewed 8df03fc0cd · turn 1 · re-run with /cloud-verifier review
|
|
||
| var count int64 | ||
| if err := sqlx.GetContext(ctx, db, &count, | ||
| "SELECT COUNT(*) FROM actions WHERE parent_action_name IS NULL AND phase = $1", |
There was a problem hiding this comment.
🟠 runs.active count scans all RUNNING child actions on every export
This count scans all RUNNING actions, including child actions, and every replica repeats it on each export. Consider a partial index on root actions by phase, or caching the count.
Signed-off-by: twpsfirmament <twpsfirmament@gmail.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
cloud-verifier · ⚠️ 1 finding, not blocking
🟠 runs.active count scans all RUNNING child actions on every poll · runs/repository/impl/action.go:88 · medium · medium likelihood
The count query can only use the generic phase index, so it visits every RUNNING action, including child actions. It now runs from two pollers on every replica (metric export and a periodic reporter). Add a partial index on root actions by phase, or cache and share the count.
Reviewed 84250d1b6d · turn 2 · re-run with /cloud-verifier review
Tracking issue
Closes #8191
Why are the changes needed?
Nothing reports how many runs are currently executing.
What changes were proposed in this pull request?
runs.activeobservable gauge in the runs repository: root actions inRUNNING, counted from the DB on each collection (5s timeout). Registered withRegisterCallbackliketaskaction.active; skipped for nil/noop providers.flyte-execution.json, usingmax()because every replica reports the same DB-wide count.
sentryutils.Gauge("flyte.active_runs"). The worker is registered inside theexisting
sentryutils.Initblock, so it does not run when telemetry is off(
DO_NOT_TRACK).Note: both are sampled (per OTel export / every 60s), so a run shorter than the
interval can be missed.
How was this patch tested?
TestActiveRunsGauge: counts RUNNING runs; ignores QUEUED, SUCCEEDED and RUNNINGchild actions.
TestRegisterActiveRunsGaugeNoop: nil/noop provider is a no-op.go test ./runs/repository/impl/passes.TestActiveRunsReporter_*: reports the count asflyte.active_runs; skipsreporting when the query fails. Not exercised against a real Sentry project.
go test ./runs/...passes.Labels
Setup process
devbox with
FLYTE_DEV=True+make devbox-monitoring, local manager with anotelotlpgrpc exporter (port-forward the
otel-collectorservice on 4317). See screenshot.Screenshots
Check all the applicable boxes