diff --git a/metrics/handler.go b/metrics/handler.go index 5af354330cc24efc03ae91ce8d646e34e7131021..769d63fc60f4afe99f2405414d8815ec8f773feb 100644 --- a/metrics/handler.go +++ b/metrics/handler.go @@ -92,12 +92,14 @@ func NewHandlerFactory(opts ...HandlerFactoryOption) HandlerFactory { ) ) - prometheus.MustRegister(httpInFlightRequests) - prometheus.MustRegister(httpRequestsTotal) - prometheus.MustRegister(httpRequestDurationSeconds) - prometheus.MustRegister(httpRequestSizeBytes) - prometheus.MustRegister(httpResponseSizeBytes) - prometheus.MustRegister(httpTimeToWriteHeaderSeconds) + factoryConfig.registerer.MustRegister( + httpInFlightRequests, + httpRequestsTotal, + httpRequestDurationSeconds, + httpRequestSizeBytes, + httpResponseSizeBytes, + httpTimeToWriteHeaderSeconds, + ) return func(next http.Handler, handlerOpts ...HandlerOption) http.Handler { handlerConfig, promOpts := applyHandlerOptions(handlerOpts) diff --git a/metrics/handler_factory_options.go b/metrics/handler_factory_options.go index 726d272d8a379ae3d5ed0d4b8945acaed7f7e7cc..1de2d49f1c9bd7d14757fc7c30b0eb18c3d5429a 100644 --- a/metrics/handler_factory_options.go +++ b/metrics/handler_factory_options.go @@ -1,6 +1,9 @@ package metrics +import "github.com/prometheus/client_golang/prometheus" + type handlerFactoryConfig struct { + registerer prometheus.Registerer namespace string subsystem string requestDurationBuckets []float64 @@ -14,7 +17,8 @@ type HandlerFactoryOption func(*handlerFactoryConfig) func applyHandlerFactoryOptions(opts []HandlerFactoryOption) handlerFactoryConfig { config := handlerFactoryConfig{ - subsystem: "http", + registerer: prometheus.DefaultRegisterer, + subsystem: "http", requestDurationBuckets: []float64{ 0.005, /* 5ms */ 0.025, /* 25ms */ @@ -91,3 +95,12 @@ func WithByteSizeBuckets(buckets []float64) HandlerFactoryOption { config.byteSizeBuckets = buckets } } + +// WithRegisterer configures the Registerer the metrics will be registered with. +// The metrics are by default registered into prometheus.DefaultRegisterer if +// this option is not provided. +func WithRegisterer(registerer prometheus.Registerer) HandlerFactoryOption { + return func(config *handlerFactoryConfig) { + config.registerer = registerer + } +} diff --git a/metrics/handler_test.go b/metrics/handler_test.go index baaea111ea52fafe7b69dd7f8515a552199d87a5..58ff0a4f6c27feea33171b05535ab72d04254b8e 100644 --- a/metrics/handler_test.go +++ b/metrics/handler_test.go @@ -15,23 +15,34 @@ import ( func TestNewHandlerFactory(t *testing.T) { var dynamicLabelKey struct{} - tests := []struct { + type TestCase struct { name string opts []HandlerFactoryOption // factory-level options handlerOpts []HandlerOption // handler-level options injectCtx func(r *http.Request) *http.Request // optional context injection wantLabels map[string]string // custom label key/values - }{ + namespace string + registry *prometheus.Registry + } + + tests := []TestCase{ { name: "basic", - opts: []HandlerFactoryOption{ - WithNamespace("basic"), - }, }, + func() TestCase { + const namespace = "custom_prefix" + return TestCase{ + name: "custom namespace", + opts: []HandlerFactoryOption{WithNamespace(namespace)}, + // Prometheus separates the namespace from the metric subsystem and name + // with an underscore. Add it here as well so the assertion correctly + // accounts for it. + namespace: namespace + "_", + } + }(), { name: "labels", opts: []HandlerFactoryOption{ - WithNamespace("labels"), WithLabels("label1", "label2"), }, handlerOpts: []HandlerOption{ @@ -42,7 +53,6 @@ func TestNewHandlerFactory(t *testing.T) { { name: "dynamic_labels", opts: []HandlerFactoryOption{ - WithNamespace("dynamic_labels"), WithLabels("color"), }, handlerOpts: []HandlerOption{ @@ -63,28 +73,48 @@ func TestNewHandlerFactory(t *testing.T) { { name: "request_duration_buckets", opts: []HandlerFactoryOption{ - WithNamespace("request_duration_buckets"), WithRequestDurationBuckets([]float64{1, 2, 3}), }, }, { name: "time_to_write_header_duration_buckets", opts: []HandlerFactoryOption{ - WithNamespace("time_to_write_header_duration_buckets"), WithTimeToWriteHeaderDurationBuckets([]float64{1, 2, 3}), }, }, { name: "byte_bucket_size_buckets", opts: []HandlerFactoryOption{ - WithNamespace("byte_bucket_size_buckets"), WithByteSizeBuckets([]float64{1, 2, 3}), }, }, + func() TestCase { + registry := prometheus.NewRegistry() + + return TestCase{ + name: "injected registerer", + opts: []HandlerFactoryOption{WithRegisterer(registry)}, + registry: registry, + } + }(), } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // NewHandlerFactory unfortunately registers the metrics by default in the global registry which + // isn't something we can change without breaking backwards compatibility. Isolate these tests by + // overriding the global defaults with a custom registry, and revert back to the originals after + // the test. + originalRegisterer, originalGatherer := prometheus.DefaultRegisterer, prometheus.DefaultGatherer + defer func() { + prometheus.DefaultRegisterer = originalRegisterer + prometheus.DefaultGatherer = originalGatherer + }() + + defaultRegistry := prometheus.NewRegistry() + prometheus.DefaultRegisterer = defaultRegistry + prometheus.DefaultGatherer = defaultRegistry + factory := NewHandlerFactory(tt.opts...) require.NotNil(t, factory) @@ -110,8 +140,19 @@ func TestNewHandlerFactory(t *testing.T) { handler.ServeHTTP(w, r) require.True(t, invoked, "handler was never invoked") - // Gather metrics from the default registry - metrics, err := prometheus.DefaultGatherer.Gather() + registry := defaultRegistry + if tt.registry != nil { + registry = tt.registry + + // If a registry was injected via options, no metrics should be registered + // into the default registry. + metrics, err := defaultRegistry.Gather() + require.NoError(t, err) + require.Empty(t, metrics) + } + + // Gather metrics from the registry + metrics, err := registry.Gather() require.NoError(t, err) require.NotEmpty(t, metrics) @@ -120,8 +161,10 @@ func TestNewHandlerFactory(t *testing.T) { keyedMetrics[v.GetName()] = v } - // For each test, the namespace is "tt.name", so we expect e.g. "basic_http_requests_total" - ns := tt.name + "_http_" + // Add the possible custom namespace the test configures into the metric names. + // So we by default expect e.g. "basic_http_requests_total", and with a custom namespace + // e.g "namespace_basic_http_requests_total". + ns := tt.namespace + "http_" // Since each test calls 1 request, we expect: // - in-flight gauge => 0 after finishing