From 41bf8a453e57bafaed6e536270fe00fde903c557 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Thu, 10 Nov 2022 21:31:23 +0800 Subject: [PATCH] Signed-off-by: Ziqi Zhao prevent conflict metric info --- exporters/prometheus/exporter.go | 135 ++++++++++++++++- exporters/prometheus/exporter_test.go | 137 ++++++++++++++++++ .../prometheus/testdata/conflict_help.txt | 11 ++ .../testdata/conflict_type_counter.txt | 9 ++ .../testdata/conflict_type_histogram.txt | 26 ++++ .../prometheus/testdata/conflict_unit.txt | 11 ++ exporters/prometheus/testdata/no_conflict.txt | 11 ++ 7 files changed, 335 insertions(+), 5 deletions(-) create mode 100644 exporters/prometheus/testdata/conflict_help.txt create mode 100644 exporters/prometheus/testdata/conflict_type_counter.txt create mode 100644 exporters/prometheus/testdata/conflict_type_histogram.txt create mode 100644 exporters/prometheus/testdata/conflict_unit.txt create mode 100644 exporters/prometheus/testdata/no_conflict.txt diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 2cb23cc19fb..cbbb2397f8f 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -23,7 +23,9 @@ import ( "unicode" "unicode/utf8" + "github.com/golang/protobuf/proto" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "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 + metricFamiliesByName map[string]*dto.MetricFamily } // prometheus counters MUST have a _total suffix: @@ -78,11 +81,12 @@ func New(opts ...Option) (*Exporter, error) { reader := metric.NewManualReader(cfg.manualReaderOptions()...) collector := &collector{ - reader: reader, - disableTargetInfo: cfg.disableTargetInfo, - withoutUnits: cfg.withoutUnits, - disableScopeInfo: cfg.disableScopeInfo, - scopeInfos: make(map[instrumentation.Scope]prometheus.Metric), + reader: reader, + disableTargetInfo: cfg.disableTargetInfo, + withoutUnits: cfg.withoutUnits, + disableScopeInfo: cfg.disableScopeInfo, + scopeInfos: make(map[instrumentation.Scope]prometheus.Metric), + metricFamiliesByName: make(map[string]*dto.MetricFamily), } if err := cfg.registerer.Register(collector); err != nil { @@ -147,6 +151,14 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { } for _, m := range scopeMetrics.Metrics { + drop, m, err := c.validateMetrics(m) + if err != nil { + fmt.Printf("warning: %v\n", err) + } + if drop { + continue + } + switch v := m.Data.(type) { case metricdata.Histogram: addHistogramMetric(ch, v, m, keys, values, c.getName(m)) @@ -344,3 +356,116 @@ func sanitizeName(n string) string { return b.String() } + +func (c *collector) validateMetrics(m metricdata.Metrics) (bool, metricdata.Metrics, error) { + // get the name of metric name + name := c.getName(m) + switch m.Data.(type) { + case metricdata.Sum[int64]: + sum := m.Data.(metricdata.Sum[int64]) + if sum.IsMonotonic { + name += counterSuffix + } + case metricdata.Sum[float64]: + sum := m.Data.(metricdata.Sum[int64]) + if sum.IsMonotonic { + name += counterSuffix + } + } + + metricFamily := &dto.MetricFamily{ + Name: proto.String(name), + Help: proto.String(m.Description), + } + switch m.Data.(type) { + case metricdata.Sum[int64]: + sum := m.Data.(metricdata.Sum[int64]) + metricFamily.Type = dto.MetricType_COUNTER.Enum() + if !sum.IsMonotonic { + metricFamily.Type = dto.MetricType_GAUGE.Enum() + } + case metricdata.Sum[float64]: + sum := m.Data.(metricdata.Sum[float64]) + metricFamily.Type = dto.MetricType_COUNTER.Enum() + if !sum.IsMonotonic { + metricFamily.Type = dto.MetricType_GAUGE.Enum() + } + case metricdata.Gauge[int64]: + metricFamily.Type = dto.MetricType_GAUGE.Enum() + case metricdata.Gauge[float64]: + metricFamily.Type = dto.MetricType_GAUGE.Enum() + case metricdata.Histogram: + metricFamily.Type = dto.MetricType_HISTOGRAM.Enum() + } + + collision := validateCollision(metricFamily, c.metricFamiliesByName) + switch collision { + case noCollision: + c.metricFamiliesByName[name] = metricFamily + return false, m, nil + case typeCollision: + return true, m, fmt.Errorf("type collision") + case helpCollision: + emf := c.metricFamiliesByName[name] + m.Description = emf.GetHelp() + return false, m, fmt.Errorf("help collision") + } + + return false, m, nil +} + +type collisionType int + +const ( + noCollision collisionType = 0 + typeCollision collisionType = 1 + helpCollision collisionType = 2 +) + +func validateCollision(mf *dto.MetricFamily, mfs map[string]*dto.MetricFamily) collisionType { + newName := mf.GetName() + newType := mf.GetType() + newNameWithoutSuffix := "" + + emf, exist := mfs[newName] + if exist { + if emf.GetType() != mf.GetType() { + return typeCollision + } + + if emf.GetHelp() != mf.GetHelp() { + return helpCollision + } + } + + switch { + case strings.HasSuffix(newName, "_count"): + newNameWithoutSuffix = newName[:len(newName)-6] + case strings.HasSuffix(newName, "_sum"): + newNameWithoutSuffix = newName[:len(newName)-4] + case strings.HasSuffix(newName, "_bucket"): + newNameWithoutSuffix = newName[:len(newName)-7] + } + + if newNameWithoutSuffix != "" { + if emf, exist = mfs[newNameWithoutSuffix]; exist { + if emf.GetType() == dto.MetricType_HISTOGRAM { + return typeCollision + } + } + } + + if newType == dto.MetricType_HISTOGRAM { + if _, exist = mfs[newName+"_count"]; exist { + return typeCollision + } + if _, exist = mfs[newName+"_sum"]; exist { + return typeCollision + } + if _, exist = mfs[newName+"_bucket"]; exist { + return typeCollision + } + } + + return noCollision +} diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index 4edf11ba48a..18a12121d07 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -386,3 +386,140 @@ 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 + expectedFile 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("type", "foo")) + + 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")) + }, + expectedFile: "testdata/no_conflict.txt", + }, + { + name: "conflict_help", + recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) { + fooA, err := meterA.SyncInt64().Counter("foo", + instrument.WithUnit(unit.Bytes), + instrument.WithDescription("meter a foo")) + assert.NoError(t, err) + fooA.Add(ctx, 100, attribute.String("type", "foo")) + + fooB, err := meterB.SyncInt64().Counter("foo", + instrument.WithUnit(unit.Bytes), + instrument.WithDescription("meter b foo")) + assert.NoError(t, err) + fooB.Add(ctx, 100, attribute.String("type", "foo")) + }, + expectedFile: "testdata/conflict_help.txt", + }, + { + name: "conflict_unit", + 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("type", "foo")) + + fooB, err := meterB.SyncInt64().Counter("foo", + instrument.WithUnit(unit.Milliseconds), + instrument.WithDescription("meter foo")) + assert.NoError(t, err) + fooB.Add(ctx, 100, attribute.String("type", "foo")) + }, + options: []Option{WithoutUnits()}, + expectedFile: "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()}, + expectedFile: "testdata/conflict_type_counter.txt", + }, + { + name: "conflict_type_histogram", + recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) { + histogram, err := meterA.SyncInt64().Histogram("foo", + instrument.WithUnit(unit.Bytes), + instrument.WithDescription("meter foo")) + assert.NoError(t, err) + histogram.Record(ctx, 100, attribute.String("type", "foo")) + + gauge, err := meterA.SyncInt64().UpDownCounter("foo_count", + instrument.WithUnit(unit.Bytes), + instrument.WithDescription("meter foo")) + assert.NoError(t, err) + gauge.Add(ctx, 200, attribute.String("type", "foo")) + }, + options: []Option{WithoutUnits()}, + expectedFile: "testdata/conflict_type_histogram.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) + + file, err := os.Open(tc.expectedFile) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, file.Close()) }) + + err = testutil.GatherAndCompare(registry, file) + require.NoError(t, err) + }) + } +} diff --git a/exporters/prometheus/testdata/conflict_help.txt b/exporters/prometheus/testdata/conflict_help.txt new file mode 100644 index 00000000000..c8d956af156 --- /dev/null +++ b/exporters/prometheus/testdata/conflict_help.txt @@ -0,0 +1,11 @@ +# HELP foo_bytes_total meter a 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 +# 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 diff --git a/exporters/prometheus/testdata/conflict_type_counter.txt b/exporters/prometheus/testdata/conflict_type_counter.txt new file mode 100644 index 00000000000..885fb3c7259 --- /dev/null +++ b/exporters/prometheus/testdata/conflict_type_counter.txt @@ -0,0 +1,9 @@ +# HELP foo_total meter foo +# TYPE foo_total counter +foo_total{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo"} 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 +# 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 diff --git a/exporters/prometheus/testdata/conflict_type_histogram.txt b/exporters/prometheus/testdata/conflict_type_histogram.txt new file mode 100644 index 00000000000..68e867ab6fc --- /dev/null +++ b/exporters/prometheus/testdata/conflict_type_histogram.txt @@ -0,0 +1,26 @@ +# HELP foo meter foo +# TYPE foo histogram +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="0"} 0 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="5"} 0 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="10"} 0 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="25"} 0 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="50"} 0 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="75"} 0 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="100"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="250"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="500"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="750"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="1000"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="2500"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="5000"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="7500"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="10000"} 1 +foo_bucket{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo",le="+Inf"} 1 +foo_sum{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo"} 100 +foo_count{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo"} 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 +# 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 diff --git a/exporters/prometheus/testdata/conflict_unit.txt b/exporters/prometheus/testdata/conflict_unit.txt new file mode 100644 index 00000000000..527941f91c0 --- /dev/null +++ b/exporters/prometheus/testdata/conflict_unit.txt @@ -0,0 +1,11 @@ +# HELP foo_total meter foo +# TYPE foo_total counter +foo_total{otel_scope_name="ma",otel_scope_version="v0.1.0",type="foo"} 100 +foo_total{otel_scope_name="mb",otel_scope_version="v0.1.0",type="foo"} 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 diff --git a/exporters/prometheus/testdata/no_conflict.txt b/exporters/prometheus/testdata/no_conflict.txt new file mode 100644 index 00000000000..dff7fb63600 --- /dev/null +++ b/exporters/prometheus/testdata/no_conflict.txt @@ -0,0 +1,11 @@ +# 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 +# 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