From eb1ce2ca3719c99170c57110533dc33f390d3dcc Mon Sep 17 00:00:00 2001 From: Sparshal Kothari <41056517+spor3006@users.noreply.github.com> Date: Wed, 13 May 2026 23:07:55 +0530 Subject: [PATCH] promhttp: don't panic when instrumenting with non-exemplar observers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- CHANGELOG.md | 1 + prometheus/promhttp/instrument_server.go | 38 +++++--- prometheus/promhttp/instrument_server_test.go | 93 +++++++++++++++++++ 3 files changed, 119 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b029c6e9..499a87f37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Unreleased * [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 +* [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 ## Unreleased `exp` module diff --git a/prometheus/promhttp/instrument_server.go b/prometheus/promhttp/instrument_server.go index c837c2767..9dec091ac 100644 --- a/prometheus/promhttp/instrument_server.go +++ b/prometheus/promhttp/instrument_server.go @@ -28,24 +28,36 @@ import ( // magicString is used for the hacky label test in checkLabels. Remove once fixed. const magicString = "zZgWfBxLqvG8kc8IMv3POi2Bb0tZI3vAnBx+gBaFi9FyPzB/CzKUer1yufDa" -// observeWithExemplar is a wrapper for [prometheus.ExemplarAdder.ExemplarObserver], -// which falls back to [prometheus.Observer.Observe] if no labels are provided. +// observeWithExemplar records val on obs. If labels is non-nil and obs +// implements [prometheus.ExemplarObserver], the exemplar is attached via +// ObserveWithExemplar; otherwise the exemplar is dropped and the value is +// recorded with a plain [prometheus.Observer.Observe]. This mirrors the +// safe-cast pattern in [prometheus.Timer.ObserveDurationWithExemplar] and +// ensures we never panic when callers pass an ObserverVec backed by a +// summary, which cannot carry exemplars in the Prometheus exposition format. func observeWithExemplar(obs prometheus.Observer, val float64, labels map[string]string) { - if labels == nil { - obs.Observe(val) - return + if labels != nil { + if eo, ok := obs.(prometheus.ExemplarObserver); ok { + eo.ObserveWithExemplar(val, labels) + return + } } - obs.(prometheus.ExemplarObserver).ObserveWithExemplar(val, labels) + obs.Observe(val) } -// addWithExemplar is a wrapper for [prometheus.ExemplarAdder.AddWithExemplar], -// which falls back to [prometheus.Counter.Add] if no labels are provided. -func addWithExemplar(obs prometheus.Counter, val float64, labels map[string]string) { - if labels == nil { - obs.Add(val) - return +// addWithExemplar records val on c. If labels is non-nil and c implements +// [prometheus.ExemplarAdder], the exemplar is attached via AddWithExemplar; +// otherwise the exemplar is dropped and the value is recorded with a plain +// [prometheus.Counter.Add]. The safe-cast keeps the helper robust against +// custom Counter implementations that do not advertise exemplar support. +func addWithExemplar(c prometheus.Counter, val float64, labels map[string]string) { + if labels != nil { + if ea, ok := c.(prometheus.ExemplarAdder); ok { + ea.AddWithExemplar(val, labels) + return + } } - obs.(prometheus.ExemplarAdder).AddWithExemplar(val, labels) + c.Add(val) } // InstrumentHandlerInFlight is a middleware that wraps the provided diff --git a/prometheus/promhttp/instrument_server_test.go b/prometheus/promhttp/instrument_server_test.go index e769cb9ba..0ad457247 100644 --- a/prometheus/promhttp/instrument_server_test.go +++ b/prometheus/promhttp/instrument_server_test.go @@ -21,6 +21,8 @@ import ( "net/http/httptest" "testing" + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/client_golang/prometheus" ) @@ -439,6 +441,97 @@ func TestMiddlewareAPI_WithExemplars(t *testing.T) { assetMetricAndExemplars(t, reg, 5, labelsToLabelPair(exemplar)) } +// TestMiddlewareAPI_SummaryWithExemplars is a regression test for +// https://github.com/prometheus/client_golang/issues/1258. SummaryVec is a +// valid prometheus.ObserverVec but the underlying summary does not implement +// prometheus.ExemplarObserver — only histograms can carry exemplars in the +// Prometheus exposition format. The instrumentation helpers must therefore +// fall back to a plain Observe when given a non-ExemplarObserver, instead of +// panicking with a failed type assertion at request time. +func TestMiddlewareAPI_SummaryWithExemplars(t *testing.T) { + reg := prometheus.NewRegistry() + + durationVec := prometheus.NewSummaryVec( + prometheus.SummaryOpts{ + Name: "request_duration_seconds", + Help: "A summary of request durations.", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, + }, + []string{"code", "method"}, + ) + reg.MustRegister(durationVec) + + exemplar := prometheus.Labels{"traceID": "abc123"} + handler := InstrumentHandlerDuration( + durationVec, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("OK")) + }), + WithExemplarFromContext(func(_ context.Context) prometheus.Labels { return exemplar }), + ) + + r, _ := http.NewRequest(http.MethodGet, "www.example.com", nil) + w := httptest.NewRecorder() + + defer func() { + if rec := recover(); rec != nil { + t.Fatalf("InstrumentHandlerDuration panicked with a SummaryVec observer: %v", rec) + } + }() + handler.ServeHTTP(w, r) +} + +// nonExemplarObserver implements prometheus.Observer but deliberately omits +// ObserveWithExemplar, so it does not satisfy prometheus.ExemplarObserver. +type nonExemplarObserver struct { + last float64 +} + +func (o *nonExemplarObserver) Observe(v float64) { o.last = v } + +// nonExemplarCounter implements prometheus.Counter but deliberately omits +// AddWithExemplar, so it does not satisfy prometheus.ExemplarAdder. +type nonExemplarCounter struct { + last float64 +} + +func (c *nonExemplarCounter) Desc() *prometheus.Desc { return nil } +func (c *nonExemplarCounter) Write(*dto.Metric) error { return nil } +func (c *nonExemplarCounter) Describe(chan<- *prometheus.Desc) {} +func (c *nonExemplarCounter) Collect(chan<- prometheus.Metric) {} +func (c *nonExemplarCounter) Inc() { c.last = 1 } +func (c *nonExemplarCounter) Add(v float64) { c.last = v } + +func TestObserveWithExemplar_NonExemplarObserverFallsBack(t *testing.T) { + obs := &nonExemplarObserver{} + + defer func() { + if rec := recover(); rec != nil { + t.Fatalf("observeWithExemplar panicked for non-ExemplarObserver: %v", rec) + } + }() + observeWithExemplar(obs, 1.5, prometheus.Labels{"traceID": "abc"}) + + if obs.last != 1.5 { + t.Fatalf("expected fallback Observe(1.5), got Observe(%v)", obs.last) + } +} + +func TestAddWithExemplar_NonExemplarAdderFallsBack(t *testing.T) { + c := &nonExemplarCounter{} + + defer func() { + if rec := recover(); rec != nil { + t.Fatalf("addWithExemplar panicked for non-ExemplarAdder: %v", rec) + } + }() + addWithExemplar(c, 2.5, prometheus.Labels{"traceID": "abc"}) + + if c.last != 2.5 { + t.Fatalf("expected fallback Add(2.5), got Add(%v)", c.last) + } +} + func TestInstrumentTimeToFirstWrite(t *testing.T) { var i int dobs := &responseWriterDelegator{