Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

prevent conflict metric info

CHANGELOG.md

@@ -52,6 +52,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Asynchronous callbacks are only called if they are registered with at least one instrument that does not use drop aggragation. (#3408)
 - Do not report empty partial-success responses in the `go.opentelemetry.io/otel/exporters/otlp` exporters. (#3438, #3432)
 - Handle partial success responses in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric` exporters. (#3162, #3440)
+- Prevent duplicate Prometheus description, unit, and type. (#3469)
 - Prevents panic when using incorrect `attribute.Value.As[Type]Slice()`. (#3489)
 
 ## Removed

exporters/prometheus/exporter.go

@@ -24,6 +24,8 @@ import (
 	"unicode/utf8"
 
 	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
@@ -62,6 +64,7 @@ type collector struct {
 	disableScopeInfo     bool
 	createTargetInfoOnce sync.Once
 	scopeInfos           map[instrumentation.Scope]prometheus.Metric
+	metricFamilies       map[string]*dto.MetricFamily
 }
 
 // prometheus counters MUST have a _total suffix:
@@ -83,6 +86,7 @@ func New(opts ...Option) (*Exporter, error) {
 		withoutUnits:      cfg.withoutUnits,
 		disableScopeInfo:  cfg.disableScopeInfo,
 		scopeInfos:        make(map[instrumentation.Scope]prometheus.Metric),
+		metricFamilies:    make(map[string]*dto.MetricFamily),
 	}
 
 	if err := cfg.registerer.Register(collector); err != nil {
@@ -149,22 +153,30 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
 		for _, m := range scopeMetrics.Metrics {
 			switch v := m.Data.(type) {
 			case metricdata.Histogram:
-				addHistogramMetric(ch, v, m, keys, values, c.getName(m))
+				addHistogramMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
 			case metricdata.Sum[int64]:
-				addSumMetric(ch, v, m, keys, values, c.getName(m))
+				addSumMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
 			case metricdata.Sum[float64]:
-				addSumMetric(ch, v, m, keys, values, c.getName(m))
+				addSumMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
 			case metricdata.Gauge[int64]:
-				addGaugeMetric(ch, v, m, keys, values, c.getName(m))
+				addGaugeMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
 			case metricdata.Gauge[float64]:
-				addGaugeMetric(ch, v, m, keys, values, c.getName(m))
+				addGaugeMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
 			}
 		}
 	}
 }
 
-func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histogram, m metricdata.Metrics, ks, vs [2]string, name string) {
+func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histogram, m metricdata.Metrics, ks, vs [2]string, name string, mfs map[string]*dto.MetricFamily) {
 	// TODO(https://github.com/open-telemetry/opentelemetry-go/issues/3163): support exemplars
+	drop, help := validateMetrics(name, m.Description, dto.MetricType_HISTOGRAM.Enum(), mfs)
+	if drop {
+		return
+	}
+	if help != "" {
+		m.Description = help
+	}
+
 	for _, dp := range histogram.DataPoints {
 		keys, values := getAttrs(dp.Attributes, ks, vs)
 
@@ -185,15 +197,26 @@ func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histog
 	}
 }
 
-func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata.Sum[N], m metricdata.Metrics, ks, vs [2]string, name string) {
+func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata.Sum[N], m metricdata.Metrics, ks, vs [2]string, name string, mfs map[string]*dto.MetricFamily) {
 	valueType := prometheus.CounterValue
+	metricType := dto.MetricType_COUNTER
 	if !sum.IsMonotonic {
 		valueType = prometheus.GaugeValue
+		metricType = dto.MetricType_GAUGE
 	}
 	if sum.IsMonotonic {
 		// Add _total suffix for counters
 		name += counterSuffix
 	}
+
+	drop, help := validateMetrics(name, m.Description, metricType.Enum(), mfs)
+	if drop {
+		return
+	}
+	if help != "" {
+		m.Description = help
+	}
+
 	for _, dp := range sum.DataPoints {
 		keys, values := getAttrs(dp.Attributes, ks, vs)
 
@@ -207,7 +230,15 @@ func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata
 	}
 }
 
-func addGaugeMetric[N int64 | float64](ch chan<- prometheus.Metric, gauge metricdata.Gauge[N], m metricdata.Metrics, ks, vs [2]string, name string) {
+func addGaugeMetric[N int64 | float64](ch chan<- prometheus.Metric, gauge metricdata.Gauge[N], m metricdata.Metrics, ks, vs [2]string, name string, mfs map[string]*dto.MetricFamily) {
+	drop, help := validateMetrics(name, m.Description, dto.MetricType_GAUGE.Enum(), mfs)
+	if drop {
+		return
+	}
+	if help != "" {
+		m.Description = help
+	}
+
 	for _, dp := range gauge.DataPoints {
 		keys, values := getAttrs(dp.Attributes, ks, vs)
 
@@ -344,3 +375,27 @@ func sanitizeName(n string) string {
 
 	return b.String()
 }
+
+func validateMetrics(name, description string, metricType *dto.MetricType, mfs map[string]*dto.MetricFamily) (drop bool, help string) {
+	emf, exist := mfs[name]
+	if !exist {
+		mfs[name] = &dto.MetricFamily{
+			Name: proto.String(name),
+			Help: proto.String(description),
+			Type: metricType,
+		}
+		return false, ""
+	}
+	if emf.GetHelp() != description {
+		if emf.GetType() != *metricType && (emf.GetType() == dto.MetricType_HISTOGRAM || *metricType == dto.MetricType_HISTOGRAM) {
+			return false, ""
+		}
+		otel.Handle(fmt.Errorf("the description %q of instrument %q conflicts with existing description %q, overriding with existing one", description, name, emf.GetHelp()))
+		return false, emf.GetHelp()
+	}
+	if emf.GetType() != *metricType {
+		otel.Handle(fmt.Errorf("the type %q of instrument %q conflicts with existing type %q, drop the new one. Please use a view that selects this instrument, and rename to a different name", *metricType, name, emf.GetType()))
+		return true, ""
+	}
+	return false, ""
+}

exporters/prometheus/exporter_test.go

@@ -386,3 +386,178 @@ func TestMultiScopes(t *testing.T) {
 	err = testutil.GatherAndCompare(registry, file)
 	require.NoError(t, err)
 }
+
+func TestDuplicateMetrics(t *testing.T) {
+	testCases := []struct {
+		name                  string
+		customResouceAttrs    []attribute.KeyValue
+		recordMetrics         func(ctx context.Context, meterA, meterB otelmetric.Meter)
+		options               []Option
+		possibleExpectedFiles []string
+	}{
+		{
+			name: "no_conflict",
+			recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) {
+				fooA, err := meterA.SyncInt64().Counter("foo",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter foo"))
+				assert.NoError(t, err)
+				fooA.Add(ctx, 100, attribute.String("A", "B"))
+
+				fooB, err := meterB.SyncInt64().Counter("foo",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter foo"))
+				assert.NoError(t, err)
+				fooB.Add(ctx, 100, attribute.String("A", "B"))
+
+				fooHistogramB, err := meterB.SyncInt64().Histogram("foo",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter histogram foo"))
+				assert.NoError(t, err)
+				fooHistogramB.Record(ctx, 100, attribute.String("A", "B"))
+			},
+			possibleExpectedFiles: []string{"testdata/no_conflict.txt"},
+		},
+		{
+			name: "conflict_help_counter",
+			recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) {
+				barA, err := meterA.SyncInt64().Counter("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter a bar"))
+				assert.NoError(t, err)
+				barA.Add(ctx, 100, attribute.String("type", "bar"))
+
+				barB, err := meterB.SyncInt64().Counter("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter b bar"))
+				assert.NoError(t, err)
+				barB.Add(ctx, 100, attribute.String("type", "bar"))
+			},
+			possibleExpectedFiles: []string{
+				"testdata/conflict_help_counter_1.txt",
+				"testdata/conflict_help_counter_2.txt",
+			},
+		},
+		{
+			name: "conflict_help_updowncounter",
+			recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) {
+				barA, err := meterA.SyncInt64().UpDownCounter("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter a bar"))
+				assert.NoError(t, err)
+				barA.Add(ctx, 100, attribute.String("type", "bar"))
+
+				barB, err := meterB.SyncInt64().UpDownCounter("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter b bar"))
+				assert.NoError(t, err)
+				barB.Add(ctx, 100, attribute.String("type", "bar"))
+			},
+			possibleExpectedFiles: []string{
+				"testdata/conflict_help_updowncounter_1.txt",
+				"testdata/conflict_help_updowncounter_2.txt",
+			},
+		},
+		{
+			name: "conflict_help_histogram",
+			recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) {
+				barA, err := meterA.SyncInt64().Histogram("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter a bar"))
+				assert.NoError(t, err)
+				barA.Record(ctx, 100, attribute.String("A", "B"))
+
+				barB, err := meterB.SyncInt64().Histogram("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter b bar"))
+				assert.NoError(t, err)
+				barB.Record(ctx, 100, attribute.String("A", "B"))
+			},
+			possibleExpectedFiles: []string{
+				"testdata/conflict_help_histogram_1.txt",
+				"testdata/conflict_help_histogram_2.txt",
+			},
+		},
+		{
+			name: "conflict_unit",
+			recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) {
+				bazA, err := meterA.SyncInt64().Counter("bar",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter bar"))
+				assert.NoError(t, err)
+				bazA.Add(ctx, 100, attribute.String("type", "bar"))
+
+				bazB, err := meterB.SyncInt64().Counter("bar",
+					instrument.WithUnit(unit.Milliseconds),
+					instrument.WithDescription("meter bar"))
+				assert.NoError(t, err)
+				bazB.Add(ctx, 100, attribute.String("type", "bar"))
+			},
+			options:               []Option{WithoutUnits()},
+			possibleExpectedFiles: []string{"testdata/conflict_unit.txt"},
+		},
+		{
+			name: "conflict_type_counter",
+			recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) {
+				counter, err := meterA.SyncInt64().Counter("foo",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter foo"))
+				assert.NoError(t, err)
+				counter.Add(ctx, 100, attribute.String("type", "foo"))
+
+				gauge, err := meterA.SyncInt64().UpDownCounter("foo_total",
+					instrument.WithUnit(unit.Bytes),
+					instrument.WithDescription("meter foo"))
+				assert.NoError(t, err)
+				gauge.Add(ctx, 200, attribute.String("type", "foo"))
+			},
+			options:               []Option{WithoutUnits()},
+			possibleExpectedFiles: []string{"testdata/conflict_type_counter.txt"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// initialize registry exporter
+			ctx := context.Background()
+			registry := prometheus.NewRegistry()
+			exporter, err := New(append(tc.options, WithRegisterer(registry))...)
+			require.NoError(t, err)
+
+			// initialize resource
+			res, err := resource.New(ctx,
+				resource.WithAttributes(semconv.ServiceNameKey.String("prometheus_test")),
+				resource.WithAttributes(semconv.TelemetrySDKVersionKey.String("latest")),
+			)
+			require.NoError(t, err)
+			res, err = resource.Merge(resource.Default(), res)
+			require.NoError(t, err)
+
+			// initialize provider
+			provider := metric.NewMeterProvider(
+				metric.WithReader(exporter),
+				metric.WithResource(res),
+			)
+
+			// initialize two meter a, b
+			meterA := provider.Meter("ma", otelmetric.WithInstrumentationVersion("v0.1.0"))
+			meterB := provider.Meter("mb", otelmetric.WithInstrumentationVersion("v0.1.0"))
+
+			tc.recordMetrics(ctx, meterA, meterB)
+
+			var match = false
+			for _, filename := range tc.possibleExpectedFiles {
+				file, ferr := os.Open(filename)
+				require.NoError(t, ferr)
+				t.Cleanup(func() { require.NoError(t, file.Close()) })
+
+				err = testutil.GatherAndCompare(registry, file)
+				if err == nil {
+					match = true
+					break
+				}
+			}
+			require.Truef(t, match, "expected export not produced: %v", err)
+		})
+	}
+}

exporters/prometheus/go.mod

@@ -4,11 +4,13 @@ go 1.18
 
 require (
 	github.com/prometheus/client_golang v1.14.0
+	github.com/prometheus/client_model v0.3.0
 	github.com/stretchr/testify v1.8.1
 	go.opentelemetry.io/otel v1.11.1
 	go.opentelemetry.io/otel/metric v0.33.0
 	go.opentelemetry.io/otel/sdk v1.11.1
 	go.opentelemetry.io/otel/sdk/metric v0.33.0
+	google.golang.org/protobuf v1.28.1
 )
 
 require (
@@ -20,12 +22,10 @@ require (
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	github.com/prometheus/client_model v0.3.0 // indirect
 	github.com/prometheus/common v0.37.0 // indirect
 	github.com/prometheus/procfs v0.8.0 // indirect
 	go.opentelemetry.io/otel/trace v1.11.1 // indirect
 	golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect
-	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 

exporters/prometheus/testdata/conflict_help_counter_1.txt

@@ -0,0 +1,11 @@
+# HELP bar_bytes_total meter a bar
+# TYPE bar_bytes_total counter
+bar_bytes_total{otel_scope_name="ma",otel_scope_version="v0.1.0",type="bar"} 100
+bar_bytes_total{otel_scope_name="mb",otel_scope_version="v0.1.0",type="bar"} 100
+# HELP otel_scope_info Instrumentation Scope metadata
+# TYPE otel_scope_info gauge
+otel_scope_info{otel_scope_name="ma",otel_scope_version="v0.1.0"} 1
+otel_scope_info{otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+# HELP target_info Target metadata
+# TYPE target_info gauge
+target_info{service_name="prometheus_test",telemetry_sdk_language="go",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="latest"} 1

exporters/prometheus/testdata/conflict_help_counter_2.txt

@@ -0,0 +1,11 @@
+# HELP bar_bytes_total meter b bar
+# TYPE bar_bytes_total counter
+bar_bytes_total{otel_scope_name="ma",otel_scope_version="v0.1.0",type="bar"} 100
+bar_bytes_total{otel_scope_name="mb",otel_scope_version="v0.1.0",type="bar"} 100
+# HELP otel_scope_info Instrumentation Scope metadata
+# TYPE otel_scope_info gauge
+otel_scope_info{otel_scope_name="ma",otel_scope_version="v0.1.0"} 1
+otel_scope_info{otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+# HELP target_info Target metadata
+# TYPE target_info gauge
+target_info{service_name="prometheus_test",telemetry_sdk_language="go",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="latest"} 1