diff --git a/staging/src/k8s.io/component-base/metrics/counter.go b/staging/src/k8s.io/component-base/metrics/counter.go index e41d5383bee64..6da1108afb516 100644 --- a/staging/src/k8s.io/component-base/metrics/counter.go +++ b/staging/src/k8s.io/component-base/metrics/counter.go @@ -30,23 +30,20 @@ import ( // Counter is our internal representation for our wrapping struct around prometheus // counters. Counter implements both kubeCollector and CounterMetric. type Counter struct { - ctx context.Context CounterMetric *CounterOpts lazyMetric selfCollector } +// The metric must be register-able. +var _ Registerable = &Counter{} + // The implementation of the Metric interface is expected by testutil.GetCounterMetricValue. var _ Metric = &Counter{} -// All supported exemplar metric types implement the metricWithExemplar interface. -var _ metricWithExemplar = &Counter{} - -// exemplarCounterMetric holds a context to extract exemplar labels from, and a counter metric to attach them to. It implements the metricWithExemplar interface. -type exemplarCounterMetric struct { - *Counter -} +// The implementation of kubeCollector is expected for collector registration. +var _ kubeCollector = &Counter{} // NewCounter returns an object which satisfies the kubeCollector and CounterMetric interfaces. // However, the object returned will not measure anything unless the collector is first @@ -63,6 +60,27 @@ func NewCounter(opts *CounterOpts) *Counter { return kc } +// initializeDeprecatedMetric invocation creates the actual (but deprecated) Counter. Until this method +// is called the underlying counter is a no-op. +func (c *Counter) initializeDeprecatedMetric() { + c.CounterOpts.markDeprecated() + c.initializeMetric() +} + +// initializeMetric invocation creates the actual underlying Counter. Until this method is called +// the underlying counter is a no-op. +func (c *Counter) initializeMetric() { + c.CounterOpts.annotateStabilityLevel() + // this actually creates the underlying prometheus counter. + c.setPrometheusCounter(prometheus.NewCounter(c.CounterOpts.toPromCounterOpts())) +} + +// setPrometheusCounter sets the underlying CounterMetric object, i.e. the thing that does the measurement. +func (c *Counter) setPrometheusCounter(counter prometheus.Counter) { + c.CounterMetric = counter + c.initSelfCollection(counter) +} + func (c *Counter) Desc() *prometheus.Desc { return c.metric.Desc() } @@ -79,55 +97,45 @@ func (c *Counter) Reset() { c.setPrometheusCounter(prometheus.NewCounter(c.CounterOpts.toPromCounterOpts())) } -// setPrometheusCounter sets the underlying CounterMetric object, i.e. the thing that does the measurement. -func (c *Counter) setPrometheusCounter(counter prometheus.Counter) { - c.CounterMetric = counter - c.initSelfCollection(counter) -} - // DeprecatedVersion returns a pointer to the Version or nil func (c *Counter) DeprecatedVersion() *semver.Version { return parseSemver(c.CounterOpts.DeprecatedVersion) } -// initializeMetric invocation creates the actual underlying Counter. Until this method is called -// the underlying counter is a no-op. -func (c *Counter) initializeMetric() { - c.CounterOpts.annotateStabilityLevel() - // this actually creates the underlying prometheus counter. - c.setPrometheusCounter(prometheus.NewCounter(c.CounterOpts.toPromCounterOpts())) -} - -// initializeDeprecatedMetric invocation creates the actual (but deprecated) Counter. Until this method -// is called the underlying counter is a no-op. -func (c *Counter) initializeDeprecatedMetric() { - c.CounterOpts.markDeprecated() - c.initializeMetric() -} - -// WithContext allows the normal Counter metric to pass in context. -func (c *Counter) WithContext(ctx context.Context) CounterMetric { - c.ctx = ctx - return c.CounterMetric +func (c *Counter) Add(v float64) { + c.CounterMetric.Add(v) } -// withExemplar initializes the exemplarMetric object and sets the exemplar value. -func (c *Counter) withExemplar(v float64) { - (&exemplarCounterMetric{c}).withExemplar(v) +func (c *Counter) Inc() { + c.CounterMetric.Inc() } -func (c *Counter) Add(v float64) { - c.withExemplar(v) +// CounterWithContext is the wrapper of Counter with context. +// It allows for passing contextual counters without copying their internal lock(s). +type CounterWithContext struct { + ctx context.Context + *Counter } -func (c *Counter) Inc() { - c.withExemplar(1) +// Post-equipping a counter with context, folks should be able to use it as a regular counter, with exemplar support. +var _ Metric = &CounterWithContext{} +var _ metricWithExemplar = &CounterWithContext{} + +// WithContext allows a Counter to bind to a context. +func (c *Counter) WithContext(ctx context.Context) *CounterWithContext { + // Return reference to a new counter as modifying the existing one overrides the older context, + // and blocks with semaphores. So this is a better option, see: + // https://github.com/kubernetes/kubernetes/pull/128575#discussion_r1829509144. + return &CounterWithContext{ + ctx: ctx, + Counter: c, + } } // withExemplar attaches an exemplar to the metric. -func (e *exemplarCounterMetric) withExemplar(v float64) { - if m, ok := e.CounterMetric.(prometheus.ExemplarAdder); ok { - maybeSpanCtx := trace.SpanContextFromContext(e.ctx) +func (c *CounterWithContext) withExemplar(v float64) { + if m, ok := c.CounterMetric.(prometheus.ExemplarAdder); ok { + maybeSpanCtx := trace.SpanContextFromContext(c.ctx) if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { exemplarLabels := prometheus.Labels{ "trace_id": maybeSpanCtx.TraceID().String(), @@ -138,7 +146,15 @@ func (e *exemplarCounterMetric) withExemplar(v float64) { } } - e.CounterMetric.Add(v) + c.CounterMetric.Add(v) +} + +func (c *CounterWithContext) Add(v float64) { + c.withExemplar(v) +} + +func (c *CounterWithContext) Inc() { + c.withExemplar(1) } // CounterVec is the internal representation of our wrapping struct around prometheus @@ -150,14 +166,13 @@ type CounterVec struct { originalLabels []string } +// The implementation of kubeCollector is expected for collector registration. +// CounterVec implements the kubeCollector, but not the CounterVecMetric interface. var _ kubeCollector = &CounterVec{} -// TODO: make this true: var _ CounterVecMetric = &CounterVec{} - -// NewCounterVec returns an object which satisfies the kubeCollector and (almost) CounterVecMetric interfaces. -// However, the object returned will not measure anything unless the collector is first -// registered, since the metric is lazily instantiated, and only members extracted after -// registration will actually measure anything. +// NewCounterVec returns an object which satisfies the kubeCollector and (almost) CounterVecMetric interfaces. However, +// the object returned will not measure anything unless the collector is first registered, since the metric is lazily +// instantiated, and only members extracted after registration will actually measure anything. func NewCounterVec(opts *CounterOpts, labels []string) *CounterVec { opts.StabilityLevel.setDefaults() @@ -234,6 +249,7 @@ func (v *CounterVec) WithLabelValues(lvs ...string) CounterMetric { // must match those of the VariableLabels in Desc). If that label map is // accessed for the first time, a new Counter is created IFF the counterVec has // been registered to a metrics registry. +// TODO: Do a k8s-wide refactor changing this to prometheus.Labels so CounterVec actually implements CounterVecMetric func (v *CounterVec) With(labels map[string]string) CounterMetric { if !v.IsCreated() { return noop // return no-op counter @@ -284,26 +300,10 @@ func (v *CounterVec) ResetLabelAllowLists() { v.LabelValueAllowLists = nil } -// WithContext returns wrapped CounterVec with context -func (v *CounterVec) WithContext(ctx context.Context) *CounterVecWithContext { - return &CounterVecWithContext{ - ctx: ctx, - CounterVec: v, - } -} - -// CounterVecWithContext is the wrapper of CounterVec with context. -type CounterVecWithContext struct { - *CounterVec - ctx context.Context -} - -// WithLabelValues is the wrapper of CounterVec.WithLabelValues. -func (vc *CounterVecWithContext) WithLabelValues(lvs ...string) CounterMetric { - return vc.CounterVec.WithLabelValues(lvs...) -} - -// With is the wrapper of CounterVec.With. -func (vc *CounterVecWithContext) With(labels map[string]string) CounterMetric { - return vc.CounterVec.With(labels) +// WithContext is a no-op for now, users should still attach this to vectors for seamless future API upgrades (which rely on the context). +// This is done to keep extensions (such as exemplars) on the counter and not its derivatives. +// Note that there are no actual uses for this in the codebase except for chaining it with `WithLabelValues`, which makes no use of the context. +// Furthermore, Prometheus, which is upstream from this library, does not support contextual vectors, so we don't want to diverge. +func (v *CounterVec) WithContext(_ context.Context) *CounterVec { + return v } diff --git a/staging/src/k8s.io/component-base/metrics/counter_test.go b/staging/src/k8s.io/component-base/metrics/counter_test.go index 2afcc4d63044c..1068ef82ce354 100644 --- a/staging/src/k8s.io/component-base/metrics/counter_test.go +++ b/staging/src/k8s.io/component-base/metrics/counter_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/blang/semver/v4" + "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" "github.com/stretchr/testify/assert" @@ -292,7 +293,7 @@ func TestCounterWithLabelValueAllowList(t *testing.T) { } func TestCounterWithExemplar(t *testing.T) { - // Set exemplar. + // Create context. fn := func(offset int) []byte { arr := make([]byte, 16) for i := 0; i < 16; i++ { @@ -300,6 +301,13 @@ func TestCounterWithExemplar(t *testing.T) { } return arr } + originalTraceID := trace.TraceID(fn(0)) + originalSpanID := trace.SpanID(fn(1)) + redundantCtxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ + SpanID: originalSpanID, + TraceID: originalTraceID, + TraceFlags: trace.FlagsSampled, + })) traceID := trace.TraceID(fn(1)) spanID := trace.SpanID(fn(2)) ctxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ @@ -313,8 +321,7 @@ func TestCounterWithExemplar(t *testing.T) { counter := NewCounter(&CounterOpts{ Name: "metric_exemplar_test", Help: "helpless", - }) - _ = counter.WithContext(ctxForSpanCtx) + }).WithContext(ctxForSpanCtx) // Register counter. registry := newKubeRegistry(apimachineryversion.Info{ @@ -324,17 +331,93 @@ func TestCounterWithExemplar(t *testing.T) { }) registry.MustRegister(counter) - // Call underlying exemplar methods. - counter.Add(toAdd) - counter.Inc() - counter.Inc() + // Call underlying exemplar methods, overriding the original span context. + counter.WithContext(redundantCtxForSpanCtx).Add(toAdd) // This should be counted, but it's context should be overridden. + counter.WithContext(redundantCtxForSpanCtx).Inc() // This should be counted, but it's context should be overridden. + counter.Inc() // This should be counted, and it's (parent's) context should be used. + + // Verify exemplar with the overridden span context. + exemplarGatherAndVerify(t, registry, traceID, spanID, 1, toAdd+2) + + // Modify span context. + altTraceID := trace.TraceID(fn(2)) + altSpanID := trace.SpanID(fn(3)) + altCtxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ + SpanID: altSpanID, + TraceID: altTraceID, + TraceFlags: trace.FlagsSampled, + })) + + // Call underlying exemplar methods with different span context. + counter.WithContext(ctxForSpanCtx).Add(toAdd) // This should be counted, but it's context should be overridden. + counter.WithContext(ctxForSpanCtx).Inc() // This should be counted, but it's context should be overridden. + counter.WithContext(altCtxForSpanCtx).Inc() // This should be counted, and it's context should be used. + + // Verify exemplar with different span context. + exemplarGatherAndVerify(t, registry, altTraceID, altSpanID, 1, 2*toAdd+4) + + // Verify that all contextual counter calls are exclusive. + contextualCounter := NewCounter(&CounterOpts{ + Name: "contextual_counter", + Help: "helpless", + }) + spanIDa := trace.SpanID(fn(3)) + traceIDa := trace.TraceID(fn(4)) + contextualCounterA := contextualCounter.WithContext(trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + SpanID: spanIDa, + TraceID: traceIDa, + TraceFlags: trace.FlagsSampled, + }), + )) + spanIDb := trace.SpanID(fn(5)) + traceIDb := trace.TraceID(fn(6)) + contextualCounterB := contextualCounter.WithContext(trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + SpanID: spanIDb, + TraceID: traceIDb, + TraceFlags: trace.FlagsSampled, + }), + )) + + runs := []struct { + spanID trace.SpanID + traceID trace.TraceID + contextualCounter *CounterWithContext + }{ + { + spanID: spanIDa, + traceID: traceIDa, + contextualCounter: contextualCounterA, + }, + { + spanID: spanIDb, + traceID: traceIDb, + contextualCounter: contextualCounterB, + }, + } + for i, run := range runs { + registry.MustRegister(run.contextualCounter) + run.contextualCounter.Inc() + exemplarGatherAndVerify(t, registry, run.traceID, run.spanID, 2, float64(i+1)) + registry.Unregister(run.contextualCounter) + } +} + +func exemplarGatherAndVerify(t *testing.T, + registry *kubeRegistry, + traceID trace.TraceID, + spanID trace.SpanID, + expectedMetricFamilies int, + expectedValue float64, +) { // Gather. mfs, err := registry.Gather() if err != nil { t.Fatalf("Gather failed %v", err) } - if len(mfs) != 1 { + if len(mfs) != expectedMetricFamilies { t.Fatalf("Got %v metric families, Want: 1 metric family", len(mfs)) } @@ -349,7 +432,7 @@ func TestCounterWithExemplar(t *testing.T) { } // Verify value. - want := toAdd + 2 + want := expectedValue got := m.GetCounter().GetValue() if got != want { t.Fatalf("Got %f, wanted %f as the count", got, want) @@ -382,3 +465,68 @@ func TestCounterWithExemplar(t *testing.T) { } } } + +// Prometheus does not support exemplars for metric vectors, but only the metric scalars themselves. +// The following is the only way Prometheus supports exemplars for vectors. +// We test this to make sure the same expectations are met. +func TestCounterVecWithExemplar(t *testing.T) { + registry := newKubeRegistry(apimachineryversion.Info{ + Major: "1", + Minor: "15", + GitVersion: "v1.15.0-alpha-1.12345", + }) + counterVec := NewCounterVec(&CounterOpts{ + Name: "metric_test_name", + Help: "counter help", + Subsystem: "subsystem", + Namespace: "namespace", + }, []string{"label_a", "label_b"}) + registry.MustRegister(counterVec) + + // no-op, but this shouldn't panic. + c := counterVec.WithContext(nil) + + c.WithLabelValues("1", "2").Inc() + c.WithLabelValues("1", "2").(prometheus.ExemplarAdder).AddWithExemplar(100, prometheus.Labels{ + "exemplar_label": "42", + }) + + mfs, err := registry.Gather() + require.NoError(t, err, "Gather failed %v", err) + + mf := mfs[0] + + assert.Lenf(t, mfs, 1, "Got %v metric families, Want: 1 metric family", len(mfs)) + assert.Equal(t, BuildFQName("namespace", "subsystem", "metric_test_name"), *mf.Name) + assert.Equal(t, "[ALPHA] counter help", mf.GetHelp()) + assert.Lenf(t, mf.GetMetric(), 1, "Got %v metrics, wanted 1 as the count", len(mf.GetMetric())) + + var m *dto.Metric + switch mf.GetType() { + case dto.MetricType_COUNTER: + m = mfs[0].GetMetric()[0] + default: + t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_COUNTER) + } + + var aValue, bValue string + for _, l := range m.GetLabel() { + if *l.Name == "label_a" { + aValue = *l.Value + } + if *l.Name == "label_b" { + bValue = *l.Value + } + } + labelValuePair := aValue + " " + bValue + gotValue := int(m.GetCounter().GetValue()) + gotExemplarValue := int(m.GetCounter().GetExemplar().GetValue()) + gotExemplarLabelName := m.GetCounter().GetExemplar().GetLabel()[0].GetName() + gotExemplarLabelValue := m.GetCounter().GetExemplar().GetLabel()[0].GetValue() + + assert.Equalf(t, "1 2", labelValuePair, "Got %v, wanted %v as the label values", labelValuePair, "1 2") + assert.Equalf(t, 101, gotValue, "Got %v, wanted %v as the count while setting label_a to %v and label b to %v", gotValue, 101, aValue, bValue) + assert.Equalf(t, 100, gotExemplarValue, "Got %v, wanted %v as the exemplar value while setting label_a to %v and label b to %v", gotExemplarValue, 100, aValue, bValue) + assert.Equalf(t, "exemplar_label", gotExemplarLabelName, "Got %v, wanted %v as the exemplar label name", gotExemplarLabelName, "exemplar_label") + assert.Equalf(t, "42", gotExemplarLabelValue, "Got %v, wanted %v as the exemplar label value", gotExemplarLabelValue, "42") +} diff --git a/staging/src/k8s.io/component-base/metrics/histogram.go b/staging/src/k8s.io/component-base/metrics/histogram.go index b410951b67832..30e67e61ea016 100644 --- a/staging/src/k8s.io/component-base/metrics/histogram.go +++ b/staging/src/k8s.io/component-base/metrics/histogram.go @@ -22,54 +22,26 @@ import ( "github.com/blang/semver/v4" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "go.opentelemetry.io/otel/trace" ) -// Histogram is our internal representation for our wrapping struct around prometheus -// histograms. Summary implements both kubeCollector and ObserverMetric +// Histogram is our internal representation for our wrapping struct around prometheus histograms. type Histogram struct { - ctx context.Context ObserverMetric *HistogramOpts lazyMetric selfCollector } -// exemplarHistogramMetric holds a context to extract exemplar labels from, and a historgram metric to attach them to. It implements the metricWithExemplar interface. -type exemplarHistogramMetric struct { - *Histogram -} +// The metric must be register-able. +var _ Registerable = &Histogram{} -type exemplarHistogramVec struct { - *HistogramVecWithContext - observer prometheus.Observer -} +// The implementation of the Metric interface is expected by testutil.GetHistogramMetricValue. +var _ Metric = &Histogram{} -func (h *Histogram) Observe(v float64) { - h.withExemplar(v) -} - -// withExemplar initializes the exemplarMetric object and sets the exemplar value. -func (h *Histogram) withExemplar(v float64) { - (&exemplarHistogramMetric{h}).withExemplar(v) -} - -// withExemplar attaches an exemplar to the metric. -func (e *exemplarHistogramMetric) withExemplar(v float64) { - if m, ok := e.Histogram.ObserverMetric.(prometheus.ExemplarObserver); ok { - maybeSpanCtx := trace.SpanContextFromContext(e.ctx) - if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { - exemplarLabels := prometheus.Labels{ - "trace_id": maybeSpanCtx.TraceID().String(), - "span_id": maybeSpanCtx.SpanID().String(), - } - m.ObserveWithExemplar(v, exemplarLabels) - return - } - } - - e.ObserverMetric.Observe(v) -} +// The implementation of kubeCollector is expected for collector registration. +var _ kubeCollector = &Histogram{} // NewHistogram returns an object which is Histogram-like. However, nothing // will be measured until the histogram is registered somewhere. @@ -85,36 +57,89 @@ func NewHistogram(opts *HistogramOpts) *Histogram { return h } +// initializeDeprecatedMetric invokes the actual prometheus.Histogram object instantiation +// but modifies the Help description prior to object instantiation. +func (h *Histogram) initializeDeprecatedMetric() { + h.HistogramOpts.markDeprecated() + h.initializeMetric() +} + +// initializeMetric invokes the actual prometheus.Histogram object instantiation +// and stores a reference to it +func (h *Histogram) initializeMetric() { + h.HistogramOpts.annotateStabilityLevel() + // this actually creates the underlying prometheus gauge. + h.setPrometheusHistogram(prometheus.NewHistogram(h.HistogramOpts.toPromHistogramOpts())) +} + // setPrometheusHistogram sets the underlying KubeGauge object, i.e. the thing that does the measurement. func (h *Histogram) setPrometheusHistogram(histogram prometheus.Histogram) { h.ObserverMetric = histogram h.initSelfCollection(histogram) } +// Desc returns the prometheus.Desc for the metric. +// This method is required by the Metric interface. +func (h *Histogram) Desc() *prometheus.Desc { + return h.metric.Desc() +} + +// Write writes the metric to the provided dto.Metric. +// This method is required by the Metric interface. +func (h *Histogram) Write(to *dto.Metric) error { + return h.metric.Write(to) +} + // DeprecatedVersion returns a pointer to the Version or nil func (h *Histogram) DeprecatedVersion() *semver.Version { return parseSemver(h.HistogramOpts.DeprecatedVersion) } -// initializeMetric invokes the actual prometheus.Histogram object instantiation -// and stores a reference to it -func (h *Histogram) initializeMetric() { - h.HistogramOpts.annotateStabilityLevel() - // this actually creates the underlying prometheus gauge. - h.setPrometheusHistogram(prometheus.NewHistogram(h.HistogramOpts.toPromHistogramOpts())) +// Observe is the method that implements the ObserverMetric interface. +func (h *Histogram) Observe(v float64) { + h.ObserverMetric.Observe(v) } -// initializeDeprecatedMetric invokes the actual prometheus.Histogram object instantiation -// but modifies the Help description prior to object instantiation. -func (h *Histogram) initializeDeprecatedMetric() { - h.HistogramOpts.markDeprecated() - h.initializeMetric() +// HistogramWithContext holds a context to extract exemplar labels from, and a historgram metric to attach them to. It implements the metricWithExemplar interface. +type HistogramWithContext struct { + ctx context.Context + *Histogram +} + +// Post-equipping a histogram with context, folks should be able to use it as a regular histogram, with exemplar support. +var _ Metric = &HistogramWithContext{} +var _ metricWithExemplar = &HistogramWithContext{} + +// WithContext allows a Counter to bind to a context. +func (h *Histogram) WithContext(ctx context.Context) *HistogramWithContext { + // Return reference to a new histogram as modifying the existing one overrides the older context, + // and blocks with semaphores. So this is a better option, see: + // https://github.com/kubernetes/kubernetes/pull/128575#discussion_r1829509144. + return &HistogramWithContext{ + ctx: ctx, + Histogram: h, + } } -// WithContext allows the normal Histogram metric to pass in context. The context is no-op now. -func (h *Histogram) WithContext(ctx context.Context) ObserverMetric { - h.ctx = ctx - return h.ObserverMetric +// withExemplar attaches an exemplar to the metric. +func (e *HistogramWithContext) withExemplar(v float64) { + if m, ok := e.Histogram.ObserverMetric.(prometheus.ExemplarObserver); ok { + maybeSpanCtx := trace.SpanContextFromContext(e.ctx) + if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { + exemplarLabels := prometheus.Labels{ + "trace_id": maybeSpanCtx.TraceID().String(), + "span_id": maybeSpanCtx.SpanID().String(), + } + m.ObserveWithExemplar(v, exemplarLabels) + return + } + } + + e.ObserverMetric.Observe(v) +} + +func (e *HistogramWithContext) Observe(v float64) { + e.withExemplar(v) } // HistogramVec is the internal representation of our wrapping struct around prometheus @@ -126,12 +151,13 @@ type HistogramVec struct { originalLabels []string } -// NewHistogramVec returns an object which satisfies kubeCollector and wraps the -// prometheus.HistogramVec object. However, the object returned will not measure -// anything unless the collector is first registered, since the metric is lazily instantiated, -// and only members extracted after -// registration will actually measure anything. +// The implementation of kubeCollector is expected for collector registration. +// HistogramVec implements the kubeCollector interface, and not ObserverMetric interface. +var _ kubeCollector = &HistogramVec{} +// NewHistogramVec returns an object which satisfies kubeCollector and wraps the prometheus.HistogramVec object. +// However, the object returned will not measure anything unless the collector is first registered, since the metric is +// lazily instantiated, and only members extracted after registration will actually measure anything. func NewHistogramVec(opts *HistogramOpts, labels []string) *HistogramVec { opts.StabilityLevel.setDefaults() @@ -254,51 +280,10 @@ func (v *HistogramVec) ResetLabelAllowLists() { v.LabelValueAllowLists = nil } -// WithContext returns wrapped HistogramVec with context -func (v *HistogramVec) WithContext(ctx context.Context) *HistogramVecWithContext { - return &HistogramVecWithContext{ - ctx: ctx, - HistogramVec: v, - } -} - -// HistogramVecWithContext is the wrapper of HistogramVec with context. -type HistogramVecWithContext struct { - *HistogramVec - ctx context.Context -} - -func (h *exemplarHistogramVec) Observe(v float64) { - h.withExemplar(v) -} - -func (h *exemplarHistogramVec) withExemplar(v float64) { - if m, ok := h.observer.(prometheus.ExemplarObserver); ok { - maybeSpanCtx := trace.SpanContextFromContext(h.HistogramVecWithContext.ctx) - if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { - m.ObserveWithExemplar(v, prometheus.Labels{ - "trace_id": maybeSpanCtx.TraceID().String(), - "span_id": maybeSpanCtx.SpanID().String(), - }) - return - } - } - - h.observer.Observe(v) -} - -// WithLabelValues is the wrapper of HistogramVec.WithLabelValues. -func (vc *HistogramVecWithContext) WithLabelValues(lvs ...string) *exemplarHistogramVec { - return &exemplarHistogramVec{ - HistogramVecWithContext: vc, - observer: vc.HistogramVec.WithLabelValues(lvs...), - } -} - -// With is the wrapper of HistogramVec.With. -func (vc *HistogramVecWithContext) With(labels map[string]string) *exemplarHistogramVec { - return &exemplarHistogramVec{ - HistogramVecWithContext: vc, - observer: vc.HistogramVec.With(labels), - } +// WithContext is a no-op for now, users should still attach this to vectors for seamless future API upgrades (which rely on the context). +// This is done to keep extensions (such as exemplars) on the counter and not its derivatives. +// Note that there are no actual uses for this in the codebase except for chaining it with `WithLabelValues`, which makes no use of the context. +// Furthermore, Prometheus, which is upstream from this library, does not support contextual vectors, so we don't want to diverge. +func (v *HistogramVec) WithContext(_ context.Context) *HistogramVec { + return v } diff --git a/staging/src/k8s.io/component-base/metrics/histogram_test.go b/staging/src/k8s.io/component-base/metrics/histogram_test.go index 5efbfb6eeae2d..3b8e9affb4434 100644 --- a/staging/src/k8s.io/component-base/metrics/histogram_test.go +++ b/staging/src/k8s.io/component-base/metrics/histogram_test.go @@ -318,7 +318,7 @@ func TestHistogramWithLabelValueAllowList(t *testing.T) { } func TestHistogramWithExemplar(t *testing.T) { - // Arrange. + // Create context. traceID := trace.TraceID([]byte("trace-0000-xxxxx")) spanID := trace.SpanID([]byte("span-0000-xxxxx")) ctxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ @@ -328,13 +328,14 @@ func TestHistogramWithExemplar(t *testing.T) { })) value := float64(10) + // Create contextual histogram. histogram := NewHistogram(&HistogramOpts{ Name: "histogram_exemplar_test", Help: "helpless", Buckets: []float64{100}, - }) - _ = histogram.WithContext(ctxForSpanCtx) + }).WithContext(ctxForSpanCtx) + // Register histogram. registry := newKubeRegistry(apimachineryversion.Info{ Major: "1", Minor: "15", @@ -342,53 +343,51 @@ func TestHistogramWithExemplar(t *testing.T) { }) registry.MustRegister(histogram) - // Act. + // Call underlying exemplar methods. histogram.Observe(value) - // Assert. + // Gather. mfs, err := registry.Gather() if err != nil { t.Fatalf("Gather failed %v", err) } - if len(mfs) != 1 { t.Fatalf("Got %v metric families, Want: 1 metric family", len(mfs)) } + // Verify metric type. mf := mfs[0] var m *dto.Metric switch mf.GetType() { case dto.MetricType_HISTOGRAM: m = mfs[0].GetMetric()[0] default: - t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_COUNTER) + t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_HISTOGRAM) } + // Verify value. want := value got := m.GetHistogram().GetSampleSum() if got != want { t.Fatalf("Got %f, wanted %f as the count", got, want) } + // Verify exemplars. buckets := m.GetHistogram().GetBucket() if len(buckets) == 0 { t.Fatalf("Got 0 buckets, wanted 1") } - e := buckets[0].GetExemplar() if e == nil { t.Fatalf("Got nil exemplar, wanted an exemplar") } - eLabels := e.GetLabel() if eLabels == nil { t.Fatalf("Got nil exemplar label, wanted an exemplar label") } - if len(eLabels) != 2 { t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(eLabels)) } - for _, l := range eLabels { switch *l.Name { case "trace_id": @@ -403,92 +402,150 @@ func TestHistogramWithExemplar(t *testing.T) { t.Fatalf("Got unexpected label %s", *l.Name) } } -} - -func TestHistogramVecWithExemplar(t *testing.T) { - // Arrange. - traceID := trace.TraceID([]byte("trace-0000-xxxxx")) - spanID := trace.SpanID([]byte("span-0000-xxxxx")) - ctxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: traceID, - SpanID: spanID, - TraceFlags: trace.FlagsSampled, - })) - value := float64(10) - histogramVec := NewHistogramVec(&HistogramOpts{ - Name: "histogram_exemplar_test", + // Verify that all contextual histogram calls are exclusive. + contextualHistogram := NewHistogram(&HistogramOpts{ + Name: "contextual_histogram", Help: "helpless", Buckets: []float64{100}, - }, []string{"group"}) - h := histogramVec.WithContext(ctxForSpanCtx) + }) + traceIDa := trace.TraceID([]byte("trace-0000-aaaaa")) + spanIDa := trace.SpanID([]byte("span-0000-aaaaa")) + contextualHistogramA := contextualHistogram.WithContext(trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceIDa, + SpanID: spanIDa, + TraceFlags: trace.FlagsSampled, + }), + )) + traceIDb := trace.TraceID([]byte("trace-0000-bbbbb")) + spanIDb := trace.SpanID([]byte("span-0000-bbbbb")) + contextualHistogramB := contextualHistogram.WithContext(trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceIDb, + SpanID: spanIDb, + TraceFlags: trace.FlagsSampled, + }), + )) + + runs := []struct { + spanID trace.SpanID + traceID trace.TraceID + contextualHistogram *HistogramWithContext + }{ + { + spanID: spanIDa, + traceID: traceIDa, + contextualHistogram: contextualHistogramA, + }, + { + spanID: spanIDb, + traceID: traceIDb, + contextualHistogram: contextualHistogramB, + }, + } + for _, run := range runs { + registry.MustRegister(run.contextualHistogram) + run.contextualHistogram.Observe(value) + + mfs, err = registry.Gather() + if err != nil { + t.Fatalf("Gather failed %v", err) + } + if len(mfs) != 2 { + t.Fatalf("Got %v metric families, Want: 2 metric families", len(mfs)) + } + + dtoMetric := mfs[0].GetMetric()[0] + dtoMetricBuckets := dtoMetric.GetHistogram().GetBucket() + if len(dtoMetricBuckets) == 0 { + t.Fatalf("Got nil buckets") + } + dtoMetricBucketsExemplar := dtoMetricBuckets[0].GetExemplar() + if dtoMetricBucketsExemplar == nil { + t.Fatalf("Got nil exemplar") + } + + dtoMetricLabels := dtoMetricBucketsExemplar.GetLabel() + if len(dtoMetricLabels) != 2 { + t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(dtoMetricLabels)) + } + for _, l := range dtoMetricLabels { + switch *l.Name { + case "trace_id": + if *l.Value != run.traceID.String() { + t.Fatalf("Got %s as traceID, wanted %s", *l.Value, run.traceID.String()) + } + case "span_id": + if *l.Value != run.spanID.String() { + t.Fatalf("Got %s as spanID, wanted %s", *l.Value, run.spanID.String()) + } + default: + t.Fatalf("Got unexpected label %s", *l.Name) + } + } + + registry.Unregister(run.contextualHistogram) + } +} +func TestHistogramVecWithExemplar(t *testing.T) { registry := newKubeRegistry(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", }) + histogramVec := NewHistogramVec(&HistogramOpts{ + Name: "histogram_exemplar_test", + Help: "histogram help", + Namespace: "namespace", + Subsystem: "subsystem", + Buckets: []float64{100}, + }, []string{"group"}) registry.MustRegister(histogramVec) - // Act. + // no-op, but this shouldn't panic. + h := histogramVec.WithContext(nil) + + value := float64(1) h.WithLabelValues("foo").Observe(value) + h.WithLabelValues("foo").(prometheus.ExemplarObserver).ObserveWithExemplar(100, prometheus.Labels{ + "exemplar_label": "42", + }) - // Assert. mfs, err := registry.Gather() - if err != nil { - t.Fatalf("Gather failed %v", err) - } - - if len(mfs) != 1 { - t.Fatalf("Got %v metric families, Want: 1 metric family", len(mfs)) - } + require.NoError(t, err, "Gather failed %v", err) mf := mfs[0] + + assert.Lenf(t, mfs, 1, "Got %v metric families, Want: 1 metric family", len(mfs)) + assert.Equal(t, BuildFQName("namespace", "subsystem", "histogram_exemplar_test"), *mf.Name) + assert.Equal(t, "[ALPHA] histogram help", mf.GetHelp()) + assert.Lenf(t, mf.GetMetric(), 1, "Got %v metrics, wanted 1 as the count", len(mf.GetMetric())) + var m *dto.Metric switch mf.GetType() { case dto.MetricType_HISTOGRAM: m = mfs[0].GetMetric()[0] default: - t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_COUNTER) - } - - want := value - got := m.GetHistogram().GetSampleSum() - if got != want { - t.Fatalf("Got %f, wanted %f as the count", got, want) - } - - buckets := m.GetHistogram().GetBucket() - if len(buckets) == 0 { - t.Fatalf("Got 0 buckets, wanted 1") + t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_HISTOGRAM) } - e := buckets[0].GetExemplar() - if e == nil { - t.Fatalf("Got nil exemplar, wanted an exemplar") - } - - eLabels := e.GetLabel() - if eLabels == nil { - t.Fatalf("Got nil exemplar label, wanted an exemplar label") + var labelValue string + for _, l := range m.GetLabel() { + if *l.Name == "group" { + labelValue = *l.Value + } } - if len(eLabels) != 2 { - t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(eLabels)) - } + gotValue := m.GetHistogram().GetSampleSum() + gotExemplarValue := m.GetHistogram().GetBucket()[0].GetExemplar().GetValue() + gotExemplarLabelName := m.GetHistogram().GetBucket()[0].GetExemplar().GetLabel()[0].GetName() + gotExemplarLabelValue := m.GetHistogram().GetBucket()[0].GetExemplar().GetLabel()[0].GetValue() - for _, l := range eLabels { - switch *l.Name { - case "trace_id": - if *l.Value != traceID.String() { - t.Fatalf("Got %s as traceID, wanted %s", *l.Value, traceID.String()) - } - case "span_id": - if *l.Value != spanID.String() { - t.Fatalf("Got %s as spanID, wanted %s", *l.Value, spanID.String()) - } - default: - t.Fatalf("Got unexpected label %s", *l.Name) - } - } + assert.Equalf(t, "foo", labelValue, "Got %v, wanted %v as the label values", labelValue, "foo") + assert.Equalf(t, 101, int(gotValue), "Got %v, wanted %v as the count while setting group to %v", gotValue, 101, labelValue) + assert.Equalf(t, 100, int(gotExemplarValue), "Got %v, wanted %v as the exemplar value while setting group to %v", gotExemplarValue, 100, labelValue) + assert.Equalf(t, "exemplar_label", gotExemplarLabelName, "Got %v, wanted %v as the exemplar label name", gotExemplarLabelName, "exemplar_label") + assert.Equalf(t, "42", gotExemplarLabelValue, "Got %v, wanted %v as the exemplar label value", gotExemplarLabelValue, "42") }