Skip to content

Commit e0b7a44

Browse files
authored
promhttp: don't panic when instrumenting with non-exemplar observers (#2005)
InstrumentHandlerDuration and InstrumentHandlerCounter unconditionally type-asserted their observer/counter to ExemplarObserver / ExemplarAdder when an exemplar was provided. This panicked at request time if the caller passed a SummaryVec — a valid prometheus.ObserverVec whose underlying *summary does not implement ObserveWithExemplar, because summaries cannot carry exemplars in the Prometheus exposition format. Switch to the safe-cast + fallback pattern already used by Timer.ObserveDurationWithExemplar (prometheus/timer.go): if the observer/counter does not implement the exemplar interface, drop the exemplar and record the value with the plain Observe/Add path. No public API change. Adds three regression tests: - TestMiddlewareAPI_SummaryWithExemplars: exact panic reproduction from the upstream issue, through InstrumentHandlerDuration. - TestObserveWithExemplar_NonExemplarObserverFallsBack: unit-level contract for the helper. - TestAddWithExemplar_NonExemplarAdderFallsBack: same, for the counter helper. Fixes #1258 Signed-off-by: Sparshal Kothari <41056517+spor3006@users.noreply.github.com>
1 parent 2c6e6c1 commit e0b7a44

3 files changed

Lines changed: 119 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## Unreleased
22

33
* [FEATURE] HTTP handlers created by `promhttp` package now support metrics filtering by providing one or more `name[]` query parameters. The default behavior when none are provided remains the same, returning all metrics. #1925
4+
* [BUGFIX] promhttp: `InstrumentHandlerDuration` and `InstrumentHandlerCounter` no longer panic when given an observer/counter that does not implement `ExemplarObserver`/`ExemplarAdder` (e.g. a `SummaryVec`). The exemplar is dropped and the value is recorded via the plain `Observe`/`Add` path, matching the safe-cast already used by `Timer.ObserveDurationWithExemplar`. #2005
45

56
## Unreleased `exp` module
67

prometheus/promhttp/instrument_server.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,36 @@ import (
2828
// magicString is used for the hacky label test in checkLabels. Remove once fixed.
2929
const magicString = "zZgWfBxLqvG8kc8IMv3POi2Bb0tZI3vAnBx+gBaFi9FyPzB/CzKUer1yufDa"
3030

31-
// observeWithExemplar is a wrapper for [prometheus.ExemplarAdder.ExemplarObserver],
32-
// which falls back to [prometheus.Observer.Observe] if no labels are provided.
31+
// observeWithExemplar records val on obs. If labels is non-nil and obs
32+
// implements [prometheus.ExemplarObserver], the exemplar is attached via
33+
// ObserveWithExemplar; otherwise the exemplar is dropped and the value is
34+
// recorded with a plain [prometheus.Observer.Observe]. This mirrors the
35+
// safe-cast pattern in [prometheus.Timer.ObserveDurationWithExemplar] and
36+
// ensures we never panic when callers pass an ObserverVec backed by a
37+
// summary, which cannot carry exemplars in the Prometheus exposition format.
3338
func observeWithExemplar(obs prometheus.Observer, val float64, labels map[string]string) {
34-
if labels == nil {
35-
obs.Observe(val)
36-
return
39+
if labels != nil {
40+
if eo, ok := obs.(prometheus.ExemplarObserver); ok {
41+
eo.ObserveWithExemplar(val, labels)
42+
return
43+
}
3744
}
38-
obs.(prometheus.ExemplarObserver).ObserveWithExemplar(val, labels)
45+
obs.Observe(val)
3946
}
4047

41-
// addWithExemplar is a wrapper for [prometheus.ExemplarAdder.AddWithExemplar],
42-
// which falls back to [prometheus.Counter.Add] if no labels are provided.
43-
func addWithExemplar(obs prometheus.Counter, val float64, labels map[string]string) {
44-
if labels == nil {
45-
obs.Add(val)
46-
return
48+
// addWithExemplar records val on c. If labels is non-nil and c implements
49+
// [prometheus.ExemplarAdder], the exemplar is attached via AddWithExemplar;
50+
// otherwise the exemplar is dropped and the value is recorded with a plain
51+
// [prometheus.Counter.Add]. The safe-cast keeps the helper robust against
52+
// custom Counter implementations that do not advertise exemplar support.
53+
func addWithExemplar(c prometheus.Counter, val float64, labels map[string]string) {
54+
if labels != nil {
55+
if ea, ok := c.(prometheus.ExemplarAdder); ok {
56+
ea.AddWithExemplar(val, labels)
57+
return
58+
}
4759
}
48-
obs.(prometheus.ExemplarAdder).AddWithExemplar(val, labels)
60+
c.Add(val)
4961
}
5062

5163
// InstrumentHandlerInFlight is a middleware that wraps the provided

prometheus/promhttp/instrument_server_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
"sync"
2323
"testing"
2424

25+
dto "github.com/prometheus/client_model/go"
26+
2527
"github.com/prometheus/client_golang/prometheus"
2628
)
2729

@@ -440,6 +442,97 @@ func TestMiddlewareAPI_WithExemplars(t *testing.T) {
440442
assetMetricAndExemplars(t, reg, 5, labelsToLabelPair(exemplar))
441443
}
442444

445+
// TestMiddlewareAPI_SummaryWithExemplars is a regression test for
446+
// https://github.com/prometheus/client_golang/issues/1258. SummaryVec is a
447+
// valid prometheus.ObserverVec but the underlying summary does not implement
448+
// prometheus.ExemplarObserver — only histograms can carry exemplars in the
449+
// Prometheus exposition format. The instrumentation helpers must therefore
450+
// fall back to a plain Observe when given a non-ExemplarObserver, instead of
451+
// panicking with a failed type assertion at request time.
452+
func TestMiddlewareAPI_SummaryWithExemplars(t *testing.T) {
453+
reg := prometheus.NewRegistry()
454+
455+
durationVec := prometheus.NewSummaryVec(
456+
prometheus.SummaryOpts{
457+
Name: "request_duration_seconds",
458+
Help: "A summary of request durations.",
459+
Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001},
460+
},
461+
[]string{"code", "method"},
462+
)
463+
reg.MustRegister(durationVec)
464+
465+
exemplar := prometheus.Labels{"traceID": "abc123"}
466+
handler := InstrumentHandlerDuration(
467+
durationVec,
468+
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
469+
_, _ = w.Write([]byte("OK"))
470+
}),
471+
WithExemplarFromContext(func(_ context.Context) prometheus.Labels { return exemplar }),
472+
)
473+
474+
r, _ := http.NewRequest(http.MethodGet, "www.example.com", nil)
475+
w := httptest.NewRecorder()
476+
477+
defer func() {
478+
if rec := recover(); rec != nil {
479+
t.Fatalf("InstrumentHandlerDuration panicked with a SummaryVec observer: %v", rec)
480+
}
481+
}()
482+
handler.ServeHTTP(w, r)
483+
}
484+
485+
// nonExemplarObserver implements prometheus.Observer but deliberately omits
486+
// ObserveWithExemplar, so it does not satisfy prometheus.ExemplarObserver.
487+
type nonExemplarObserver struct {
488+
last float64
489+
}
490+
491+
func (o *nonExemplarObserver) Observe(v float64) { o.last = v }
492+
493+
// nonExemplarCounter implements prometheus.Counter but deliberately omits
494+
// AddWithExemplar, so it does not satisfy prometheus.ExemplarAdder.
495+
type nonExemplarCounter struct {
496+
last float64
497+
}
498+
499+
func (c *nonExemplarCounter) Desc() *prometheus.Desc { return nil }
500+
func (c *nonExemplarCounter) Write(*dto.Metric) error { return nil }
501+
func (c *nonExemplarCounter) Describe(chan<- *prometheus.Desc) {}
502+
func (c *nonExemplarCounter) Collect(chan<- prometheus.Metric) {}
503+
func (c *nonExemplarCounter) Inc() { c.last = 1 }
504+
func (c *nonExemplarCounter) Add(v float64) { c.last = v }
505+
506+
func TestObserveWithExemplar_NonExemplarObserverFallsBack(t *testing.T) {
507+
obs := &nonExemplarObserver{}
508+
509+
defer func() {
510+
if rec := recover(); rec != nil {
511+
t.Fatalf("observeWithExemplar panicked for non-ExemplarObserver: %v", rec)
512+
}
513+
}()
514+
observeWithExemplar(obs, 1.5, prometheus.Labels{"traceID": "abc"})
515+
516+
if obs.last != 1.5 {
517+
t.Fatalf("expected fallback Observe(1.5), got Observe(%v)", obs.last)
518+
}
519+
}
520+
521+
func TestAddWithExemplar_NonExemplarAdderFallsBack(t *testing.T) {
522+
c := &nonExemplarCounter{}
523+
524+
defer func() {
525+
if rec := recover(); rec != nil {
526+
t.Fatalf("addWithExemplar panicked for non-ExemplarAdder: %v", rec)
527+
}
528+
}()
529+
addWithExemplar(c, 2.5, prometheus.Labels{"traceID": "abc"})
530+
531+
if c.last != 2.5 {
532+
t.Fatalf("expected fallback Add(2.5), got Add(%v)", c.last)
533+
}
534+
}
535+
443536
func TestInstrumentTimeToFirstWrite(t *testing.T) {
444537
var i int
445538
dobs := &responseWriterDelegator{

0 commit comments

Comments
 (0)