fix(metrics): record watchable_depth as the coalesced update count - #9860
fix(metrics): record watchable_depth as the coalesced update count#9860somaz94 wants to merge 1 commit into
Conversation
Signed-off-by: somaz <genius5711@gmail.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f90d808a0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // batch was being handled, so its length is the backlog that built up during | ||
| // that pass. len(subscription) cannot report that: Subscribe returns an | ||
| // unbuffered channel, so its length is always 0. | ||
| watchableDepth.With(meta.LabelValues()...).Record(float64(len(snapshot.Updates))) |
There was a problem hiding this comment.
When a nonempty snapshot finishes processing and the publisher becomes idle—or the subscription closes—this remains at len(snapshot.Updates) indefinitely because Gauge.Record stores the last value (internal/metrics/otel_metric_gauge.go:30-38) and no later zero is recorded. Consequently, the shipped Grafana depth panel (charts/gateway-addons-helm/dashboards/envoy-gateway-global.json:277) can show a permanent backlog, and alerts can remain active, even though no snapshot is currently being handled. Reset the series after the batch completes, or use a metric whose documented semantics explicitly represent the last observed batch rather than current depth.
Useful? React with 👍 / 👎.
1 participant
What this PR does / why we need it:
watchable_depthrecordslen(subscription), wheresubscriptionis the channelreturned by
watchable.Map.Subscribe(). That channel is unbuffered(
downstream := make(chan Snapshot[K, V])in the watchable library), so the gaugecan only ever read 0.
It isn't dead weight: the shipped Grafana dashboard graphs it, so a flat zero reads
as "no backlog" while the control plane can be seconds behind.
This records
len(snapshot.Updates)instead. The watchable library documentsSnapshot.Updatesas "the list of mutations that have happened since the previoussnapshot", accumulated while the consumer is busy, so its length is the backlog that
built up during the previous handler pass. It's always available, unlike
watchable_debounce_pending, which only reports when debouncing is on.The pre-coalesce count is deliberate.
coalesceUpdatesdedupes by key, so itsresult measures the work left to do while the raw count measures how much arrived.
Happy to switch to the coalesced count if you'd rather the gauge track that.
The metric name and labels are unchanged, so the dashboard's
label_values(watchable_depth, ...)template variables keep working and the depthpanel becomes meaningful rather than needing to be repointed. Anything that treated
the constant 0 as meaningful will now see real values, hence the breaking-change
note.
Validation, run locally on darwin/arm64 with the go.mod-pinned Go 1.26.6:
make lintpasses (golangci-lint, yamllint, codespell, shellcheck, release-notesfilename check).
make gen-checkpasses and leaves the tree clean.go test ./internal/message/... ./internal/metrics/...passes.actual: 0when the oldlen(subscription)line is putback, which is the bug it's pinning.
Which issue(s) this PR fixes:
Fixes #9776
Item 2 of that issue (subscribe-duration bucket spacing) already landed in #9812;
this is item 1.
PR Checklist
git commit -s)./api.make lintandmake gen-checkpass locally, and the unit tests for the touched packages pass.TestHandleSubscriptionRecordsCoalescedUpdateCountininternal/message/watchutil_internal_test.go.watchable_depthrow insite/content/en/latest/tasks/observability/gateway-exported-metrics.md.release-notes/current/breaking_changes/9776-watchable-depth-coalesced-updates.md.make gen-checkis clean.release-notes/current/breaking_changes/.On the last two boxes: I can't request either reviewer myself — as an external
contributor I don't have push or triage on this repo, so
--add-reviewerreturns404, and Copilot doesn't appear to be set up here. Codex looks like it skips
drafts, so I'm marking this ready for it to pick up, and I'll tick that box once
I've addressed its comments.