From ac5639159f9c8ec161a93b1990e878c74022d49b Mon Sep 17 00:00:00 2001
From: Damien Mathieu <damien.mathieu@elastic.co>
Date: Thu, 14 Sep 2023 09:51:51 +0200
Subject: [PATCH] Call scopeInfoMetrics only once even if it returns an error
 (#4499)

---
 CHANGELOG.md                          |  4 ++++
 exporters/prometheus/exporter.go      | 17 ++++++++++++++++-
 exporters/prometheus/exporter_test.go | 19 +++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7e264600fd4..056f12c45fb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,6 +38,10 @@ This release drops the compatibility guarantee of [Go 1.19].
 - Removed the deprecated internal packages in `go.opentelemetry.io/otel/exporters/otlp` and its sub-packages. (#4469)
 - Dropped guaranteed support for versions of Go less than 1.20. (#4481)
 
+### Fixed
+
+- In `go.opentelemetry.op/otel/exporters/prometheus`, don't try to create the prometheus metric on every `Collect` if we know the scope is invalid. (#4499)
+
 ## [1.17.0/0.40.0/0.0.5] 2023-08-28
 
 ### Added
diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go
index c88e82a1385..8973b2028e6 100644
--- a/exporters/prometheus/exporter.go
+++ b/exporters/prometheus/exporter.go
@@ -45,7 +45,11 @@ const (
 	scopeInfoDescription = "Instrumentation Scope metadata"
 )
 
-var scopeInfoKeys = [2]string{"otel_scope_name", "otel_scope_version"}
+var (
+	scopeInfoKeys = [2]string{"otel_scope_name", "otel_scope_version"}
+
+	errScopeInvalid = errors.New("invalid scope")
+)
 
 // Exporter is a Prometheus Exporter that embeds the OTel metric.Reader
 // interface for easy instantiation with a MeterProvider.
@@ -87,6 +91,7 @@ type collector struct {
 	disableTargetInfo bool
 	targetInfo        prometheus.Metric
 	scopeInfos        map[instrumentation.Scope]prometheus.Metric
+	scopeInfosInvalid map[instrumentation.Scope]struct{}
 	metricFamilies    map[string]*dto.MetricFamily
 }
 
@@ -110,6 +115,7 @@ func New(opts ...Option) (*Exporter, error) {
 		withoutCounterSuffixes: cfg.withoutCounterSuffixes,
 		disableScopeInfo:       cfg.disableScopeInfo,
 		scopeInfos:             make(map[instrumentation.Scope]prometheus.Metric),
+		scopeInfosInvalid:      make(map[instrumentation.Scope]struct{}),
 		metricFamilies:         make(map[string]*dto.MetricFamily),
 		namespace:              cfg.namespace,
 	}
@@ -177,6 +183,10 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
 
 		if !c.disableScopeInfo {
 			scopeInfo, err := c.scopeInfo(scopeMetrics.Scope)
+			if err == errScopeInvalid {
+				// Do not report the same error multiple times.
+				continue
+			}
 			if err != nil {
 				otel.Handle(err)
 				continue
@@ -469,8 +479,13 @@ func (c *collector) scopeInfo(scope instrumentation.Scope) (prometheus.Metric, e
 		return scopeInfo, nil
 	}
 
+	if _, ok := c.scopeInfosInvalid[scope]; ok {
+		return nil, errScopeInvalid
+	}
+
 	scopeInfo, err := createScopeInfoMetric(scope)
 	if err != nil {
+		c.scopeInfosInvalid[scope] = struct{}{}
 		return nil, fmt.Errorf("cannot create scope info metric: %w", err)
 	}
 
diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go
index 86cc8dc0b20..89fc87df3b9 100644
--- a/exporters/prometheus/exporter_test.go
+++ b/exporters/prometheus/exporter_test.go
@@ -16,6 +16,7 @@ package prometheus
 
 import (
 	"context"
+	"io"
 	"os"
 	"sync"
 	"testing"
@@ -25,6 +26,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	otelmetric "go.opentelemetry.io/otel/metric"
 	"go.opentelemetry.io/otel/sdk/metric"
@@ -790,6 +792,14 @@ func TestCollectorConcurrentSafe(t *testing.T) {
 }
 
 func TestIncompatibleMeterName(t *testing.T) {
+	defer func(orig otel.ErrorHandler) {
+		otel.SetErrorHandler(orig)
+	}(otel.GetErrorHandler())
+
+	errs := []error{}
+	eh := otel.ErrorHandlerFunc(func(e error) { errs = append(errs, e) })
+	otel.SetErrorHandler(eh)
+
 	// This test checks that Prometheus exporter ignores
 	// when it encounters incompatible meter name.
 
@@ -815,4 +825,13 @@ func TestIncompatibleMeterName(t *testing.T) {
 
 	err = testutil.GatherAndCompare(registry, file)
 	require.NoError(t, err)
+
+	assert.Equal(t, 1, len(errs))
+
+	// A second collect shouldn't trigger new errors
+	_, err = file.Seek(0, io.SeekStart)
+	assert.NoError(t, err)
+	err = testutil.GatherAndCompare(registry, file)
+	require.NoError(t, err)
+	assert.Equal(t, 1, len(errs))
 }