From 7e6da12625133d21149ce246dfd510eba7ed959f Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 25 Oct 2023 10:16:10 -0400 Subject: [PATCH] Prometheus exporter no longer collects metrics after shutdown (#4648) * prometheus exporter no longer collects metrics after shutdown * use errors.Is --- CHANGELOG.md | 4 ++++ exporters/prometheus/exporter.go | 5 +++- exporters/prometheus/exporter_test.go | 34 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33937681837..b24494ac9a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` does no longer depend on `go.opentelemetry.io/otel/exporters/otlp/otlpmetric`. (#4660) - `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` does no longer depend on `go.opentelemetry.io/otel/exporters/otlp/otlpmetric`. (#4660) +### Fixed + +- In `go.opentelemetry.op/otel/exporters/prometheus`, the exporter no longer `Collect`s metrics after `Shutdown` is invoked. (#4648) + ## [1.19.0/0.42.0/0.0.7] 2023-09-28 This release contains the first stable release of the OpenTelemetry Go [metric SDK]. diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 8973b2028e6..92651c38cec 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -148,8 +148,11 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { metrics := metricdata.ResourceMetrics{} err := c.reader.Collect(context.TODO(), &metrics) if err != nil { + if errors.Is(err, metric.ErrReaderShutdown) { + return + } otel.Handle(err) - if err == metric.ErrReaderNotRegistered { + if errors.Is(err, metric.ErrReaderNotRegistered) { return } } diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index 0f211d8318e..bd31657825e 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -16,6 +16,7 @@ package prometheus import ( "context" + "errors" "io" "os" "sync" @@ -835,3 +836,36 @@ func TestIncompatibleMeterName(t *testing.T) { require.NoError(t, err) assert.Equal(t, 1, len(errs)) } + +func TestShutdownExporter(t *testing.T) { + var handledError error + eh := otel.ErrorHandlerFunc(func(e error) { handledError = errors.Join(handledError, e) }) + otel.SetErrorHandler(eh) + + ctx := context.Background() + registry := prometheus.NewRegistry() + + for i := 0; i < 3; i++ { + exporter, err := New(WithRegisterer(registry)) + require.NoError(t, err) + provider := metric.NewMeterProvider( + metric.WithResource(resource.Default()), + metric.WithReader(exporter)) + meter := provider.Meter("testmeter") + cnt, err := meter.Int64Counter("foo") + require.NoError(t, err) + cnt.Add(ctx, 100) + + // verify that metrics added to a previously shutdown MeterProvider + // do not conflict with metrics added in this loop. + _, err = registry.Gather() + require.NoError(t, err) + + // Shutdown should cause future prometheus Gather() calls to no longer + // include metrics from this loop's MeterProvider. + err = provider.Shutdown(ctx) + require.NoError(t, err) + } + // ensure we aren't unnecessarily logging errors from the shutdown MeterProvider + require.NoError(t, handledError) +}