| Lorenz Brun | 5c52062 | 2025-01-14 14:14:52 +0100 | [diff] [blame^] | 1 | From 580612cea9e4eaec1c25821d872e1551335f8c95 Mon Sep 17 00:00:00 2001 |
| 2 | From: Pranshu Srivastava <rexagod@gmail.com> |
| 3 | Date: Tue, 5 Nov 2024 18:37:40 +0530 |
| 4 | Subject: [PATCH] address `metrics.(*Counter)` context data-race |
| 5 | |
| 6 | PR: https://github.com/kubernetes/kubernetes/pull/128575 |
| 7 | Fixes: https://github.com/kubernetes/kubernetes/issues/128548 |
| 8 | Signed-off-by: Pranshu Srivastava <rexagod@gmail.com> |
| 9 | --- |
| 10 | .../k8s.io/component-base/metrics/counter.go | 105 +++++--- |
| 11 | .../component-base/metrics/counter_test.go | 254 +++++++++++++++++- |
| 12 | .../component-base/metrics/histogram.go | 44 +-- |
| 13 | .../component-base/metrics/histogram_test.go | 207 ++++++++++++-- |
| 14 | 4 files changed, 537 insertions(+), 73 deletions(-) |
| 15 | |
| 16 | diff --git a/metrics/counter.go b/metrics/counter.go |
| 17 | index 8a7dd71541c23..f92c8d59f00e5 100644 |
| 18 | --- a/metrics/counter.go |
| 19 | +++ b/metrics/counter.go |
| 20 | @@ -30,23 +30,22 @@ import ( |
| 21 | // Counter is our internal representation for our wrapping struct around prometheus |
| 22 | // counters. Counter implements both kubeCollector and CounterMetric. |
| 23 | type Counter struct { |
| 24 | - ctx context.Context |
| 25 | CounterMetric |
| 26 | *CounterOpts |
| 27 | lazyMetric |
| 28 | selfCollector |
| 29 | } |
| 30 | |
| 31 | +type CounterWithContext struct { |
| 32 | + ctx context.Context |
| 33 | + *Counter |
| 34 | +} |
| 35 | + |
| 36 | // The implementation of the Metric interface is expected by testutil.GetCounterMetricValue. |
| 37 | var _ Metric = &Counter{} |
| 38 | |
| 39 | // All supported exemplar metric types implement the metricWithExemplar interface. |
| 40 | -var _ metricWithExemplar = &Counter{} |
| 41 | - |
| 42 | -// exemplarCounterMetric holds a context to extract exemplar labels from, and a counter metric to attach them to. It implements the metricWithExemplar interface. |
| 43 | -type exemplarCounterMetric struct { |
| 44 | - *Counter |
| 45 | -} |
| 46 | +var _ metricWithExemplar = &CounterWithContext{} |
| 47 | |
| 48 | // NewCounter returns an object which satisfies the kubeCollector and CounterMetric interfaces. |
| 49 | // However, the object returned will not measure anything unless the collector is first |
| 50 | @@ -106,28 +105,17 @@ func (c *Counter) initializeDeprecatedMetric() { |
| 51 | } |
| 52 | |
| 53 | // WithContext allows the normal Counter metric to pass in context. |
| 54 | -func (c *Counter) WithContext(ctx context.Context) CounterMetric { |
| 55 | - c.ctx = ctx |
| 56 | - return c.CounterMetric |
| 57 | -} |
| 58 | - |
| 59 | -// withExemplar initializes the exemplarMetric object and sets the exemplar value. |
| 60 | -func (c *Counter) withExemplar(v float64) { |
| 61 | - (&exemplarCounterMetric{c}).withExemplar(v) |
| 62 | -} |
| 63 | - |
| 64 | -func (c *Counter) Add(v float64) { |
| 65 | - c.withExemplar(v) |
| 66 | -} |
| 67 | - |
| 68 | -func (c *Counter) Inc() { |
| 69 | - c.withExemplar(1) |
| 70 | +func (c *Counter) WithContext(ctx context.Context) *CounterWithContext { |
| 71 | + return &CounterWithContext{ |
| 72 | + ctx: ctx, |
| 73 | + Counter: c, |
| 74 | + } |
| 75 | } |
| 76 | |
| 77 | // withExemplar attaches an exemplar to the metric. |
| 78 | -func (e *exemplarCounterMetric) withExemplar(v float64) { |
| 79 | - if m, ok := e.CounterMetric.(prometheus.ExemplarAdder); ok { |
| 80 | - maybeSpanCtx := trace.SpanContextFromContext(e.ctx) |
| 81 | +func (c *CounterWithContext) withExemplar(v float64) { |
| 82 | + if m, ok := c.CounterMetric.(prometheus.ExemplarAdder); ok { |
| 83 | + maybeSpanCtx := trace.SpanContextFromContext(c.ctx) |
| 84 | if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { |
| 85 | exemplarLabels := prometheus.Labels{ |
| 86 | "trace_id": maybeSpanCtx.TraceID().String(), |
| 87 | @@ -138,7 +126,23 @@ func (e *exemplarCounterMetric) withExemplar(v float64) { |
| 88 | } |
| 89 | } |
| 90 | |
| 91 | - e.CounterMetric.Add(v) |
| 92 | + c.CounterMetric.Add(v) |
| 93 | +} |
| 94 | + |
| 95 | +func (c *Counter) Add(v float64) { |
| 96 | + c.CounterMetric.Add(v) |
| 97 | +} |
| 98 | + |
| 99 | +func (c *Counter) Inc() { |
| 100 | + c.CounterMetric.Inc() |
| 101 | +} |
| 102 | + |
| 103 | +func (c *CounterWithContext) Add(v float64) { |
| 104 | + c.withExemplar(v) |
| 105 | +} |
| 106 | + |
| 107 | +func (c *CounterWithContext) Inc() { |
| 108 | + c.withExemplar(1) |
| 109 | } |
| 110 | |
| 111 | // CounterVec is the internal representation of our wrapping struct around prometheus |
| 112 | @@ -295,12 +299,51 @@ type CounterVecWithContext struct { |
| 113 | ctx context.Context |
| 114 | } |
| 115 | |
| 116 | +// CounterVecWithContextWithCounter is the wrapper of CounterVecWithContext with counter. |
| 117 | +type CounterVecWithContextWithCounter struct { |
| 118 | + *CounterVecWithContext |
| 119 | + counter CounterMetric |
| 120 | +} |
| 121 | + |
| 122 | +// withExemplar attaches an exemplar to the metric. |
| 123 | +func (vcc *CounterVecWithContextWithCounter) withExemplar(v float64) { |
| 124 | + if m, ok := vcc.counter.(prometheus.ExemplarAdder); ok { |
| 125 | + maybeSpanCtx := trace.SpanContextFromContext(vcc.ctx) |
| 126 | + if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { |
| 127 | + exemplarLabels := prometheus.Labels{ |
| 128 | + "trace_id": maybeSpanCtx.TraceID().String(), |
| 129 | + "span_id": maybeSpanCtx.SpanID().String(), |
| 130 | + } |
| 131 | + m.AddWithExemplar(v, exemplarLabels) |
| 132 | + return |
| 133 | + } |
| 134 | + } |
| 135 | + |
| 136 | + vcc.counter.Add(v) |
| 137 | +} |
| 138 | + |
| 139 | +// Add adds the given value to the counter with the provided labels. |
| 140 | +func (vcc *CounterVecWithContextWithCounter) Add(v float64) { |
| 141 | + vcc.withExemplar(v) |
| 142 | +} |
| 143 | + |
| 144 | +// Inc increments the counter with the provided labels. |
| 145 | +func (vcc *CounterVecWithContextWithCounter) Inc() { |
| 146 | + vcc.withExemplar(1) |
| 147 | +} |
| 148 | + |
| 149 | // WithLabelValues is the wrapper of CounterVec.WithLabelValues. |
| 150 | -func (vc *CounterVecWithContext) WithLabelValues(lvs ...string) CounterMetric { |
| 151 | - return vc.CounterVec.WithLabelValues(lvs...) |
| 152 | +func (vc *CounterVecWithContext) WithLabelValues(lvs ...string) *CounterVecWithContextWithCounter { |
| 153 | + return &CounterVecWithContextWithCounter{ |
| 154 | + CounterVecWithContext: vc, |
| 155 | + counter: vc.CounterVec.WithLabelValues(lvs...), |
| 156 | + } |
| 157 | } |
| 158 | |
| 159 | // With is the wrapper of CounterVec.With. |
| 160 | -func (vc *CounterVecWithContext) With(labels map[string]string) CounterMetric { |
| 161 | - return vc.CounterVec.With(labels) |
| 162 | +func (vc *CounterVecWithContext) With(labels map[string]string) *CounterVecWithContextWithCounter { |
| 163 | + return &CounterVecWithContextWithCounter{ |
| 164 | + CounterVecWithContext: vc, |
| 165 | + counter: vc.CounterVec.With(labels), |
| 166 | + } |
| 167 | } |
| 168 | diff --git a/metrics/counter_test.go b/metrics/counter_test.go |
| 169 | index 2afcc4d63044c..ea47cab282985 100644 |
| 170 | --- a/metrics/counter_test.go |
| 171 | +++ b/metrics/counter_test.go |
| 172 | @@ -292,7 +292,7 @@ func TestCounterWithLabelValueAllowList(t *testing.T) { |
| 173 | } |
| 174 | |
| 175 | func TestCounterWithExemplar(t *testing.T) { |
| 176 | - // Set exemplar. |
| 177 | + // Create context. |
| 178 | fn := func(offset int) []byte { |
| 179 | arr := make([]byte, 16) |
| 180 | for i := 0; i < 16; i++ { |
| 181 | @@ -313,8 +313,7 @@ func TestCounterWithExemplar(t *testing.T) { |
| 182 | counter := NewCounter(&CounterOpts{ |
| 183 | Name: "metric_exemplar_test", |
| 184 | Help: "helpless", |
| 185 | - }) |
| 186 | - _ = counter.WithContext(ctxForSpanCtx) |
| 187 | + }).WithContext(ctxForSpanCtx) |
| 188 | |
| 189 | // Register counter. |
| 190 | registry := newKubeRegistry(apimachineryversion.Info{ |
| 191 | @@ -381,4 +380,253 @@ func TestCounterWithExemplar(t *testing.T) { |
| 192 | t.Fatalf("Got unexpected label %s", *l.Name) |
| 193 | } |
| 194 | } |
| 195 | + |
| 196 | + // Verify that all contextual counter calls are exclusive. |
| 197 | + contextualCounter := NewCounter(&CounterOpts{ |
| 198 | + Name: "contextual_counter", |
| 199 | + Help: "helpless", |
| 200 | + }) |
| 201 | + spanIDa := trace.SpanID(fn(3)) |
| 202 | + traceIDa := trace.TraceID(fn(4)) |
| 203 | + contextualCounterA := contextualCounter.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 204 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 205 | + SpanID: spanIDa, |
| 206 | + TraceID: traceIDa, |
| 207 | + TraceFlags: trace.FlagsSampled, |
| 208 | + }), |
| 209 | + )) |
| 210 | + spanIDb := trace.SpanID(fn(5)) |
| 211 | + traceIDb := trace.TraceID(fn(6)) |
| 212 | + contextualCounterB := contextualCounter.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 213 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 214 | + SpanID: spanIDb, |
| 215 | + TraceID: traceIDb, |
| 216 | + TraceFlags: trace.FlagsSampled, |
| 217 | + }), |
| 218 | + )) |
| 219 | + |
| 220 | + runs := []struct { |
| 221 | + spanID trace.SpanID |
| 222 | + traceID trace.TraceID |
| 223 | + contextualCounter *CounterWithContext |
| 224 | + }{ |
| 225 | + { |
| 226 | + spanID: spanIDa, |
| 227 | + traceID: traceIDa, |
| 228 | + contextualCounter: contextualCounterA, |
| 229 | + }, |
| 230 | + { |
| 231 | + spanID: spanIDb, |
| 232 | + traceID: traceIDb, |
| 233 | + contextualCounter: contextualCounterB, |
| 234 | + }, |
| 235 | + } |
| 236 | + for _, run := range runs { |
| 237 | + registry.MustRegister(run.contextualCounter) |
| 238 | + run.contextualCounter.Inc() |
| 239 | + |
| 240 | + mfs, err = registry.Gather() |
| 241 | + if err != nil { |
| 242 | + t.Fatalf("Gather failed %v", err) |
| 243 | + } |
| 244 | + if len(mfs) != 2 { |
| 245 | + t.Fatalf("Got %v metric families, Want: 2 metric families", len(mfs)) |
| 246 | + } |
| 247 | + |
| 248 | + dtoMetric := mfs[0].GetMetric()[0] |
| 249 | + if dtoMetric.GetCounter().GetExemplar() == nil { |
| 250 | + t.Fatalf("Got nil exemplar, wanted an exemplar") |
| 251 | + } |
| 252 | + dtoMetricLabels := dtoMetric.GetCounter().GetExemplar().GetLabel() |
| 253 | + if len(dtoMetricLabels) != 2 { |
| 254 | + t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(dtoMetricLabels)) |
| 255 | + } |
| 256 | + for _, l := range dtoMetricLabels { |
| 257 | + switch *l.Name { |
| 258 | + case "trace_id": |
| 259 | + if *l.Value != run.traceID.String() { |
| 260 | + t.Fatalf("Got %s as traceID, wanted %s", *l.Value, run.traceID.String()) |
| 261 | + } |
| 262 | + case "span_id": |
| 263 | + if *l.Value != run.spanID.String() { |
| 264 | + t.Fatalf("Got %s as spanID, wanted %s", *l.Value, run.spanID.String()) |
| 265 | + } |
| 266 | + default: |
| 267 | + t.Fatalf("Got unexpected label %s", *l.Name) |
| 268 | + } |
| 269 | + } |
| 270 | + |
| 271 | + registry.Unregister(run.contextualCounter) |
| 272 | + } |
| 273 | +} |
| 274 | + |
| 275 | +func TestCounterVecWithExemplar(t *testing.T) { |
| 276 | + // Create context. |
| 277 | + fn := func(offset int) []byte { |
| 278 | + arr := make([]byte, 16) |
| 279 | + for i := 0; i < 16; i++ { |
| 280 | + arr[i] = byte(2<<7 - i - offset) |
| 281 | + } |
| 282 | + return arr |
| 283 | + } |
| 284 | + traceID := trace.TraceID(fn(1)) |
| 285 | + spanID := trace.SpanID(fn(2)) |
| 286 | + ctxForSpanCtx := trace.ContextWithSpanContext(context.Background(), |
| 287 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 288 | + SpanID: spanID, |
| 289 | + TraceID: traceID, |
| 290 | + TraceFlags: trace.FlagsSampled, |
| 291 | + }), |
| 292 | + ) |
| 293 | + toAdd := float64(40) |
| 294 | + |
| 295 | + // Create contextual counter. |
| 296 | + counter := NewCounterVec(&CounterOpts{ |
| 297 | + Name: "metricvec_exemplar_test", |
| 298 | + Help: "helpless", |
| 299 | + }, []string{"label_a"}).WithContext(ctxForSpanCtx) |
| 300 | + |
| 301 | + // Register counter. |
| 302 | + registry := newKubeRegistry(apimachineryversion.Info{ |
| 303 | + Major: "1", |
| 304 | + Minor: "15", |
| 305 | + GitVersion: "v1.15.0-alpha-1.12345", |
| 306 | + }) |
| 307 | + registry.MustRegister(counter) |
| 308 | + |
| 309 | + // Call underlying exemplar methods. |
| 310 | + counter.WithLabelValues("a").Add(toAdd) |
| 311 | + counter.WithLabelValues("a").Inc() |
| 312 | + counter.WithLabelValues("a").Inc() |
| 313 | + |
| 314 | + // Gather. |
| 315 | + mfs, err := registry.Gather() |
| 316 | + if err != nil { |
| 317 | + t.Fatalf("Gather failed %v", err) |
| 318 | + } |
| 319 | + if len(mfs) != 1 { |
| 320 | + t.Fatalf("Got %v metric families, Want: 1 metric family", len(mfs)) |
| 321 | + } |
| 322 | + |
| 323 | + // Verify metric type. |
| 324 | + mf := mfs[0] |
| 325 | + var m *dto.Metric |
| 326 | + switch mf.GetType() { |
| 327 | + case dto.MetricType_COUNTER: |
| 328 | + m = mfs[0].GetMetric()[0] |
| 329 | + default: |
| 330 | + t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_COUNTER) |
| 331 | + } |
| 332 | + |
| 333 | + // Verify value. |
| 334 | + want := toAdd + 2 |
| 335 | + got := m.GetCounter().GetValue() |
| 336 | + if got != want { |
| 337 | + t.Fatalf("Got %f, wanted %f as the count", got, want) |
| 338 | + } |
| 339 | + |
| 340 | + // Verify exemplars. |
| 341 | + e := m.GetCounter().GetExemplar() |
| 342 | + if e == nil { |
| 343 | + t.Fatalf("Got nil exemplar, wanted an exemplar") |
| 344 | + } |
| 345 | + eLabels := e.GetLabel() |
| 346 | + if eLabels == nil { |
| 347 | + t.Fatalf("Got nil exemplar label, wanted an exemplar label") |
| 348 | + } |
| 349 | + if len(eLabels) != 2 { |
| 350 | + t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(eLabels)) |
| 351 | + } |
| 352 | + for _, l := range eLabels { |
| 353 | + switch *l.Name { |
| 354 | + case "trace_id": |
| 355 | + if *l.Value != traceID.String() { |
| 356 | + t.Fatalf("Got %s as traceID, wanted %s", *l.Value, traceID.String()) |
| 357 | + } |
| 358 | + case "span_id": |
| 359 | + if *l.Value != spanID.String() { |
| 360 | + t.Fatalf("Got %s as spanID, wanted %s", *l.Value, spanID.String()) |
| 361 | + } |
| 362 | + default: |
| 363 | + t.Fatalf("Got unexpected label %s", *l.Name) |
| 364 | + } |
| 365 | + } |
| 366 | + |
| 367 | + // Verify that all contextual counter calls are exclusive. |
| 368 | + contextualCounterVec := NewCounterVec(&CounterOpts{ |
| 369 | + Name: "contextual_counter", |
| 370 | + Help: "helpless", |
| 371 | + }, []string{"label_a"}) |
| 372 | + spanIDa := trace.SpanID(fn(3)) |
| 373 | + traceIDa := trace.TraceID(fn(4)) |
| 374 | + contextualCounterVecA := contextualCounterVec.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 375 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 376 | + SpanID: spanIDa, |
| 377 | + TraceID: traceIDa, |
| 378 | + TraceFlags: trace.FlagsSampled, |
| 379 | + }), |
| 380 | + )) |
| 381 | + spanIDb := trace.SpanID(fn(5)) |
| 382 | + traceIDb := trace.TraceID(fn(6)) |
| 383 | + contextualCounterVecB := contextualCounterVec.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 384 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 385 | + SpanID: spanIDb, |
| 386 | + TraceID: traceIDb, |
| 387 | + TraceFlags: trace.FlagsSampled, |
| 388 | + }), |
| 389 | + )) |
| 390 | + |
| 391 | + runs := []struct { |
| 392 | + spanID trace.SpanID |
| 393 | + traceID trace.TraceID |
| 394 | + contextualCounter *CounterVecWithContext |
| 395 | + }{ |
| 396 | + { |
| 397 | + spanID: spanIDa, |
| 398 | + traceID: traceIDa, |
| 399 | + contextualCounter: contextualCounterVecA, |
| 400 | + }, |
| 401 | + { |
| 402 | + spanID: spanIDb, |
| 403 | + traceID: traceIDb, |
| 404 | + contextualCounter: contextualCounterVecB, |
| 405 | + }, |
| 406 | + } |
| 407 | + for _, run := range runs { |
| 408 | + registry.MustRegister(run.contextualCounter) |
| 409 | + run.contextualCounter.WithLabelValues("a").Inc() |
| 410 | + |
| 411 | + mfs, err = registry.Gather() |
| 412 | + if err != nil { |
| 413 | + t.Fatalf("Gather failed %v", err) |
| 414 | + } |
| 415 | + if len(mfs) != 2 { |
| 416 | + t.Fatalf("Got %v metric families, Want: 2 metric families", len(mfs)) |
| 417 | + } |
| 418 | + |
| 419 | + dtoMetric := mfs[0].GetMetric()[0] |
| 420 | + if dtoMetric.GetCounter().GetExemplar() == nil { |
| 421 | + t.Fatalf("Got nil exemplar, wanted an exemplar") |
| 422 | + } |
| 423 | + dtoMetricLabels := dtoMetric.GetCounter().GetExemplar().GetLabel() |
| 424 | + if len(dtoMetricLabels) != 2 { |
| 425 | + t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(dtoMetricLabels)) |
| 426 | + } |
| 427 | + for _, l := range dtoMetricLabels { |
| 428 | + switch *l.Name { |
| 429 | + case "trace_id": |
| 430 | + if *l.Value != run.traceID.String() { |
| 431 | + t.Fatalf("Got %s as traceID, wanted %s", *l.Value, run.traceID.String()) |
| 432 | + } |
| 433 | + case "span_id": |
| 434 | + if *l.Value != run.spanID.String() { |
| 435 | + t.Fatalf("Got %s as spanID, wanted %s", *l.Value, run.spanID.String()) |
| 436 | + } |
| 437 | + default: |
| 438 | + t.Fatalf("Got unexpected label %s", *l.Name) |
| 439 | + } |
| 440 | + } |
| 441 | + |
| 442 | + registry.Unregister(run.contextualCounter) |
| 443 | + } |
| 444 | } |
| 445 | diff --git a/metrics/histogram.go b/metrics/histogram.go |
| 446 | index 3065486ab41d1..de9ed858fcfaf 100644 |
| 447 | --- a/metrics/histogram.go |
| 448 | +++ b/metrics/histogram.go |
| 449 | @@ -28,34 +28,38 @@ import ( |
| 450 | // Histogram is our internal representation for our wrapping struct around prometheus |
| 451 | // histograms. Summary implements both kubeCollector and ObserverMetric |
| 452 | type Histogram struct { |
| 453 | - ctx context.Context |
| 454 | ObserverMetric |
| 455 | *HistogramOpts |
| 456 | lazyMetric |
| 457 | selfCollector |
| 458 | } |
| 459 | |
| 460 | -// exemplarHistogramMetric holds a context to extract exemplar labels from, and a historgram metric to attach them to. It implements the metricWithExemplar interface. |
| 461 | -type exemplarHistogramMetric struct { |
| 462 | +// TODO: make this true: var _ Metric = &Histogram{} |
| 463 | + |
| 464 | +// HistogramWithContext implements the metricWithExemplar interface. |
| 465 | +var _ metricWithExemplar = &HistogramWithContext{} |
| 466 | + |
| 467 | +// HistogramWithContext holds a context to extract exemplar labels from, and a historgram metric to attach them to. It implements the metricWithExemplar interface. |
| 468 | +type HistogramWithContext struct { |
| 469 | + ctx context.Context |
| 470 | *Histogram |
| 471 | } |
| 472 | |
| 473 | -type exemplarHistogramVec struct { |
| 474 | +type HistogramVecWithContextWithObserver struct { |
| 475 | *HistogramVecWithContext |
| 476 | - observer prometheus.Observer |
| 477 | + observer ObserverMetric |
| 478 | } |
| 479 | |
| 480 | func (h *Histogram) Observe(v float64) { |
| 481 | - h.withExemplar(v) |
| 482 | + h.ObserverMetric.Observe(v) |
| 483 | } |
| 484 | |
| 485 | -// withExemplar initializes the exemplarMetric object and sets the exemplar value. |
| 486 | -func (h *Histogram) withExemplar(v float64) { |
| 487 | - (&exemplarHistogramMetric{h}).withExemplar(v) |
| 488 | +func (e *HistogramWithContext) Observe(v float64) { |
| 489 | + e.withExemplar(v) |
| 490 | } |
| 491 | |
| 492 | // withExemplar attaches an exemplar to the metric. |
| 493 | -func (e *exemplarHistogramMetric) withExemplar(v float64) { |
| 494 | +func (e *HistogramWithContext) withExemplar(v float64) { |
| 495 | if m, ok := e.Histogram.ObserverMetric.(prometheus.ExemplarObserver); ok { |
| 496 | maybeSpanCtx := trace.SpanContextFromContext(e.ctx) |
| 497 | if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { |
| 498 | @@ -112,9 +116,11 @@ func (h *Histogram) initializeDeprecatedMetric() { |
| 499 | } |
| 500 | |
| 501 | // WithContext allows the normal Histogram metric to pass in context. The context is no-op now. |
| 502 | -func (h *Histogram) WithContext(ctx context.Context) ObserverMetric { |
| 503 | - h.ctx = ctx |
| 504 | - return h.ObserverMetric |
| 505 | +func (h *Histogram) WithContext(ctx context.Context) *HistogramWithContext { |
| 506 | + return &HistogramWithContext{ |
| 507 | + ctx: ctx, |
| 508 | + Histogram: h, |
| 509 | + } |
| 510 | } |
| 511 | |
| 512 | // HistogramVec is the internal representation of our wrapping struct around prometheus |
| 513 | @@ -263,11 +269,11 @@ type HistogramVecWithContext struct { |
| 514 | ctx context.Context |
| 515 | } |
| 516 | |
| 517 | -func (h *exemplarHistogramVec) Observe(v float64) { |
| 518 | +func (h *HistogramVecWithContextWithObserver) Observe(v float64) { |
| 519 | h.withExemplar(v) |
| 520 | } |
| 521 | |
| 522 | -func (h *exemplarHistogramVec) withExemplar(v float64) { |
| 523 | +func (h *HistogramVecWithContextWithObserver) withExemplar(v float64) { |
| 524 | if m, ok := h.observer.(prometheus.ExemplarObserver); ok { |
| 525 | maybeSpanCtx := trace.SpanContextFromContext(h.HistogramVecWithContext.ctx) |
| 526 | if maybeSpanCtx.IsValid() && maybeSpanCtx.IsSampled() { |
| 527 | @@ -283,16 +289,16 @@ func (h *exemplarHistogramVec) withExemplar(v float64) { |
| 528 | } |
| 529 | |
| 530 | // WithLabelValues is the wrapper of HistogramVec.WithLabelValues. |
| 531 | -func (vc *HistogramVecWithContext) WithLabelValues(lvs ...string) *exemplarHistogramVec { |
| 532 | - return &exemplarHistogramVec{ |
| 533 | +func (vc *HistogramVecWithContext) WithLabelValues(lvs ...string) *HistogramVecWithContextWithObserver { |
| 534 | + return &HistogramVecWithContextWithObserver{ |
| 535 | HistogramVecWithContext: vc, |
| 536 | observer: vc.HistogramVec.WithLabelValues(lvs...), |
| 537 | } |
| 538 | } |
| 539 | |
| 540 | // With is the wrapper of HistogramVec.With. |
| 541 | -func (vc *HistogramVecWithContext) With(labels map[string]string) *exemplarHistogramVec { |
| 542 | - return &exemplarHistogramVec{ |
| 543 | +func (vc *HistogramVecWithContext) With(labels map[string]string) *HistogramVecWithContextWithObserver { |
| 544 | + return &HistogramVecWithContextWithObserver{ |
| 545 | HistogramVecWithContext: vc, |
| 546 | observer: vc.HistogramVec.With(labels), |
| 547 | } |
| 548 | diff --git a/metrics/histogram_test.go b/metrics/histogram_test.go |
| 549 | index 5efbfb6eeae2d..3cf4e703912ea 100644 |
| 550 | --- a/metrics/histogram_test.go |
| 551 | +++ b/metrics/histogram_test.go |
| 552 | @@ -318,7 +318,7 @@ func TestHistogramWithLabelValueAllowList(t *testing.T) { |
| 553 | } |
| 554 | |
| 555 | func TestHistogramWithExemplar(t *testing.T) { |
| 556 | - // Arrange. |
| 557 | + // Create context. |
| 558 | traceID := trace.TraceID([]byte("trace-0000-xxxxx")) |
| 559 | spanID := trace.SpanID([]byte("span-0000-xxxxx")) |
| 560 | ctxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ |
| 561 | @@ -328,13 +328,14 @@ func TestHistogramWithExemplar(t *testing.T) { |
| 562 | })) |
| 563 | value := float64(10) |
| 564 | |
| 565 | + // Create contextual histogram. |
| 566 | histogram := NewHistogram(&HistogramOpts{ |
| 567 | Name: "histogram_exemplar_test", |
| 568 | Help: "helpless", |
| 569 | Buckets: []float64{100}, |
| 570 | - }) |
| 571 | - _ = histogram.WithContext(ctxForSpanCtx) |
| 572 | + }).WithContext(ctxForSpanCtx) |
| 573 | |
| 574 | + // Register histogram. |
| 575 | registry := newKubeRegistry(apimachineryversion.Info{ |
| 576 | Major: "1", |
| 577 | Minor: "15", |
| 578 | @@ -342,53 +343,51 @@ func TestHistogramWithExemplar(t *testing.T) { |
| 579 | }) |
| 580 | registry.MustRegister(histogram) |
| 581 | |
| 582 | - // Act. |
| 583 | + // Call underlying exemplar methods. |
| 584 | histogram.Observe(value) |
| 585 | |
| 586 | - // Assert. |
| 587 | + // Gather. |
| 588 | mfs, err := registry.Gather() |
| 589 | if err != nil { |
| 590 | t.Fatalf("Gather failed %v", err) |
| 591 | } |
| 592 | - |
| 593 | if len(mfs) != 1 { |
| 594 | t.Fatalf("Got %v metric families, Want: 1 metric family", len(mfs)) |
| 595 | } |
| 596 | |
| 597 | + // Verify metric type. |
| 598 | mf := mfs[0] |
| 599 | var m *dto.Metric |
| 600 | switch mf.GetType() { |
| 601 | case dto.MetricType_HISTOGRAM: |
| 602 | m = mfs[0].GetMetric()[0] |
| 603 | default: |
| 604 | - t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_COUNTER) |
| 605 | + t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_HISTOGRAM) |
| 606 | } |
| 607 | |
| 608 | + // Verify value. |
| 609 | want := value |
| 610 | got := m.GetHistogram().GetSampleSum() |
| 611 | if got != want { |
| 612 | t.Fatalf("Got %f, wanted %f as the count", got, want) |
| 613 | } |
| 614 | |
| 615 | + // Verify exemplars. |
| 616 | buckets := m.GetHistogram().GetBucket() |
| 617 | if len(buckets) == 0 { |
| 618 | t.Fatalf("Got 0 buckets, wanted 1") |
| 619 | } |
| 620 | - |
| 621 | e := buckets[0].GetExemplar() |
| 622 | if e == nil { |
| 623 | t.Fatalf("Got nil exemplar, wanted an exemplar") |
| 624 | } |
| 625 | - |
| 626 | eLabels := e.GetLabel() |
| 627 | if eLabels == nil { |
| 628 | t.Fatalf("Got nil exemplar label, wanted an exemplar label") |
| 629 | } |
| 630 | - |
| 631 | if len(eLabels) != 2 { |
| 632 | t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(eLabels)) |
| 633 | } |
| 634 | - |
| 635 | for _, l := range eLabels { |
| 636 | switch *l.Name { |
| 637 | case "trace_id": |
| 638 | @@ -403,10 +402,95 @@ func TestHistogramWithExemplar(t *testing.T) { |
| 639 | t.Fatalf("Got unexpected label %s", *l.Name) |
| 640 | } |
| 641 | } |
| 642 | + |
| 643 | + // Verify that all contextual histogram calls are exclusive. |
| 644 | + contextualHistogram := NewHistogram(&HistogramOpts{ |
| 645 | + Name: "contextual_histogram", |
| 646 | + Help: "helpless", |
| 647 | + Buckets: []float64{100}, |
| 648 | + }) |
| 649 | + traceIDa := trace.TraceID([]byte("trace-0000-aaaaa")) |
| 650 | + spanIDa := trace.SpanID([]byte("span-0000-aaaaa")) |
| 651 | + contextualHistogramA := contextualHistogram.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 652 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 653 | + TraceID: traceIDa, |
| 654 | + SpanID: spanIDa, |
| 655 | + TraceFlags: trace.FlagsSampled, |
| 656 | + }), |
| 657 | + )) |
| 658 | + traceIDb := trace.TraceID([]byte("trace-0000-bbbbb")) |
| 659 | + spanIDb := trace.SpanID([]byte("span-0000-bbbbb")) |
| 660 | + contextualHistogramB := contextualHistogram.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 661 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 662 | + TraceID: traceIDb, |
| 663 | + SpanID: spanIDb, |
| 664 | + TraceFlags: trace.FlagsSampled, |
| 665 | + }), |
| 666 | + )) |
| 667 | + |
| 668 | + runs := []struct { |
| 669 | + spanID trace.SpanID |
| 670 | + traceID trace.TraceID |
| 671 | + contextualHistogram *HistogramWithContext |
| 672 | + }{ |
| 673 | + { |
| 674 | + spanID: spanIDa, |
| 675 | + traceID: traceIDa, |
| 676 | + contextualHistogram: contextualHistogramA, |
| 677 | + }, |
| 678 | + { |
| 679 | + spanID: spanIDb, |
| 680 | + traceID: traceIDb, |
| 681 | + contextualHistogram: contextualHistogramB, |
| 682 | + }, |
| 683 | + } |
| 684 | + for _, run := range runs { |
| 685 | + registry.MustRegister(run.contextualHistogram) |
| 686 | + run.contextualHistogram.Observe(value) |
| 687 | + |
| 688 | + mfs, err = registry.Gather() |
| 689 | + if err != nil { |
| 690 | + t.Fatalf("Gather failed %v", err) |
| 691 | + } |
| 692 | + if len(mfs) != 2 { |
| 693 | + t.Fatalf("Got %v metric families, Want: 2 metric families", len(mfs)) |
| 694 | + } |
| 695 | + |
| 696 | + dtoMetric := mfs[0].GetMetric()[0] |
| 697 | + dtoMetricBuckets := dtoMetric.GetHistogram().GetBucket() |
| 698 | + if len(dtoMetricBuckets) == 0 { |
| 699 | + t.Fatalf("Got nil buckets") |
| 700 | + } |
| 701 | + dtoMetricBucketsExemplar := dtoMetricBuckets[0].GetExemplar() |
| 702 | + if dtoMetricBucketsExemplar == nil { |
| 703 | + t.Fatalf("Got nil exemplar") |
| 704 | + } |
| 705 | + |
| 706 | + dtoMetricLabels := dtoMetricBucketsExemplar.GetLabel() |
| 707 | + if len(dtoMetricLabels) != 2 { |
| 708 | + t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(dtoMetricLabels)) |
| 709 | + } |
| 710 | + for _, l := range dtoMetricLabels { |
| 711 | + switch *l.Name { |
| 712 | + case "trace_id": |
| 713 | + if *l.Value != run.traceID.String() { |
| 714 | + t.Fatalf("Got %s as traceID, wanted %s", *l.Value, run.traceID.String()) |
| 715 | + } |
| 716 | + case "span_id": |
| 717 | + if *l.Value != run.spanID.String() { |
| 718 | + t.Fatalf("Got %s as spanID, wanted %s", *l.Value, run.spanID.String()) |
| 719 | + } |
| 720 | + default: |
| 721 | + t.Fatalf("Got unexpected label %s", *l.Name) |
| 722 | + } |
| 723 | + } |
| 724 | + |
| 725 | + registry.Unregister(run.contextualHistogram) |
| 726 | + } |
| 727 | } |
| 728 | |
| 729 | func TestHistogramVecWithExemplar(t *testing.T) { |
| 730 | - // Arrange. |
| 731 | + // Create context. |
| 732 | traceID := trace.TraceID([]byte("trace-0000-xxxxx")) |
| 733 | spanID := trace.SpanID([]byte("span-0000-xxxxx")) |
| 734 | ctxForSpanCtx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ |
| 735 | @@ -416,6 +500,7 @@ func TestHistogramVecWithExemplar(t *testing.T) { |
| 736 | })) |
| 737 | value := float64(10) |
| 738 | |
| 739 | + // Create contextual histogram. |
| 740 | histogramVec := NewHistogramVec(&HistogramOpts{ |
| 741 | Name: "histogram_exemplar_test", |
| 742 | Help: "helpless", |
| 743 | @@ -423,6 +508,7 @@ func TestHistogramVecWithExemplar(t *testing.T) { |
| 744 | }, []string{"group"}) |
| 745 | h := histogramVec.WithContext(ctxForSpanCtx) |
| 746 | |
| 747 | + // Register histogram. |
| 748 | registry := newKubeRegistry(apimachineryversion.Info{ |
| 749 | Major: "1", |
| 750 | Minor: "15", |
| 751 | @@ -430,53 +516,51 @@ func TestHistogramVecWithExemplar(t *testing.T) { |
| 752 | }) |
| 753 | registry.MustRegister(histogramVec) |
| 754 | |
| 755 | - // Act. |
| 756 | + // Call underlying exemplar methods. |
| 757 | h.WithLabelValues("foo").Observe(value) |
| 758 | |
| 759 | - // Assert. |
| 760 | + // Gather. |
| 761 | mfs, err := registry.Gather() |
| 762 | if err != nil { |
| 763 | t.Fatalf("Gather failed %v", err) |
| 764 | } |
| 765 | - |
| 766 | if len(mfs) != 1 { |
| 767 | t.Fatalf("Got %v metric families, Want: 1 metric family", len(mfs)) |
| 768 | } |
| 769 | |
| 770 | + // Verify metric type. |
| 771 | mf := mfs[0] |
| 772 | var m *dto.Metric |
| 773 | switch mf.GetType() { |
| 774 | case dto.MetricType_HISTOGRAM: |
| 775 | m = mfs[0].GetMetric()[0] |
| 776 | default: |
| 777 | - t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_COUNTER) |
| 778 | + t.Fatalf("Got %v metric type, Want: %v metric type", mf.GetType(), dto.MetricType_HISTOGRAM) |
| 779 | } |
| 780 | |
| 781 | + // Verify value. |
| 782 | want := value |
| 783 | got := m.GetHistogram().GetSampleSum() |
| 784 | if got != want { |
| 785 | t.Fatalf("Got %f, wanted %f as the count", got, want) |
| 786 | } |
| 787 | |
| 788 | + // Verify exemplars. |
| 789 | buckets := m.GetHistogram().GetBucket() |
| 790 | if len(buckets) == 0 { |
| 791 | t.Fatalf("Got 0 buckets, wanted 1") |
| 792 | } |
| 793 | - |
| 794 | e := buckets[0].GetExemplar() |
| 795 | if e == nil { |
| 796 | t.Fatalf("Got nil exemplar, wanted an exemplar") |
| 797 | } |
| 798 | - |
| 799 | eLabels := e.GetLabel() |
| 800 | if eLabels == nil { |
| 801 | t.Fatalf("Got nil exemplar label, wanted an exemplar label") |
| 802 | } |
| 803 | - |
| 804 | if len(eLabels) != 2 { |
| 805 | t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(eLabels)) |
| 806 | } |
| 807 | - |
| 808 | for _, l := range eLabels { |
| 809 | switch *l.Name { |
| 810 | case "trace_id": |
| 811 | @@ -491,4 +575,87 @@ func TestHistogramVecWithExemplar(t *testing.T) { |
| 812 | t.Fatalf("Got unexpected label %s", *l.Name) |
| 813 | } |
| 814 | } |
| 815 | + |
| 816 | + // Verify that all contextual histogram calls are exclusive. |
| 817 | + contextualHistogramVec := NewHistogramVec(&HistogramOpts{ |
| 818 | + Name: "contextual_histogram_vec", |
| 819 | + Help: "helpless", |
| 820 | + Buckets: []float64{100}, |
| 821 | + }, []string{"group"}) |
| 822 | + traceIDa := trace.TraceID([]byte("trace-0000-aaaaa")) |
| 823 | + spanIDa := trace.SpanID([]byte("span-0000-aaaaa")) |
| 824 | + contextualHistogramVecA := contextualHistogramVec.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 825 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 826 | + TraceID: traceIDa, |
| 827 | + SpanID: spanIDa, |
| 828 | + TraceFlags: trace.FlagsSampled, |
| 829 | + }), |
| 830 | + )) |
| 831 | + traceIDb := trace.TraceID([]byte("trace-0000-bbbbb")) |
| 832 | + spanIDb := trace.SpanID([]byte("span-0000-bbbbb")) |
| 833 | + contextualHistogramVecB := contextualHistogramVec.WithContext(trace.ContextWithSpanContext(context.Background(), |
| 834 | + trace.NewSpanContext(trace.SpanContextConfig{ |
| 835 | + TraceID: traceIDb, |
| 836 | + SpanID: spanIDb, |
| 837 | + TraceFlags: trace.FlagsSampled, |
| 838 | + }), |
| 839 | + )) |
| 840 | + runs := []struct { |
| 841 | + spanID trace.SpanID |
| 842 | + traceID trace.TraceID |
| 843 | + contextualHistogramVec *HistogramVecWithContext |
| 844 | + }{ |
| 845 | + { |
| 846 | + spanID: spanIDa, |
| 847 | + traceID: traceIDa, |
| 848 | + contextualHistogramVec: contextualHistogramVecA, |
| 849 | + }, { |
| 850 | + spanID: spanIDb, |
| 851 | + traceID: traceIDb, |
| 852 | + contextualHistogramVec: contextualHistogramVecB, |
| 853 | + }, |
| 854 | + } |
| 855 | + for _, run := range runs { |
| 856 | + registry.MustRegister(run.contextualHistogramVec) |
| 857 | + run.contextualHistogramVec.WithLabelValues("foo").Observe(value) |
| 858 | + |
| 859 | + mfs, err = registry.Gather() |
| 860 | + if err != nil { |
| 861 | + t.Fatalf("Gather failed %v", err) |
| 862 | + } |
| 863 | + if len(mfs) != 2 { |
| 864 | + t.Fatalf("Got %v metric families, Want: 2 metric families", len(mfs)) |
| 865 | + } |
| 866 | + |
| 867 | + dtoMetric := mfs[0].GetMetric()[0] |
| 868 | + dtoMetricBuckets := dtoMetric.GetHistogram().GetBucket() |
| 869 | + if len(dtoMetricBuckets) == 0 { |
| 870 | + t.Fatalf("Got nil buckets") |
| 871 | + } |
| 872 | + dtoMetricBucketsExemplar := dtoMetricBuckets[0].GetExemplar() |
| 873 | + if dtoMetricBucketsExemplar == nil { |
| 874 | + t.Fatalf("Got nil exemplar") |
| 875 | + } |
| 876 | + |
| 877 | + dtoMetricLabels := dtoMetricBucketsExemplar.GetLabel() |
| 878 | + if len(dtoMetricLabels) != 2 { |
| 879 | + t.Fatalf("Got %v exemplar labels, wanted 2 exemplar labels", len(dtoMetricLabels)) |
| 880 | + } |
| 881 | + for _, l := range dtoMetricLabels { |
| 882 | + switch *l.Name { |
| 883 | + case "trace_id": |
| 884 | + if *l.Value != run.traceID.String() { |
| 885 | + t.Fatalf("Got %s as traceID, wanted %s", *l.Value, run.traceID.String()) |
| 886 | + } |
| 887 | + case "span_id": |
| 888 | + if *l.Value != run.spanID.String() { |
| 889 | + t.Fatalf("Got %s as spanID, wanted %s", *l.Value, run.spanID.String()) |
| 890 | + } |
| 891 | + default: |
| 892 | + t.Fatalf("Got unexpected label %s", *l.Name) |
| 893 | + } |
| 894 | + } |
| 895 | + |
| 896 | + registry.Unregister(run.contextualHistogramVec) |
| 897 | + } |
| 898 | } |