Skip to content

metrics: add canary label to metrics and clean up stale series on canary/namespace removal - #1961

Open
dkulchinsky wants to merge 3 commits into
fluxcd:mainfrom
dkulchinsky:dannyk/fix_1960
Open

metrics: add canary label to metrics and clean up stale series on canary/namespace removal#1961
dkulchinsky wants to merge 3 commits into
fluxcd:mainfrom
dkulchinsky:dannyk/fix_1960

Conversation

@dkulchinsky

@dkulchinsky dkulchinsky commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

fixes #1960

Three Four related fixes to Flagger's Prometheus metrics:

  1. flagger_canary_* metrics gain a canary label. The canary label will be the Canary resource name (cd.Name) and will be consistent across all metrics, which allows for a stable key to be used across all the metrics. Final result:

    Metric Labels (after)
    flagger_canary_status canary, name, namespace
    flagger_canary_duration_seconds canary, name, namespace
    flagger_canary_metric_analysis canary, name, namespace, metric
    flagger_canary_weight canary, workload, namespace
    flagger_canary_successes_total canary, name, namespace, deployment_strategy, analysis_status
    flagger_canary_failures_total canary, name, namespace, deployment_strategy, analysis_status
  2. Per-canary series are cleaned up on Canary deletion. The controller's DeleteFunc only removed the canary from an in-memory job map; the Prometheus series lived for the process lifetime. A new Recorder.DeleteFor(name, namespace) calls DeletePartialMatch({name, namespace}) on every per-canary vec (duration, status, weight, analysis, successes, failures) and is invoked from DeleteFunc.

  3. flagger_canary_total is cleaned up when a namespace no longer has any canaries. scheduleCanaries previously only wrote totals for namespaces that currently held canaries, so a namespace that dropped to zero kept its last value forever. The controller now tracks reported namespaces and calls Recorder.DeleteTotalFor(namespace) for any that fall out on the next tick.

  4. Emit flagger_canary_weight in steady state so it survives restarts and appears right after initialization — previously it was only set while a canary was progressing, leaving initialized/succeeded canaries (and everything after a controller restart) without a weight series until the next rollout. The !shouldAdvance path now also reads the current routes and calls SetWeight, with a regression test asserting the gauge is present for a freshly-initialized canary.

Behaviour changes to be aware of

  • canary label is added to all flagger_canary_* metrics
  • Deleting a Canary now removes its counter/gauge series immediately, including flagger_canary_successes_total and flagger_canary_failures_total.
  • flagger_canary_weight metric is populated as soon as the canary is initialized and when flagger restarts.

Tests

  • Updated scheduler_metrics_test.go weight assertions for the new label.
  • Added TestRecorder_DeleteFor and TestRecorder_DeleteTotalFor in pkg/metrics/recorder_test.go.
  • go test ./pkg/metrics/... ./pkg/controller/... passes.

… canary/namespace removal

Signed-off-by: Danny Kulchinsky <dkulchinsky@fastly.com>
@dkulchinsky

dkulchinsky commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

After further review, I realized name in all metrics is actually spec.targetRef.name and not the Canary resource name, this threw me off at first because we tend to name the Canaries the same as the target.

as such, I've changed my proposal in this PR to the following:

  1. for consistency remove name from flagger_canary_weight (i.e. revert to original state)
  2. add a new label - canary which would be the Canary resource name to all the metrics

We end up with the following set of labels per metric:

Metric Labels (after)
flagger_canary_status canary, name, namespace
flagger_canary_duration_seconds canary, name, namespace
flagger_canary_metric_analysis canary, name, namespace, metric
flagger_canary_weight canary, workload, namespace
flagger_canary_successes_total canary, name, namespace, deployment_strategy, analysis_status
flagger_canary_failures_total canary, name, namespace, deployment_strategy, analysis_status

Adding a label is additive, not breaking: every existing time series retains all of its current labels and values, so existing queries, dashboards, recording rules, and alerts that select on name/namespace/workload continue to match exactly as before. Prometheus treats the new label as just another dimension; nothing that previously worked stops working.

What it buys us is a stable, consistent identity key across every canary metric. Today the name label actually carries the target workload name (spec.targetRef.name), which only coincidentally equals the Canary resource name in the common case. That makes cross-metric correlation fragile — there's no reliable key to line up flagger_canary_status, _weight, _metric_analysis, and the success/failure counters for the same Canary object, particularly when the resource name and target name differ.

The canary label fixes that by exposing the Canary resource name uniformly on all six metrics. Concretely, it enables clean joins like:

flagger_canary_status
  * on (canary, namespace) group_left()
flagger_canary_weight

without having to reason about whether name means the workload or the resource in each metric.

Cost is negligible: cardinality is unchanged in practice, we're simply adding an additional static label to existing metrics so no new/unbound dimensions.

Net: low risk, no migration required for existing consumers, and it gives us a durable correlation key going forward. If maintainers prefer different naming (e.g. standardizing on workload for the target and reserving name for the resource), that's a separate, orthogonal discussion — this change doesn't foreclose it.

the additional changes to cleanup metric registry when canaries are deleted remain largely unchanged.

@dkulchinsky dkulchinsky changed the title metrics: add name label to canary_weight and clean up stale series on canary/namespace removal metrics: add canary label to metrics and clean up stale series on canary/namespace removal Aug 25, 2026
…ary resource name

Signed-off-by: Danny Kulchinsky <dkulchinsky@fastly.com>
Signed-off-by: Danny Kulchinsky <dkulchinsky@fastly.com>
@dkulchinsky

dkulchinsky commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

folks, apologies for piling on here, but just another small commit addressing the metrics area:

260a2ad emits flagger_canary_weight in steady state so it survives restarts and appears right after initialization — previously it was only set while a canary was progressing, leaving initialized/succeeded canaries (and everything after a controller restart) without a weight series until the next rollout. The !shouldAdvance path now also reads the current routes and calls SetWeight, with a regression test asserting the gauge is present for a freshly-initialized canary.

I'm happy to split up these changes into separate PRs if you prefer, I've kept them in separate commits for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flagger_canary_weight metric is missing the name label

1 participant