[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 <[email protected]>
open-telemetry · Mar 8, 2022 · 32430b4 · 32430b4
1 parent 64a87c1
commit 32430b4
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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) {