fix for reviews

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
open-telemetry · Nov 25, 2022 · 1e72b9d · 1e72b9d
1 parent 3c84770
commit 1e72b9d
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 10 deletions.
diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go
@@ -171,11 +171,9 @@ func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histog
 	// 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 {
-		otel.Handle(fmt.Errorf("type conflict"))
 		return
 	}
 	if help != "" {
-		otel.Handle(fmt.Errorf("help conflict"))
 		m.Description = help
 	}
 
@@ -213,11 +211,9 @@ func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata
 
 	drop, help := validateMetrics(name, m.Description, metricType.Enum(), mfs)
 	if drop {
-		otel.Handle(fmt.Errorf("type conflict"))
 		return
 	}
 	if help != "" {
-		otel.Handle(fmt.Errorf("help conflict"))
 		m.Description = help
 	}
 
@@ -237,11 +233,9 @@ 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, mfs map[string]*dto.MetricFamily) {
 	drop, help := validateMetrics(name, m.Description, dto.MetricType_GAUGE.Enum(), mfs)
 	if drop {
-		otel.Handle(fmt.Errorf("type conflict"))
 		return
 	}
 	if help != "" {
-		otel.Handle(fmt.Errorf("help conflict"))
 		m.Description = help
 	}
 
@@ -396,9 +390,11 @@ func validateMetrics(name, description string, metricType *dto.MetricType, mfs m
 		if emf.GetType() == dto.MetricType_HISTOGRAM || *metricType == dto.MetricType_HISTOGRAM {
 			return false, ""
 		}
+		otel.Handle(fmt.Errorf("The description '%s' of instrument '%s' conflicts with existing description '%s', override with existing one.", description, name, emf.GetHelp()))
 		return false, emf.GetHelp()
 	}
 	if emf.GetType() != *metricType {
+		otel.Handle(fmt.Errorf("The type '%s' of instrument '%s' conflicts with existing type '%s', 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, ""

diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go
@@ -402,13 +402,19 @@ func TestDuplicateMetrics(t *testing.T) {
 					instrument.WithUnit(unit.Bytes),
 					instrument.WithDescription("meter foo"))
 				assert.NoError(t, err)
-				fooA.Add(ctx, 100, attribute.String("type", "foo"))
+				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("type", "foo"))
+				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"},
 		},
@@ -506,6 +512,7 @@ func TestDuplicateMetrics(t *testing.T) {
 				t.Cleanup(func() { require.NoError(t, file.Close()) })
 
 				err = testutil.GatherAndCompare(registry, file)
+				require.NoError(t, err)
 				if err == nil {
 					match = true
 					break

diff --git a/exporters/prometheus/testdata/no_conflict.txt b/exporters/prometheus/testdata/no_conflict.txt
@@ -1,7 +1,27 @@
 # HELP foo_bytes_total meter foo
 # TYPE foo_bytes_total counter
-foo_bytes_total{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo"} 100
-foo_bytes_total{otel_scope_name="mb",otel_scope_version="v0.1.0",type="foo"} 100
+foo_bytes_total{A="B",otel_scope_name="ma",otel_scope_version="v0.1.0"} 100
+foo_bytes_total{A="B",otel_scope_name="mb",otel_scope_version="v0.1.0"} 100
+# HELP foo_bytes meter histogram foo
+# TYPE foo_bytes histogram
+foo_bytes_bucket{A="B",le="0",otel_scope_name="mb",otel_scope_version="v0.1.0"} 0
+foo_bytes_bucket{A="B",le="5",otel_scope_name="mb",otel_scope_version="v0.1.0"} 0
+foo_bytes_bucket{A="B",le="10",otel_scope_name="mb",otel_scope_version="v0.1.0"} 0
+foo_bytes_bucket{A="B",le="25",otel_scope_name="mb",otel_scope_version="v0.1.0"} 0
+foo_bytes_bucket{A="B",le="50",otel_scope_name="mb",otel_scope_version="v0.1.0"} 0
+foo_bytes_bucket{A="B",le="75",otel_scope_name="mb",otel_scope_version="v0.1.0"} 0
+foo_bytes_bucket{A="B",le="100",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="250",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="500",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="750",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="1000",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="2500",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="5000",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="7500",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="10000",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_bucket{A="B",le="+Inf",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
+foo_bytes_sum{A="B",otel_scope_name="mb",otel_scope_version="v0.1.0"} 100
+foo_bytes_count{A="B",otel_scope_name="mb",otel_scope_version="v0.1.0"} 1
 # 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