Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processor / spanmetricsprocessor] dropping the condition to replace _ with key_ as __ is reserved and _ is not #8057

Merged
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@
- `resourcedetectionprocessor`: add the [consul](https://www.consul.io/) detector (#6382)
- `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)
- `spanmetricsprocessor`: Dropping the condition to replace _ with key_ as __ label is reserved and _ is not (#7506)
saikrishna397 marked this conversation as resolved.
Show resolved Hide resolved


### 🛑 Breaking changes 🛑
Expand Down
10 changes: 10 additions & 0 deletions processor/spanmetricsprocessor/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/model/pdata"
"go.opentelemetry.io/collector/service/featuregate"
)

const (
Expand Down Expand Up @@ -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: "exporter.prometheus.PermissiveLabelSanitization",
saikrishna397 marked this conversation as resolved.
Show resolved Hide resolved
Enabled: false,
Description: "Controls whether to change labels starting with '_' to 'key_'",
}

// GetAggregationTemporality converts the string value given in the config into a MetricAggregationTemporality.
Expand Down
2 changes: 2 additions & 0 deletions processor/spanmetricsprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/processor/processorhelper"
"go.opentelemetry.io/collector/service/featuregate"
)

const (
Expand All @@ -42,6 +43,7 @@ func createDefaultConfig() config.Processor {
ProcessorSettings: config.NewProcessorSettings(config.NewComponentID(typeStr)),
AggregationTemporality: "AGGREGATION_TEMPORALITY_CUMULATIVE",
DimensionsCacheSize: defaultDimensionsCacheSize,
skipSanitizeLabel: featuregate.IsEnabled(dropSanitizationGate.ID),
}
}

Expand Down
17 changes: 11 additions & 6 deletions processor/spanmetricsprocessor/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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{}{}

Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
30 changes: 21 additions & 9 deletions processor/spanmetricsprocessor/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down