[connector/spanmetrics] Deprecate the unused configuration DimensionsCacheSize (#39648)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
fix:
#39646

Author: Frapschen

.chloggen/fix-issue-39646.yaml

@@ -0,0 +1,28 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: spanmetricsconnector
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate the unused configuration `dimensions_cache_size`
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [39646]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  Deprecated configuration `dimensions_cache_size`, please use `aggregation_cardinality_limit` instead
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

connector/spanmetricsconnector/README.md

@@ -106,7 +106,7 @@ The following settings can be optionally configured:
 
   If no `default` is provided, this dimension will be **omitted** from the metric.
 - `exclude_dimensions`: the list of dimensions to be excluded from the default set of dimensions. Use to exclude unneeded data from metrics. 
-- `dimensions_cache_size` (default: `1000`): the size of cache for storing Dimensions to improve collectors memory usage. Must be a positive number.
+- `dimensions_cache_size`: this setting is deprecated, please use aggregation_cardinality_limit instead.
 - `include_instrumentation_scope`: a list of instrumentation scope names to include from the traces.
 - `resource_metrics_cache_size` (default: `1000`): the size of the cache holding metrics for a service. This is mostly relevant for
    cumulative temporality to avoid memory leaks and correct metric timestamp resets.

connector/spanmetricsconnector/config.go

@@ -48,6 +48,7 @@ type Config struct {
 	// DimensionsCacheSize defines the size of cache for storing Dimensions, which helps to avoid cache memory growing
 	// indefinitely over the lifetime of the collector.
 	// Optional. See defaultDimensionsCacheSize in connector.go for the default value.
+	// Deprecated:  Please use AggregationCardinalityLimit instead
 	DimensionsCacheSize int `mapstructure:"dimensions_cache_size"`
 
 	// ResourceMetricsCacheSize defines the size of the cache holding metrics for a service. This is mostly relevant for
@@ -133,13 +134,6 @@ func (c Config) Validate() error {
 		return fmt.Errorf("failed validating event dimensions: %w", err)
 	}
 
-	if c.DimensionsCacheSize <= 0 {
-		return fmt.Errorf(
-			"invalid cache size: %v, the maximum number of the items in the cache should be positive",
-			c.DimensionsCacheSize,
-		)
-	}
-
 	if c.Histogram.Explicit != nil && c.Histogram.Exponential != nil {
 		return errors.New("use either `explicit` or `exponential` buckets histogram")
 	}

connector/spanmetricsconnector/config_test.go

@@ -53,7 +53,6 @@ func TestLoadConfig(t *testing.T) {
 					{Name: "http.status_code", Default: (*string)(nil)},
 				},
 				Namespace:                DefaultNamespace,
-				DimensionsCacheSize:      1500,
 				ResourceMetricsCacheSize: 1600,
 				MetricsFlushInterval:     30 * time.Second,
 				Exemplars: ExemplarsConfig{
@@ -76,7 +75,6 @@ func TestLoadConfig(t *testing.T) {
 			expected: &Config{
 				Namespace:                DefaultNamespace,
 				AggregationTemporality:   cumulative,
-				DimensionsCacheSize:      defaultDimensionsCacheSize,
 				ResourceMetricsCacheSize: defaultResourceMetricsCacheSize,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram: HistogramConfig{
@@ -103,7 +101,6 @@ func TestLoadConfig(t *testing.T) {
 			id: component.NewIDWithName(metadata.Type, "exemplars_enabled"),
 			expected: &Config{
 				AggregationTemporality:   "AGGREGATION_TEMPORALITY_CUMULATIVE",
-				DimensionsCacheSize:      defaultDimensionsCacheSize,
 				ResourceMetricsCacheSize: defaultResourceMetricsCacheSize,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram:                HistogramConfig{Disable: false, Unit: defaultUnit},
@@ -115,7 +112,6 @@ func TestLoadConfig(t *testing.T) {
 			id: component.NewIDWithName(metadata.Type, "exemplars_enabled_with_max_per_datapoint"),
 			expected: &Config{
 				AggregationTemporality:   "AGGREGATION_TEMPORALITY_CUMULATIVE",
-				DimensionsCacheSize:      defaultDimensionsCacheSize,
 				ResourceMetricsCacheSize: defaultResourceMetricsCacheSize,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram:                HistogramConfig{Disable: false, Unit: defaultUnit},
@@ -127,7 +123,6 @@ func TestLoadConfig(t *testing.T) {
 			id: component.NewIDWithName(metadata.Type, "resource_metrics_key_attributes"),
 			expected: &Config{
 				AggregationTemporality:       "AGGREGATION_TEMPORALITY_CUMULATIVE",
-				DimensionsCacheSize:          defaultDimensionsCacheSize,
 				ResourceMetricsCacheSize:     defaultResourceMetricsCacheSize,
 				ResourceMetricsKeyAttributes: []string{"service.name", "telemetry.sdk.language", "telemetry.sdk.name"},
 				MetricsFlushInterval:         60 * time.Second,
@@ -140,7 +135,6 @@ func TestLoadConfig(t *testing.T) {
 			expected: &Config{
 				AggregationTemporality:   "AGGREGATION_TEMPORALITY_DELTA",
 				TimestampCacheSize:       &customTimestampCacheSize,
-				DimensionsCacheSize:      defaultDimensionsCacheSize,
 				ResourceMetricsCacheSize: defaultResourceMetricsCacheSize,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram:                HistogramConfig{Disable: false, Unit: defaultUnit},
@@ -151,7 +145,6 @@ func TestLoadConfig(t *testing.T) {
 			id: component.NewIDWithName(metadata.Type, "default_delta_timestamp_cache_size"),
 			expected: &Config{
 				AggregationTemporality:   "AGGREGATION_TEMPORALITY_DELTA",
-				DimensionsCacheSize:      defaultDimensionsCacheSize,
 				ResourceMetricsCacheSize: defaultResourceMetricsCacheSize,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram:                HistogramConfig{Disable: false, Unit: defaultUnit},
@@ -298,7 +291,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "valid config",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram: HistogramConfig{
@@ -308,19 +300,9 @@ func TestConfigValidate(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "invalid dimensions cache size",
-			config: Config{
-				DimensionsCacheSize:      -1,
-				ResourceMetricsCacheSize: 1000,
-				MetricsFlushInterval:     60 * time.Second,
-			},
-			expectedErr: "invalid cache size: -1, the maximum number of the items in the cache should be positive",
-		},
 		{
 			name: "invalid metrics flush interval",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     -1 * time.Second,
 			},
@@ -329,7 +311,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "invalid metrics expiration",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     60 * time.Second,
 				MetricsExpiration:        -1 * time.Second,
@@ -339,7 +320,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "invalid delta timestamp cache size",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     60 * time.Second,
 				AggregationTemporality:   delta,
@@ -350,7 +330,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "invalid aggregation cardinality limit",
 			config: Config{
-				DimensionsCacheSize:         1000,
 				ResourceMetricsCacheSize:    1000,
 				MetricsFlushInterval:        60 * time.Second,
 				AggregationCardinalityLimit: -1,
@@ -360,7 +339,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "both explicit and exponential histogram",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     60 * time.Second,
 				Histogram: HistogramConfig{
@@ -377,7 +355,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "duplicate dimension name",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     60 * time.Second,
 				Dimensions: []Dimension{
@@ -389,7 +366,6 @@ func TestConfigValidate(t *testing.T) {
 		{
 			name: "events enabled with no dimensions",
 			config: Config{
-				DimensionsCacheSize:      1000,
 				ResourceMetricsCacheSize: 1000,
 				MetricsFlushInterval:     60 * time.Second,
 				Events: EventsConfig{

connector/spanmetricsconnector/connector.go

@@ -36,7 +36,6 @@ const (
 	instrumentationScopeVersionKey = "span.instrumentation.scope.version" // OpenTelemetry non-standard constant.
 	metricKeySeparator             = string(byte(0))
 
-	defaultDimensionsCacheSize      = 1000
 	defaultResourceMetricsCacheSize = 1000
 
 	metricNameDuration = "duration"
@@ -65,10 +64,6 @@ type connectorImp struct {
 
 	keyBuf *bytes.Buffer
 
-	// An LRU cache of dimension key-value maps keyed by a unique identifier formed by a concatenation of its values:
-	// e.g. { "foo/barOK": { "serviceName": "foo", "span.name": "/bar", "status_code": "OK" }}
-	metricKeyToDimensions *cache.Cache[metrics.Key, pcommon.Map]
-
 	clock   clockwork.Clock
 	ticker  clockwork.Ticker
 	done    chan struct{}
@@ -112,10 +107,8 @@ func newDimensions(cfgDims []Dimension) []utilattri.Dimension {
 func newConnector(logger *zap.Logger, config component.Config, clock clockwork.Clock) (*connectorImp, error) {
 	logger.Info("Building spanmetrics connector")
 	cfg := config.(*Config)
-
-	metricKeyToDimensionsCache, err := cache.NewCache[metrics.Key, pcommon.Map](cfg.DimensionsCacheSize)
-	if err != nil {
-		return nil, err
+	if cfg.DimensionsCacheSize != 0 {
+		logger.Warn("DimensionsCacheSize is deprecated, please use AggregationCardinalityLimit instead.")
 	}
 
 	resourceMetricsCache, err := cache.NewCache[resourceKey, *resourceMetrics](cfg.ResourceMetricsCacheSize)
@@ -146,7 +139,6 @@ func newConnector(logger *zap.Logger, config component.Config, clock clockwork.C
 		resourceMetricsKeyAttributes: resourceMetricsKeyAttributes,
 		dimensions:                   newDimensions(cfg.Dimensions),
 		keyBuf:                       bytes.NewBuffer(make([]byte, 0, 1024)),
-		metricKeyToDimensions:        metricKeyToDimensionsCache,
 		lastDeltaTimestamps:          lastDeltaTimestamps,
 		clock:                        clock,
 		ticker:                       clock.NewTicker(cfg.MetricsFlushInterval),
@@ -331,10 +323,8 @@ func (p *connectorImp) resetState() {
 	// If delta metrics, reset accumulated data
 	if p.config.GetAggregationTemporality() == pmetric.AggregationTemporalityDelta {
 		p.resourceMetrics.Purge()
-		p.metricKeyToDimensions.Purge()
 	} else {
 		p.resourceMetrics.RemoveEvictedItems()
-		p.metricKeyToDimensions.RemoveEvictedItems()
 
 		// If none of these features are enabled then we can skip the remaining operations.
 		// Enabling either of these features requires to go over resource metrics and do operation on each.
@@ -399,43 +389,45 @@ func (p *connectorImp) aggregateMetrics(traces ptrace.Traces) {
 				if endTime > startTime {
 					duration = float64(endTime-startTime) / float64(unitDivider)
 				}
-				key := p.buildKey(serviceName, span, p.dimensions, resourceAttr)
 
-				var attributes pcommon.Map
+				key := p.buildKey(serviceName, span, p.dimensions, resourceAttr)
+				var attributesFun metrics.BuildAttributesFun
 
 				// Note: we check cardinality limit here for sums metrics but it is the same
 				// for histograms because both use the same key and attributes.
 				if rm.sums.IsCardinalityLimitReached() {
-					attributes = pcommon.NewMap()
-					for _, d := range p.dimensions {
-						if v, exists := utilattri.GetDimensionValue(d, span.Attributes(), resourceAttr); exists {
-							v.CopyTo(attributes.PutEmpty(d.Name))
+					attributesFun = func() pcommon.Map {
+						attributes := pcommon.NewMap()
+						for _, d := range p.dimensions {
+							if v, exists := utilattri.GetDimensionValue(d, span.Attributes(), resourceAttr); exists {
+								v.CopyTo(attributes.PutEmpty(d.Name))
+							}
 						}
+						attributes.PutBool(overflowKey, true)
+
+						return attributes
 					}
-					attributes.PutBool(overflowKey, true)
 				} else {
-					var cached bool
-					attributes, cached = p.metricKeyToDimensions.Get(key)
-					if !cached {
-						attributes = p.buildAttributes(
+					attributesFun = func() pcommon.Map {
+						attributes := p.buildAttributes(
 							serviceName,
 							span,
 							resourceAttr,
 							p.dimensions,
 							ils.Scope(),
 						)
-						p.metricKeyToDimensions.Add(key, attributes)
+						return attributes
 					}
 				}
 
 				if !p.config.Histogram.Disable {
 					// aggregate histogram metrics
-					h := histograms.GetOrCreate(key, attributes, startTimestamp)
+					h := histograms.GetOrCreate(key, attributesFun, startTimestamp)
 					p.addExemplar(span, duration, h)
 					h.Observe(duration)
 				}
 				// aggregate sums metrics
-				s := sums.GetOrCreate(key, attributes, startTimestamp)
+				s := sums.GetOrCreate(key, attributesFun, startTimestamp)
 				if p.config.Exemplars.Enabled && !span.TraceID().IsEmpty() {
 					s.AddExemplar(span.TraceID(), span.SpanID(), duration)
 				}
@@ -459,20 +451,20 @@ func (p *connectorImp) aggregateMetrics(traces ptrace.Traces) {
 						})
 
 						eKey := p.buildKey(serviceName, span, eDimensions, rscAndEventAttrs)
-
-						var eAttributes pcommon.Map
 						if rm.events.IsCardinalityLimitReached() {
-							eAttributes = pcommon.NewMap()
-							rscAndEventAttrs.CopyTo(eAttributes)
-							eAttributes.PutBool(overflowKey, true)
+							attributesFun = func() pcommon.Map {
+								attributes := pcommon.NewMap()
+								rscAndEventAttrs.CopyTo(attributes)
+								attributes.PutBool(overflowKey, true)
+
+								return attributes
+							}
 						} else {
-							eAttributes, ok = p.metricKeyToDimensions.Get(eKey)
-							if !ok {
-								eAttributes = p.buildAttributes(serviceName, span, rscAndEventAttrs, eDimensions, ils.Scope())
-								p.metricKeyToDimensions.Add(eKey, eAttributes)
+							attributesFun = func() pcommon.Map {
+								return p.buildAttributes(serviceName, span, rscAndEventAttrs, eDimensions, ils.Scope())
 							}
 						}
-						e := events.GetOrCreate(eKey, eAttributes, startTimestamp)
+						e := events.GetOrCreate(eKey, attributesFun, startTimestamp)
 						if p.config.Exemplars.Enabled && !span.TraceID().IsEmpty() {
 							e.AddExemplar(span.TraceID(), span.SpanID(), duration)
 						}

connector/spanmetricsconnector/connector_test.go

@@ -46,7 +46,6 @@ const (
 	notInSpanAttrName1       = "shouldNotBeInMetric"
 	regionResourceAttrName   = "region"
 	exceptionTypeAttrName    = "exception.type"
-	dimensionsCacheSize      = 2
 	resourceMetricsCacheSize = 5
 
 	sampleRegion   = "us-east-1"
@@ -465,7 +464,6 @@ func newConnectorImp(defaultNullValue *string, histogramConfig func() HistogramC
 		Histogram:                    histogramConfig(),
 		Exemplars:                    exemplarsConfig(),
 		ExcludeDimensions:            excludedDimensions,
-		DimensionsCacheSize:          dimensionsCacheSize,
 		ResourceMetricsCacheSize:     resourceMetricsCacheSize,
 		ResourceMetricsKeyAttributes: resourceMetricsKeyAttributes,
 		Dimensions: []Dimension{
@@ -914,35 +912,6 @@ func TestConsumeTraces(t *testing.T) {
 	}
 }
 
-func TestMetricKeyCache(t *testing.T) {
-	p, err := newConnectorImp(stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, disabledEventsConfig, cumulative, 0, []string{}, 1000, clockwork.NewFakeClock())
-	require.NoError(t, err)
-	traces := buildSampleTrace()
-
-	// Test
-	ctx := metadata.NewIncomingContext(context.Background(), nil)
-
-	// 0 key was cached at beginning
-	assert.Zero(t, p.metricKeyToDimensions.Len())
-
-	err = p.ConsumeTraces(ctx, traces)
-	// Validate
-	require.NoError(t, err)
-	// 2 key was cached, 1 key was evicted and cleaned after the processing
-	assert.Eventually(t, func() bool {
-		return p.metricKeyToDimensions.Len() == dimensionsCacheSize
-	}, 10*time.Second, time.Millisecond*100)
-
-	// consume another batch of traces
-	err = p.ConsumeTraces(ctx, traces)
-	require.NoError(t, err)
-
-	// 2 key was cached, other keys were evicted and cleaned after the processing
-	assert.Eventually(t, func() bool {
-		return p.metricKeyToDimensions.Len() == dimensionsCacheSize
-	}, 10*time.Second, time.Millisecond*100)
-}
-
 func TestResourceMetricsCache(t *testing.T) {
 	p, err := newConnectorImp(stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, disabledEventsConfig, cumulative, 0, []string{}, 1000, clockwork.NewFakeClock())
 	require.NoError(t, err)

connector/spanmetricsconnector/factory.go

@@ -47,7 +47,6 @@ func NewFactory() connector.Factory {
 func createDefaultConfig() component.Config {
 	return &Config{
 		AggregationTemporality:      "AGGREGATION_TEMPORALITY_CUMULATIVE",
-		DimensionsCacheSize:         defaultDimensionsCacheSize,
 		ResourceMetricsCacheSize:    defaultResourceMetricsCacheSize,
 		MetricsFlushInterval:        60 * time.Second,
 		Histogram:                   HistogramConfig{Disable: false, Unit: defaultUnit},