From a1a652acd03503b2dbe29dafb1921d3a80db6c73 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Mon, 21 Nov 2022 12:46:42 +0800 Subject: [PATCH] enhance logic Signed-off-by: Ziqi Zhao --- exporters/prometheus/exporter.go | 45 ++++++++-- exporters/prometheus/exporter_test.go | 82 ++++++------------- ...cs_conflict_help.txt => conflict_help.txt} | 0 ...ict_type.txt => conflict_type_counter.txt} | 0 .../testdata/conflict_type_histogram.txt | 26 ++++++ ...cs_conflict_unit.txt => conflict_unit.txt} | 0 ...e_metrics_all_same.txt => no_conflict.txt} | 0 7 files changed, 88 insertions(+), 65 deletions(-) rename exporters/prometheus/testdata/{duplicate_metrics_conflict_help.txt => conflict_help.txt} (100%) rename exporters/prometheus/testdata/{duplicate_metrics_conflict_type.txt => conflict_type_counter.txt} (100%) create mode 100644 exporters/prometheus/testdata/conflict_type_histogram.txt rename exporters/prometheus/testdata/{duplicate_metrics_conflict_unit.txt => conflict_unit.txt} (100%) rename exporters/prometheus/testdata/{duplicate_metrics_all_same.txt => no_conflict.txt} (100%) diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 8341d2e3054..cbbb2397f8f 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -423,17 +423,48 @@ const ( ) func validateCollision(mf *dto.MetricFamily, mfs map[string]*dto.MetricFamily) collisionType { - emf, exist := mfs[mf.GetName()] - if !exist { - return noCollision + 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 emf.GetType() != mf.GetType() { - return typeCollision + if newNameWithoutSuffix != "" { + if emf, exist = mfs[newNameWithoutSuffix]; exist { + if emf.GetType() == dto.MetricType_HISTOGRAM { + return typeCollision + } + } } - if emf.GetHelp() != mf.GetHelp() { - return helpCollision + 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 3b23dcafea9..f3216ef2189 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -403,7 +403,7 @@ func TestDuplicateMetrics(t *testing.T) { expectedFile string }{ { - name: "all_same", + name: "no_conflict", recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) { fooA, err := meterA.SyncInt64().Counter("foo", instrument.WithUnit(unit.Bytes), @@ -417,7 +417,7 @@ func TestDuplicateMetrics(t *testing.T) { assert.NoError(t, err) fooB.Add(ctx, 100, attribute.String("type", "foo")) }, - expectedFile: "testdata/duplicate_metrics_all_same.txt", + expectedFile: "testdata/no_conflict.txt", }, { name: "conflict_help", @@ -434,7 +434,7 @@ func TestDuplicateMetrics(t *testing.T) { assert.NoError(t, err) fooB.Add(ctx, 100, attribute.String("type", "foo")) }, - expectedFile: "testdata/duplicate_metrics_conflict_help.txt", + expectedFile: "testdata/conflict_help.txt", }, { name: "conflict_unit", @@ -452,10 +452,10 @@ func TestDuplicateMetrics(t *testing.T) { fooB.Add(ctx, 100, attribute.String("type", "foo")) }, options: []Option{WithoutUnits()}, - expectedFile: "testdata/duplicate_metrics_conflict_unit.txt", + expectedFile: "testdata/conflict_unit.txt", }, { - name: "conflict_type", + name: "conflict_type_counter", recordMetrics: func(ctx context.Context, meterA, meterB otelmetric.Meter) { counter, err := meterA.SyncInt64().Counter("foo", instrument.WithUnit(unit.Bytes), @@ -470,7 +470,25 @@ func TestDuplicateMetrics(t *testing.T) { gauge.Add(ctx, 200, attribute.String("type", "foo")) }, options: []Option{WithoutUnits()}, - expectedFile: "testdata/duplicate_metrics_conflict_type.txt", + 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", }, } @@ -511,56 +529,4 @@ func TestDuplicateMetrics(t *testing.T) { require.NoError(t, err) }) } - - // foo, err := provider.Meter("meterfoo", otelmetric.WithInstrumentationVersion("v0.1.0")). - // SyncInt64().Counter( - // "foo", - // instrument.WithUnit(unit.Milliseconds), - // instrument.WithDescription("meter foo")) - // assert.NoError(t, err) - // foo.Add(ctx, 100, attribute.String("type", "foo")) - - // fooWithSameInfo, err := provider.Meter("meterfoo", otelmetric.WithInstrumentationVersion("v0.1.0")). - // SyncInt64().Counter( - // "foo", - // instrument.WithUnit(unit.Milliseconds), - // instrument.WithDescription("meter foo")) - // assert.NoError(t, err) - // fooWithSameInfo.Add(ctx, 100, attribute.String("type", "foo")) - - // families, err := registry.Gather() - // if err != nil { - // t.Fatalf("gather failed:%v", err) - // return - // } else { - // t.Logf("gather succeded\n") - // for i := range families { - // t.Logf("Name:[%v] Help:[%v] Type:[%v], Points:[%v]", *families[i].Name, *families[i].Help, families[i].Type.String(), len(families[i].Metric)) - // } - // } - - // fooWithConflictedType, err := provider.Meter("meterfoo", otelmetric.WithInstrumentationVersion("v0.1.0")). - // SyncInt64().Histogram( - // "foo", - // instrument.WithUnit(unit.Milliseconds), - // instrument.WithDescription("meter foo")) - // require.NoError(t, err) - // fooWithConflictedType.Record(ctx, 1) - // families, err = registry.Gather() - // if err != nil { - // t.Fatalf("gather failed:%v", err) - // return - // } else { - // t.Logf("gather succeded\n") - // for i := range families { - // t.Logf("Name:[%v] Help:[%v] Type:[%v], Points:[%v]", *families[i].Name, *families[i].Help, families[i].Type.String(), len(families[i].Metric)) - // } - // } - - // file, err := os.Open("testdata/multi_scopes.txt") - // 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/duplicate_metrics_conflict_help.txt b/exporters/prometheus/testdata/conflict_help.txt similarity index 100% rename from exporters/prometheus/testdata/duplicate_metrics_conflict_help.txt rename to exporters/prometheus/testdata/conflict_help.txt diff --git a/exporters/prometheus/testdata/duplicate_metrics_conflict_type.txt b/exporters/prometheus/testdata/conflict_type_counter.txt similarity index 100% rename from exporters/prometheus/testdata/duplicate_metrics_conflict_type.txt rename to exporters/prometheus/testdata/conflict_type_counter.txt 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/duplicate_metrics_conflict_unit.txt b/exporters/prometheus/testdata/conflict_unit.txt similarity index 100% rename from exporters/prometheus/testdata/duplicate_metrics_conflict_unit.txt rename to exporters/prometheus/testdata/conflict_unit.txt diff --git a/exporters/prometheus/testdata/duplicate_metrics_all_same.txt b/exporters/prometheus/testdata/no_conflict.txt similarity index 100% rename from exporters/prometheus/testdata/duplicate_metrics_all_same.txt rename to exporters/prometheus/testdata/no_conflict.txt