Skip to content

address counters and histograms context race and exemplars overhaul #128575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

address counters and histograms context race and exemplars overhaul #128575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e85cd37
address `metrics.(*Counter)` context data-race
rexagod Nov 5, 2024
5d048f1
fixup! address `metrics.(*Counter)` context data-race
rexagod Dec 19, 2024
4027286
fixup! fixup! address `metrics.(*Counter)` context data-race
rexagod Mar 5, 2025
61f7358
fixup! fixup! fixup! address `metrics.(*Counter)` context data-race
rexagod Mar 9, 2025
1b5b50b
fixup! fixup! fixup! fixup! address `metrics.(*Counter)` context data…
rexagod Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 72 additions & 72 deletions staging/src/k8s.io/component-base/metrics/counter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
}
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need to check that every time?

Wouldn't it be better to implement AddWithExemplar?

maybeSpanCtx := trace.SpanContextFromContext(c.ctx)
if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() {
exemplarLabels := prometheus.Labels{
"trace_id": maybeSpanCtx.TraceID().String(),
Expand All @@ -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
Expand All @@ -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()

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Copy link
Member Author

@rexagod rexagod Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could still be breaking for downstream users that rely on the earlier WithContext(...).ctx in some way or the other, albeit no such use case was found in Kubernetes itself.

I'm assuming general API stability cycles apply for that API, so we may want to push this out gradually, although I really doubt anyone will read value from this context rather than doing so just one step earlier in the chain (before passing 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
}
Loading