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{