diff --git a/pkg/telemetry/metrics.go b/pkg/telemetry/metrics.go index 13c7e8a..1f30386 100644 --- a/pkg/telemetry/metrics.go +++ b/pkg/telemetry/metrics.go @@ -200,6 +200,23 @@ func NewWithMeter(meter metric.Meter, log logging.Logger, deploymentName string) return newMetrics(meter, log, deploymentName) } +// secondsBoundaries are the bucket upper bounds of every *_seconds histogram this package records. +// Without them the SDK applies its default bounds, [0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, +// 2500, 5000, 7500, 10000], which were chosen for MILLISECONDS: on a seconds metric every operation +// under 5 s falls in one bucket, so a quantile reads only as a coarse upper bound (observability, +// krateo-057, 2026-11-15: reconcile means of 2.8-7.9 s all reported p95 in the 75-250 s buckets). +// These are the bounds the composition-dynamic-controller's own histograms already use (1 ms to +// 10000 s), so the two families compare bucket for bucket. They are set on the instrument, so they +// apply under any provider, including a caller's own (NewWithMeter); a provider View still overrides +// them. +var secondsBoundaries = []float64{ + 0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, + 2.5, 5, 10, 25, 50, 100, 250, 500, 1000, + 2500, 5000, 10000, +} + +var secondsBuckets = metric.WithExplicitBucketBoundaries(secondsBoundaries...) + func newMetrics(meter metric.Meter, log logging.Logger, deploymentName string) (*Metrics, error) { var err error m := &Metrics{meters: &meters{ @@ -215,25 +232,25 @@ func newMetrics(meter metric.Meter, log logging.Logger, deploymentName string) ( if m.startupFailure, err = meter.Int64Counter("unstructured_runtime.startup.failure"); err != nil { return nil, err } - if m.getDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.get.duration_seconds"); err != nil { + if m.getDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.get.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.getFailure, err = meter.Int64Counter("unstructured_runtime.reconcile.get.failure"); err != nil { return nil, err } - if m.reconcileDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.duration_seconds"); err != nil { + if m.reconcileDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.queueDepth, err = meter.Int64UpDownCounter("unstructured_runtime.reconcile.queue.depth"); err != nil { return nil, err } - if m.queueWaitDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.queue.wait.duration_seconds"); err != nil { + if m.queueWaitDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.queue.wait.duration_seconds", secondsBuckets); err != nil { return nil, err } - if m.queueOldestItemAge, err = meter.Float64Histogram("unstructured_runtime.reconcile.queue.oldest_item_age_seconds"); err != nil { + if m.queueOldestItemAge, err = meter.Float64Histogram("unstructured_runtime.reconcile.queue.oldest_item_age_seconds", secondsBuckets); err != nil { return nil, err } - if m.queueWorkDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.queue.work.duration_seconds"); err != nil { + if m.queueWorkDuration, err = meter.Float64Histogram("unstructured_runtime.reconcile.queue.work.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.reconcileInFlight, err = meter.Int64ObservableGauge("unstructured_runtime.reconcile.in_flight"); err != nil { @@ -257,49 +274,49 @@ func newMetrics(meter metric.Meter, log logging.Logger, deploymentName string) ( if m.queueRequeueTotal, err = meter.Int64Counter("unstructured_runtime.reconcile.queue.requeues"); err != nil { return nil, err } - if m.connectDuration, err = meter.Float64Histogram("unstructured_runtime.external.connect.duration_seconds"); err != nil { + if m.connectDuration, err = meter.Float64Histogram("unstructured_runtime.external.connect.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.connectFailure, err = meter.Int64Counter("unstructured_runtime.external.connect.failure"); err != nil { return nil, err } - if m.observeDuration, err = meter.Float64Histogram("unstructured_runtime.external.observe.duration_seconds"); err != nil { + if m.observeDuration, err = meter.Float64Histogram("unstructured_runtime.external.observe.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.observeFailure, err = meter.Int64Counter("unstructured_runtime.external.observe.failure"); err != nil { return nil, err } - if m.createDuration, err = meter.Float64Histogram("unstructured_runtime.external.create.duration_seconds"); err != nil { + if m.createDuration, err = meter.Float64Histogram("unstructured_runtime.external.create.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.createFailure, err = meter.Int64Counter("unstructured_runtime.external.create.failure"); err != nil { return nil, err } - if m.updateDuration, err = meter.Float64Histogram("unstructured_runtime.external.update.duration_seconds"); err != nil { + if m.updateDuration, err = meter.Float64Histogram("unstructured_runtime.external.update.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.updateFailure, err = meter.Int64Counter("unstructured_runtime.external.update.failure"); err != nil { return nil, err } - if m.deleteDuration, err = meter.Float64Histogram("unstructured_runtime.external.delete.duration_seconds"); err != nil { + if m.deleteDuration, err = meter.Float64Histogram("unstructured_runtime.external.delete.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.deleteFailure, err = meter.Int64Counter("unstructured_runtime.external.delete.failure"); err != nil { return nil, err } - if m.finalizerAddDuration, err = meter.Float64Histogram("unstructured_runtime.finalizer.add.duration_seconds"); err != nil { + if m.finalizerAddDuration, err = meter.Float64Histogram("unstructured_runtime.finalizer.add.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.finalizerAddFailure, err = meter.Int64Counter("unstructured_runtime.finalizer.add.failure"); err != nil { return nil, err } - if m.finalizerRemoveDuration, err = meter.Float64Histogram("unstructured_runtime.finalizer.remove.duration_seconds"); err != nil { + if m.finalizerRemoveDuration, err = meter.Float64Histogram("unstructured_runtime.finalizer.remove.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.finalizerRemoveFailure, err = meter.Int64Counter("unstructured_runtime.finalizer.remove.failure"); err != nil { return nil, err } - if m.statusUpdateDuration, err = meter.Float64Histogram("unstructured_runtime.status.update.duration_seconds"); err != nil { + if m.statusUpdateDuration, err = meter.Float64Histogram("unstructured_runtime.status.update.duration_seconds", secondsBuckets); err != nil { return nil, err } if m.statusUpdateFailure, err = meter.Int64Counter("unstructured_runtime.status.update.failure"); err != nil { diff --git a/pkg/telemetry/metrics_test.go b/pkg/telemetry/metrics_test.go index 7b01494..42a87a8 100644 --- a/pkg/telemetry/metrics_test.go +++ b/pkg/telemetry/metrics_test.go @@ -2,6 +2,7 @@ package telemetry import ( "context" + "fmt" "testing" "time" @@ -128,3 +129,61 @@ func hasMetric(rm metricdata.ResourceMetrics, name string) bool { return false } + +// Every *_seconds histogram carries secondsBoundaries, not the SDK's millisecond-style defaults, and +// a sub-second and a multi-second reconcile land in different buckets (under the defaults both fall +// in [0, 5]). +func TestSecondsHistogramsHaveSecondsBuckets(t *testing.T) { + reader := metric.NewManualReader() + provider := metric.NewMeterProvider(metric.WithReader(reader)) + ctx := context.Background() + defer func() { _ = provider.Shutdown(ctx) }() + m, err := newMetrics(provider.Meter("test"), logging.NewNopLogger(), "test-deployment") + if err != nil { + t.Fatal(err) + } + for _, rec := range []func(context.Context, time.Duration){ + m.RecordGetDuration, m.RecordQueueWaitDuration, m.RecordQueueOldestItemAge, m.RecordQueueWorkDuration, + m.RecordConnectDuration, m.RecordObserveDuration, m.RecordCreateDuration, m.RecordUpdateDuration, + m.RecordDeleteDuration, m.RecordFinalizerAddDuration, m.RecordFinalizerRemoveDuration, m.RecordStatusUpdateDuration, + } { + rec(ctx, 10*time.Millisecond) + } + m.RecordReconcileDuration(ctx, 300*time.Millisecond) + m.RecordReconcileDuration(ctx, 2800*time.Millisecond) + + var rm metricdata.ResourceMetrics + if err := reader.Collect(ctx, &rm); err != nil { + t.Fatal(err) + } + seen := 0 + for _, sm := range rm.ScopeMetrics { + for _, md := range sm.Metrics { + h, ok := md.Data.(metricdata.Histogram[float64]) + if !ok { + continue + } + seen++ + for _, dp := range h.DataPoints { + if fmt.Sprint(dp.Bounds) != fmt.Sprint(secondsBoundaries) { + t.Errorf("%s: bounds %v, want %v", md.Name, dp.Bounds, secondsBoundaries) + } + if md.Name != "unstructured_runtime.reconcile.duration_seconds" { + continue + } + nonEmpty := 0 + for _, c := range dp.BucketCounts { + if c > 0 { + nonEmpty++ + } + } + if nonEmpty != 2 { + t.Errorf("reconcile 0.3 s and 2.8 s: %d non-empty buckets %v, want 2", nonEmpty, dp.BucketCounts) + } + } + } + } + if seen != 13 { + t.Fatalf("histograms collected: %d, want 13 (every *_seconds instrument)", seen) + } +}