From 22543ba566cd9c5050857acfe4095b6f0ebda374 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Thu, 30 Mar 2023 15:30:16 +0300 Subject: [PATCH 1/2] [connector/spanmetricsconnector] fix default metrics initialization --- connector/spanmetricsconnector/README.md | 1 + connector/spanmetricsconnector/config.go | 20 +++++++++---- connector/spanmetricsconnector/config_test.go | 28 ++++++++++++++++--- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 6e98f603b97b..512c98d158d0 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -74,6 +74,7 @@ The following settings can be optionally configured: buckets: `[2ms, 4ms, 6ms, 8ms, 10ms, 50ms, 100ms, 200ms, 400ms, 800ms, 1s, 1400ms, 2s, 5s, 10s, 15s]` - `exponential`: - `max_size` (default: 160) the maximum number of buckets per positive or negative number range. +- `override_default_dimensions`: the list of dimensions which will override all default dimensions defined above. - `dimensions`: the list of dimensions to add together with the default dimensions defined above. Each additional dimension is defined with a `name` which is looked up in the span's collection of attributes or diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index 6a988f8e77d6..727197da5605 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -49,7 +49,8 @@ type Config struct { // - status.code // The dimensions will be fetched from the span's attributes. Examples of some conventionally used attributes: // https://github.com/open-telemetry/opentelemetry-collector/blob/main/model/semconv/opentelemetry.go. - Dimensions []Dimension `mapstructure:"dimensions"` + Dimensions []Dimension `mapstructure:"dimensions"` + OverrideDefaultDimensions []Dimension `mapstructure:"override_default_dimensions"` // DimensionsCacheSize defines the size of cache for storing Dimensions, which helps to avoid cache memory growing // indefinitely over the lifetime of the collector. @@ -86,7 +87,7 @@ var _ component.ConfigValidator = (*Config)(nil) // Validate checks if the processor configuration is valid func (c Config) Validate() error { - err := validateDimensions(c.Dimensions) + err := validateDimensions(c.Dimensions, c.OverrideDefaultDimensions) if err != nil { return err } @@ -114,10 +115,19 @@ func (c Config) GetAggregationTemporality() pmetric.AggregationTemporality { } // validateDimensions checks duplicates for reserved dimensions and additional dimensions. -func validateDimensions(dimensions []Dimension) error { +func validateDimensions(dimensions []Dimension, overrideDefaultDimensions []Dimension) error { labelNames := make(map[string]struct{}) - for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} { - labelNames[key] = struct{}{} + if len(overrideDefaultDimensions) > 0 { + for _, key := range overrideDefaultDimensions { + if _, ok := labelNames[key.Name]; ok { + return fmt.Errorf("duplicate dimension name %s", key.Name) + } + labelNames[key.Name] = struct{}{} + } + } else { + for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} { + labelNames[key] = struct{}{} + } } for _, key := range dimensions { diff --git a/connector/spanmetricsconnector/config_test.go b/connector/spanmetricsconnector/config_test.go index b61a2a260386..d1f4675b552d 100644 --- a/connector/spanmetricsconnector/config_test.go +++ b/connector/spanmetricsconnector/config_test.go @@ -128,9 +128,10 @@ func TestGetAggregationTemporality(t *testing.T) { func TestValidateDimensions(t *testing.T) { for _, tc := range []struct { - name string - dimensions []Dimension - expectedErr string + name string + dimensions []Dimension + overrideDefaultDimensions []Dimension + expectedErr string }{ { name: "no additional dimensions", @@ -158,9 +159,28 @@ func TestValidateDimensions(t *testing.T) { }, expectedErr: "duplicate dimension name service_name", }, + { + name: "no duplicate dimensions in overrideDefaultDimensions and dimensions", + dimensions: []Dimension{ + {Name: "service_name"}, + }, + overrideDefaultDimensions: []Dimension{ + {Name: "http.service"}, + }, + }, + { + name: "duplicate dimensions in overrideDefaultDimensions and dimensions", + dimensions: []Dimension{ + {Name: "service_name"}, + }, + overrideDefaultDimensions: []Dimension{ + {Name: "service_name"}, + }, + expectedErr: "duplicate dimension name service_name", + }, } { t.Run(tc.name, func(t *testing.T) { - err := validateDimensions(tc.dimensions) + err := validateDimensions(tc.dimensions, tc.overrideDefaultDimensions) if tc.expectedErr != "" { assert.EqualError(t, err, tc.expectedErr) } else { From 9998fe22998b6e4f5c8c13178fdd57deb0662999 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Thu, 30 Mar 2023 18:31:32 +0300 Subject: [PATCH 2/2] [gh-16344] Renamed from overrideDefaultDimensions to deleteKeys --- connector/spanmetricsconnector/README.md | 2 +- connector/spanmetricsconnector/config.go | 30 +++++++++++-------- connector/spanmetricsconnector/config_test.go | 19 ++++++------ 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 512c98d158d0..ff4a3fc5c0bb 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -74,7 +74,7 @@ The following settings can be optionally configured: buckets: `[2ms, 4ms, 6ms, 8ms, 10ms, 50ms, 100ms, 200ms, 400ms, 800ms, 1s, 1400ms, 2s, 5s, 10s, 15s]` - `exponential`: - `max_size` (default: 160) the maximum number of buckets per positive or negative number range. -- `override_default_dimensions`: the list of dimensions which will override all default dimensions defined above. +- `delete_keys`: the list of dimensions which will be removed from default dimensions defined above. - `dimensions`: the list of dimensions to add together with the default dimensions defined above. Each additional dimension is defined with a `name` which is looked up in the span's collection of attributes or diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index 727197da5605..ca13e27a07e9 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -35,6 +35,10 @@ var defaultHistogramBucketsMs = []float64{ } // Dimension defines the dimension name and optional default value if the Dimension is missing from a span attribute. + +type Keys struct { + Name string `mapstructure:"name"` +} type Dimension struct { Name string `mapstructure:"name"` Default *string `mapstructure:"default"` @@ -49,8 +53,8 @@ type Config struct { // - status.code // The dimensions will be fetched from the span's attributes. Examples of some conventionally used attributes: // https://github.com/open-telemetry/opentelemetry-collector/blob/main/model/semconv/opentelemetry.go. - Dimensions []Dimension `mapstructure:"dimensions"` - OverrideDefaultDimensions []Dimension `mapstructure:"override_default_dimensions"` + Dimensions []Dimension `mapstructure:"dimensions"` + DeleteKeys []Keys `mapstructure:"delete_keys"` // DimensionsCacheSize defines the size of cache for storing Dimensions, which helps to avoid cache memory growing // indefinitely over the lifetime of the collector. @@ -87,7 +91,7 @@ var _ component.ConfigValidator = (*Config)(nil) // Validate checks if the processor configuration is valid func (c Config) Validate() error { - err := validateDimensions(c.Dimensions, c.OverrideDefaultDimensions) + err := validateDimensions(c.Dimensions, c.DeleteKeys) if err != nil { return err } @@ -115,17 +119,17 @@ func (c Config) GetAggregationTemporality() pmetric.AggregationTemporality { } // validateDimensions checks duplicates for reserved dimensions and additional dimensions. -func validateDimensions(dimensions []Dimension, overrideDefaultDimensions []Dimension) error { +func validateDimensions(dimensions []Dimension, deleteKeys []Keys) error { labelNames := make(map[string]struct{}) - if len(overrideDefaultDimensions) > 0 { - for _, key := range overrideDefaultDimensions { - if _, ok := labelNames[key.Name]; ok { - return fmt.Errorf("duplicate dimension name %s", key.Name) - } - labelNames[key.Name] = struct{}{} - } - } else { - for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} { + + // Making map of elements for o(1) complexity + set := make(map[string]struct{}) + for _, item := range deleteKeys { + set[item.Name] = struct{}{} + } + + for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} { + if _, exists := set[key]; !exists { labelNames[key] = struct{}{} } } diff --git a/connector/spanmetricsconnector/config_test.go b/connector/spanmetricsconnector/config_test.go index d1f4675b552d..12d4a3a8f04b 100644 --- a/connector/spanmetricsconnector/config_test.go +++ b/connector/spanmetricsconnector/config_test.go @@ -128,10 +128,10 @@ func TestGetAggregationTemporality(t *testing.T) { func TestValidateDimensions(t *testing.T) { for _, tc := range []struct { - name string - dimensions []Dimension - overrideDefaultDimensions []Dimension - expectedErr string + name string + dimensions []Dimension + deleteKeys []Keys + expectedErr string }{ { name: "no additional dimensions", @@ -160,27 +160,26 @@ func TestValidateDimensions(t *testing.T) { expectedErr: "duplicate dimension name service_name", }, { - name: "no duplicate dimensions in overrideDefaultDimensions and dimensions", + name: "Wrong key name to delete", dimensions: []Dimension{ {Name: "service_name"}, }, - overrideDefaultDimensions: []Dimension{ + deleteKeys: []Keys{ {Name: "http.service"}, }, }, { - name: "duplicate dimensions in overrideDefaultDimensions and dimensions", + name: "Correct key to delete", dimensions: []Dimension{ {Name: "service_name"}, }, - overrideDefaultDimensions: []Dimension{ + deleteKeys: []Keys{ {Name: "service_name"}, }, - expectedErr: "duplicate dimension name service_name", }, } { t.Run(tc.name, func(t *testing.T) { - err := validateDimensions(tc.dimensions, tc.overrideDefaultDimensions) + err := validateDimensions(tc.dimensions, tc.deleteKeys) if tc.expectedErr != "" { assert.EqualError(t, err, tc.expectedErr) } else {