From 32430b4d41aebeefcbe3968c812cd8edb5df5690 Mon Sep 17 00:00:00 2001 From: Sai Krishna Reddy Syamala Date: Tue, 8 Mar 2022 06:26:51 -0600 Subject: [PATCH] [processor/spanmetrics] dropping the condition to replace _ with key_ as __ is reserved and _ is not (#8057) * dropping the condition to replace _ with key_ as __ is reserved and _ is not * featureGate changes for sanitizing labels that start with `_` featureGate changes for sanitizing labels that start with `_` * changelog update changelog update * updated the feature gate identifier updated the feature gate identifier * changelog update changelog update * dropped processor helper import in factory.go dropped processor helper import in factory.go * Update CHANGELOG.md Co-authored-by: Alex Boten --- CHANGELOG.md | 2 +- processor/spanmetricsprocessor/config.go | 10 +++++++ processor/spanmetricsprocessor/factory.go | 2 ++ processor/spanmetricsprocessor/processor.go | 17 +++++++---- .../spanmetricsprocessor/processor_test.go | 30 +++++++++++++------ 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d6702eb9350..6a53e54ad5a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - `hostreceiver/filesystemscraper`: Add filesystem utilization (#8027) - `googlecloudexporter`: [Alpha] Translate metrics directly from OTLP to gcm using the `exporter.googlecloud.OTLPDirect` feature-gate (#7177) - `simpleprometheusreceiver`: Add support for static labels (#7908) +- `spanmetricsprocessor`: Dropping the condition to replace _ with key_ as __ label is reserved and _ is not (#8057) - `podmanreceiver`: Add container.runtime attribute to container metrics (#8262) - `dockerstatsreceiver`: Add container.runtime attribute to container metrics (#8261) @@ -271,7 +272,6 @@ - `awsemfexporter`: refactor cw_client logic into separate `cwlogs` package (#7072) - `prometheusexporter`: Dropping the condition to replace _ with key_ as __ label is reserved and _ is not (#7506) - ### 🛑 Breaking changes 🛑 - `memcachedreceiver`: Update metric names (#6594) diff --git a/processor/spanmetricsprocessor/config.go b/processor/spanmetricsprocessor/config.go index dcf264077735..92ad9d4b8a13 100644 --- a/processor/spanmetricsprocessor/config.go +++ b/processor/spanmetricsprocessor/config.go @@ -19,6 +19,7 @@ import ( "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/model/pdata" + "go.opentelemetry.io/collector/service/featuregate" ) const ( @@ -58,6 +59,15 @@ type Config struct { DimensionsCacheSize int `mapstructure:"dimensions_cache_size"` AggregationTemporality string `mapstructure:"aggregation_temporality"` + + // skipSanitizeLabel if enabled, labels that start with _ are not sanitized + skipSanitizeLabel bool +} + +var dropSanitizationGate = featuregate.Gate{ + ID: "processor.spanmetrics.PermissiveLabelSanitization", + Enabled: false, + Description: "Controls whether to change labels starting with '_' to 'key_'", } // GetAggregationTemporality converts the string value given in the config into a MetricAggregationTemporality. diff --git a/processor/spanmetricsprocessor/factory.go b/processor/spanmetricsprocessor/factory.go index bfa7ea486063..8316bd42bb34 100644 --- a/processor/spanmetricsprocessor/factory.go +++ b/processor/spanmetricsprocessor/factory.go @@ -20,6 +20,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/consumer" + "go.opentelemetry.io/collector/service/featuregate" ) const ( @@ -41,6 +42,7 @@ func createDefaultConfig() config.Processor { ProcessorSettings: config.NewProcessorSettings(config.NewComponentID(typeStr)), AggregationTemporality: "AGGREGATION_TEMPORALITY_CUMULATIVE", DimensionsCacheSize: defaultDimensionsCacheSize, + skipSanitizeLabel: featuregate.IsEnabled(dropSanitizationGate.ID), } } diff --git a/processor/spanmetricsprocessor/processor.go b/processor/spanmetricsprocessor/processor.go index 68ae9acd23fd..3500986d92d3 100644 --- a/processor/spanmetricsprocessor/processor.go +++ b/processor/spanmetricsprocessor/processor.go @@ -104,7 +104,7 @@ func newProcessor(logger *zap.Logger, config config.Processor, nextConsumer cons } } - if err := validateDimensions(pConfig.Dimensions); err != nil { + if err := validateDimensions(pConfig.Dimensions, pConfig.skipSanitizeLabel); err != nil { return nil, err } @@ -151,11 +151,11 @@ func mapDurationsToMillis(vs []time.Duration) []float64 { // validateDimensions checks duplicates for reserved dimensions and additional dimensions. Considering // the usage of Prometheus related exporters, we also validate the dimensions after sanitization. -func validateDimensions(dimensions []Dimension) error { +func validateDimensions(dimensions []Dimension, skipSanitizeLabel bool) error { labelNames := make(map[string]struct{}) for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey} { labelNames[key] = struct{}{} - labelNames[sanitize(key)] = struct{}{} + labelNames[sanitize(key, skipSanitizeLabel)] = struct{}{} } labelNames[operationKey] = struct{}{} @@ -165,7 +165,7 @@ func validateDimensions(dimensions []Dimension) error { } labelNames[key.Name] = struct{}{} - sanitizedName := sanitize(key.Name) + sanitizedName := sanitize(key.Name, skipSanitizeLabel) if sanitizedName == key.Name { continue } @@ -515,7 +515,7 @@ func (p *processorImp) cache(serviceName string, span pdata.Span, k metricKey, r // copied from prometheus-go-metric-exporter // sanitize replaces non-alphanumeric characters with underscores in s. -func sanitize(s string) string { +func sanitize(s string, skipSanitizeLabel bool) string { if len(s) == 0 { return s } @@ -527,7 +527,12 @@ func sanitize(s string) string { if unicode.IsDigit(rune(s[0])) { s = "key_" + s } - if s[0] == '_' { + //replace labels starting with _ only when skipSanitizeLabel is disabled + if !skipSanitizeLabel && strings.HasPrefix(s, "_") { + s = "key" + s + } + //labels starting with __ are reserved in prometheus + if strings.HasPrefix(s, "__") { s = "key" + s } return s diff --git a/processor/spanmetricsprocessor/processor_test.go b/processor/spanmetricsprocessor/processor_test.go index d51218c4f31c..56898772bc28 100644 --- a/processor/spanmetricsprocessor/processor_test.go +++ b/processor/spanmetricsprocessor/processor_test.go @@ -698,9 +698,10 @@ func TestProcessorDuplicateDimensions(t *testing.T) { func TestValidateDimensions(t *testing.T) { for _, tc := range []struct { - name string - dimensions []Dimension - expectedErr string + name string + dimensions []Dimension + expectedErr string + skipSanitizeLabel bool }{ { name: "no additional dimensions", @@ -751,7 +752,8 @@ func TestValidateDimensions(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - err := validateDimensions(tc.dimensions) + tc.skipSanitizeLabel = false + err := validateDimensions(tc.dimensions, tc.skipSanitizeLabel) if tc.expectedErr != "" { assert.EqualError(t, err, tc.expectedErr) } else { @@ -762,11 +764,21 @@ func TestValidateDimensions(t *testing.T) { } func TestSanitize(t *testing.T) { - require.Equal(t, "", sanitize(""), "") - require.Equal(t, "key_test", sanitize("_test")) - require.Equal(t, "key_0test", sanitize("0test")) - require.Equal(t, "test", sanitize("test")) - require.Equal(t, "test__", sanitize("test_/")) + cfg := createDefaultConfig().(*Config) + require.Equal(t, "", sanitize("", cfg.skipSanitizeLabel), "") + require.Equal(t, "key_test", sanitize("_test", cfg.skipSanitizeLabel)) + require.Equal(t, "key__test", sanitize("__test", cfg.skipSanitizeLabel)) + require.Equal(t, "key_0test", sanitize("0test", cfg.skipSanitizeLabel)) + require.Equal(t, "test", sanitize("test", cfg.skipSanitizeLabel)) + require.Equal(t, "test__", sanitize("test_/", cfg.skipSanitizeLabel)) + //testcases with skipSanitizeLabel flag turned on + cfg.skipSanitizeLabel = true + require.Equal(t, "", sanitize("", cfg.skipSanitizeLabel), "") + require.Equal(t, "_test", sanitize("_test", cfg.skipSanitizeLabel)) + require.Equal(t, "key__test", sanitize("__test", cfg.skipSanitizeLabel)) + require.Equal(t, "key_0test", sanitize("0test", cfg.skipSanitizeLabel)) + require.Equal(t, "test", sanitize("test", cfg.skipSanitizeLabel)) + require.Equal(t, "test__", sanitize("test_/", cfg.skipSanitizeLabel)) } func TestSetLatencyExemplars(t *testing.T) {