Repository navigation
atecontroller: add a zero-count bucket to empty exponential histograms - #1933
Conversation
An idle workqueue bridges as an exponential histogram with no positive buckets. Google Cloud Monitoring rejects that with num_finite_buckets < 1 and drops it every push tick. Filter those data points out before the OTLP push. No data is lost, an idle queue has nothing to report anyway. Fixes agent-substrate#1472. Signed-off-by: Eric Curtin <[email protected]>
| // | ||
| // controller-runtime's workqueue metrics use native (exponential) Prometheus | ||
| // histograms, and the bridge produces one the moment a queue is created, before | ||
| // any item is ever processed. Google Cloud Monitoring rejects a data point in |
There was a problem hiding this comment.
This only applies to Google Telemetry API (telemetry.googleapis.com).
Suggested text: "The Telemetry API (the Cloud Monitoring OTLP endpoint) rejects a data point in that state ...
There was a problem hiding this comment.
Reworded to name the Telemetry API.
| if len(dp.PositiveBucket.Counts) == 0 { | ||
| continue |
There was a problem hiding this comment.
A dropped point changes the data for every backend, but only one backend (Google Telemetry API) has the problem.
I tested an alternative on a GKE cluster with Managed OTel enabled. Instead of a drop, give the point one positive bucket with a count of 0:
if len(dp.PositiveBucket.Counts) == 0 {
dp.PositiveBucket = metricdata.ExponentialBucket{Counts: []uint64{0}}
}
This would also work, meanwhile doesn't change the behaviour of metrics in other backends.
There was a problem hiding this comment.
Nice, switched to the zero-count bucket. Thanks for testing it on GKE.
Only the Telemetry API rejects them. Keep the point for other backends.
Eric Curtin (ericcurtin)
changed the title
|
Jeff Luo (@JeffLuoo) Now pads with a zero-count bucket as you suggested. PTAL when you get a chance. Thank you! |
Jeff Luo (JeffLuoo)
left a comment
Jeff Luo (JeffLuoo)
left a comment
There was a problem hiding this comment.
Please also change the PR title to "atecontroller: add a zero-count bucket to empty exponential histograms"
| func TestPadEmptyExponentialHistogramsIdleWorkqueue(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| q := workqueue.NewNamed("atecontroller-metrics-pad-probe") |
There was a problem hiding this comment.
This function is marked as deprecated. Can you use NewWithConfig?
There was a problem hiding this comment.
Done, now uses NewWithConfig. Title updated too.
There was a problem hiding this comment.
Done, thanks. NewWithConfig is deprecated too, now NewTypedWithConfig (fixes lint).
There was a problem hiding this comment.
Ah, thanks for fixing it!
Eric Curtin (ericcurtin)
changed the title
|
Jeff Luo (@JeffLuoo) PTAL when you get a chance. Thank you! |
Fixes #1472.
The Telemetry API rejects an exponential histogram point with no positive buckets. Give such points one zero-count bucket instead.