From a97859957ccaca3406d55b444be127a5c0e5672c Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Thu, 21 Nov 2024 19:50:39 +0000 Subject: [PATCH] sanitize exemplar keys --- CHANGELOG.md | 1 + exporters/prometheus/exporter.go | 3 +- exporters/prometheus/exporter_test.go | 82 +++++++++++++++++++++------ 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 774cbc1c3d1..ed304721b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5954) - Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5954) - Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5954) +- Fix invalid exemplar keys in `go.opentelemetry.io/otel/exporters/prometheus`. (#TODO) diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 3cd457a03c3..50c95a16f7f 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -547,7 +547,8 @@ func addExemplars[N int64 | float64](m prometheus.Metric, exemplars []metricdata func attributesToLabels(attrs []attribute.KeyValue) prometheus.Labels { labels := make(map[string]string) for _, attr := range attrs { - labels[string(attr.Key)] = attr.Value.Emit() + key := model.EscapeName(string(attr.Key), model.NameEscapingScheme) + labels[key] = attr.Value.Emit() } return labels } diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index 5592071d95b..e06824a316d 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -949,37 +949,93 @@ func TestShutdownExporter(t *testing.T) { func TestExemplars(t *testing.T) { attrsOpt := otelmetric.WithAttributes( - attribute.Key("A").String("B"), - attribute.Key("C").String("D"), - attribute.Key("E").Bool(true), - attribute.Key("F").Int(42), + attribute.Key("A.1").String("B"), + attribute.Key("C.2").String("D"), + attribute.Key("E.3").Bool(true), + attribute.Key("F.4").Int(42), ) + expectedUnsanitizedLabels := map[string]string{ + traceIDExemplarKey: "01000000000000000000000000000000", + spanIDExemplarKey: "0100000000000000", + "A.1": "B", + "C.2": "D", + "E.3": "true", + "F.4": "42", + } + expectedSanitizedLabels := map[string]string{ + traceIDExemplarKey: "01000000000000000000000000000000", + spanIDExemplarKey: "0100000000000000", + "A_1": "B", + "C_2": "D", + "E_3": "true", + "F_4": "42", + } for _, tc := range []struct { name string recordMetrics func(ctx context.Context, meter otelmetric.Meter) expectedExemplarValue float64 + expectedLabels map[string]string + escapingScheme model.EscapingScheme + validationScheme model.ValidationScheme }{ { - name: "counter", + name: "sanitized counter", recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { counter, err := meter.Float64Counter("foo") require.NoError(t, err) counter.Add(ctx, 9, attrsOpt) }, expectedExemplarValue: 9, + expectedLabels: expectedSanitizedLabels, + escapingScheme: model.UnderscoreEscaping, + validationScheme: model.LegacyValidation, }, { - name: "histogram", + name: "sanitized histogram", recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { hist, err := meter.Int64Histogram("foo") require.NoError(t, err) hist.Record(ctx, 9, attrsOpt) }, expectedExemplarValue: 9, + expectedLabels: expectedSanitizedLabels, + escapingScheme: model.UnderscoreEscaping, + validationScheme: model.LegacyValidation, + }, + { + name: "unsanitized counter", + recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { + counter, err := meter.Float64Counter("foo") + require.NoError(t, err) + counter.Add(ctx, 9, attrsOpt) + }, + expectedExemplarValue: 9, + expectedLabels: expectedUnsanitizedLabels, + escapingScheme: model.NoEscaping, + validationScheme: model.UTF8Validation, + }, + { + name: "unsanitized histogram", + recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { + hist, err := meter.Int64Histogram("foo") + require.NoError(t, err) + hist.Record(ctx, 9, attrsOpt) + }, + expectedExemplarValue: 9, + expectedLabels: expectedUnsanitizedLabels, + escapingScheme: model.NoEscaping, + validationScheme: model.UTF8Validation, }, } { t.Run(tc.name, func(t *testing.T) { - t.Setenv("OTEL_GO_X_EXEMPLAR", "true") + originalEscapingScheme := model.NameEscapingScheme + originalValidationScheme := model.NameValidationScheme + model.NameEscapingScheme = tc.escapingScheme + model.NameValidationScheme = tc.validationScheme + defer func() { + model.NameEscapingScheme = originalEscapingScheme + model.NameValidationScheme = originalValidationScheme + }() // initialize registry exporter ctx := context.Background() registry := prometheus.NewRegistry() @@ -1044,17 +1100,9 @@ func TestExemplars(t *testing.T) { } require.NotNil(t, exemplar) require.Equal(t, tc.expectedExemplarValue, exemplar.GetValue()) - expectedLabels := map[string]string{ - traceIDExemplarKey: "01000000000000000000000000000000", - spanIDExemplarKey: "0100000000000000", - "A": "B", - "C": "D", - "E": "true", - "F": "42", - } - require.Equal(t, len(expectedLabels), len(exemplar.GetLabel())) + require.Equal(t, len(tc.expectedLabels), len(exemplar.GetLabel())) for _, label := range exemplar.GetLabel() { - val, ok := expectedLabels[label.GetName()] + val, ok := tc.expectedLabels[label.GetName()] require.True(t, ok) require.Equal(t, label.GetValue(), val) }