Merge the processors overrides set through runtime overrides and user…

…-configurable overrides (grafana#3125)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * [ENHANCEMENT] Introduced `AttributePolicyMatch` & `IntrinsicPolicyMatch` structures to match span attributes based on strongly typed values & precompiled regexp [#3025](https://github.com/grafana/tempo/pull/3025) (@andriusluk)
 * [CHANGE] TraceQL/Structural operators performance improvement. [#3088](https://github.com/grafana/tempo/pull/3088) (@joe-elliott)
+* [CHANGE] Merge the processors overrides set through runtime overrides and user-configurable overrides [#3125](https://github.com/grafana/tempo/pull/3125) (@kvrhdn)
 * [FEATURE] Introduce list_blocks_concurrency on GCS and S3 backends to control backend load and performance. [#2652](https://github.com/grafana/tempo/pull/2652) (@zalegrala)
 * [BUGFIX] Include statusMessage intrinsic attribute in tag search. [#3084](https://github.com/grafana/tempo/pull/3084) (@rcrowe)
 * [ENHANCEMENT] Update poller to make use of previous results and reduce backend load. [#2652](https://github.com/grafana/tempo/pull/2652) (@zalegrala)

modules/overrides/user_configurable_overrides.go

@@ -21,6 +21,7 @@ import (
 
 	userconfigurableoverrides "github.com/grafana/tempo/modules/overrides/userconfigurable/client"
 	filterconfig "github.com/grafana/tempo/pkg/spanfilter/config"
+	"github.com/grafana/tempo/pkg/util/listtomap"
 	tempo_log "github.com/grafana/tempo/pkg/util/log"
 	"github.com/grafana/tempo/pkg/util/tracing"
 	"github.com/grafana/tempo/tempodb/backend"
@@ -202,10 +203,11 @@ func (o *userConfigurableOverridesManager) Forwarders(userID string) []string {
 }
 
 func (o *userConfigurableOverridesManager) MetricsGeneratorProcessors(userID string) map[string]struct{} {
-	if processors, ok := o.getTenantLimits(userID).GetMetricsGenerator().GetProcessors(); ok {
-		return processors.GetMap()
-	}
-	return o.Interface.MetricsGeneratorProcessors(userID)
+	// We merge settings from both layers meaning if a processor is enabled on any layer it will be always enabled (OR logic)
+	processorsUserConfigurable, _ := o.getTenantLimits(userID).GetMetricsGenerator().GetProcessors()
+	processorsRuntime := o.Interface.MetricsGeneratorProcessors(userID)
+
+	return listtomap.Merge(processorsUserConfigurable, processorsRuntime)
 }
 
 func (o *userConfigurableOverridesManager) MetricsGeneratorDisableCollection(userID string) bool {

modules/overrides/user_configurable_overrides_test.go

@@ -11,18 +11,19 @@ import (
 	"time"
 
 	"github.com/grafana/dskit/services"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/common/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v2"
+
 	userconfigurableoverrides "github.com/grafana/tempo/modules/overrides/userconfigurable/client"
 	tempo_api "github.com/grafana/tempo/pkg/api"
 	"github.com/grafana/tempo/pkg/sharedconfig"
 	filterconfig "github.com/grafana/tempo/pkg/spanfilter/config"
 	"github.com/grafana/tempo/pkg/util/listtomap"
 	"github.com/grafana/tempo/tempodb/backend"
 	"github.com/grafana/tempo/tempodb/backend/local"
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/common/model"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v2"
 )
 
 const (
@@ -371,15 +372,24 @@ func TestUserConfigOverridesManager_MergeRuntimeConfig(t *testing.T) {
 	// Set Forwarders in UserConfigOverrides limits
 	mgr.tenantLimits[tenantID] = &userconfigurableoverrides.Limits{
 		Forwarders: &[]string{"my-other-forwarder"},
+		MetricsGenerator: &userconfigurableoverrides.LimitsMetricsGenerator{
+			Processors: map[string]struct{}{"local-blocks": {}},
+		},
 	}
 
-	// test all override methods
+	// Test all override methods
+
+	// Forwarders are set in user-configurable overrides and will override runtime overrides
+	assert.NotEqual(t, mgr.Forwarders(tenantID), baseMgr.Forwarders(tenantID))
+
+	// Processors will be the merged result between runtime and user-configurable overrides
+	assert.Equal(t, mgr.MetricsGeneratorProcessors(tenantID), map[string]struct{}{"local-blocks": {}, "service-graphs": {}, "span-metrics": {}})
+
+	// For the remaining settings runtime overrides will bubble up
 	assert.Equal(t, mgr.IngestionRateStrategy(), baseMgr.IngestionRateStrategy())
 	assert.Equal(t, mgr.MaxLocalTracesPerUser(tenantID), baseMgr.MaxLocalTracesPerUser(tenantID))
 	assert.Equal(t, mgr.MaxGlobalTracesPerUser(tenantID), baseMgr.MaxGlobalTracesPerUser(tenantID))
 	assert.Equal(t, mgr.MaxBytesPerTrace(tenantID), baseMgr.MaxBytesPerTrace(tenantID))
-	// Forwarders are set in userconfigurableoverrides so not same as runtime overrides
-	assert.NotEqual(t, mgr.Forwarders(tenantID), baseMgr.Forwarders(tenantID))
 	assert.Equal(t, mgr.Forwarders(tenantID), []string{"my-other-forwarder"})
 	assert.Equal(t, baseMgr.Forwarders(tenantID), []string{"fwd", "fwd-2"})
 
@@ -389,7 +399,6 @@ func TestUserConfigOverridesManager_MergeRuntimeConfig(t *testing.T) {
 	assert.Equal(t, mgr.IngestionBurstSizeBytes(tenantID), baseMgr.IngestionBurstSizeBytes(tenantID))
 	assert.Equal(t, mgr.MetricsGeneratorIngestionSlack(tenantID), baseMgr.MetricsGeneratorIngestionSlack(tenantID))
 	assert.Equal(t, mgr.MetricsGeneratorRingSize(tenantID), baseMgr.MetricsGeneratorRingSize(tenantID))
-	assert.Equal(t, mgr.MetricsGeneratorProcessors(tenantID), baseMgr.MetricsGeneratorProcessors(tenantID))
 	assert.Equal(t, mgr.MetricsGeneratorMaxActiveSeries(tenantID), baseMgr.MetricsGeneratorMaxActiveSeries(tenantID))
 	assert.Equal(t, mgr.MetricsGeneratorCollectionInterval(tenantID), baseMgr.MetricsGeneratorCollectionInterval(tenantID))
 	assert.Equal(t, mgr.MetricsGeneratorDisableCollection(tenantID), baseMgr.MetricsGeneratorDisableCollection(tenantID))

pkg/util/listtomap/list_to_map.go

@@ -74,3 +74,14 @@ func (l *ListToMap) GetMap() map[string]struct{} {
 	}
 	return *l
 }
+
+func Merge(m1, m2 ListToMap) ListToMap {
+	merged := make(ListToMap)
+	for k := range m1 {
+		merged[k] = struct{}{}
+	}
+	for k := range m2 {
+		merged[k] = struct{}{}
+	}
+	return merged
+}

pkg/util/listtomap/list_to_map_test.go

@@ -93,3 +93,34 @@ func TestListToMapMarshalOperationsJSON(t *testing.T) {
 		})
 	}
 }
+
+func TestMerge(t *testing.T) {
+	testCases := []struct {
+		name           string
+		m1, m2, merged ListToMap
+	}{
+		{
+			"merge keys from both maps",
+			map[string]struct{}{"key1": {}, "key3": {}},
+			map[string]struct{}{"key2": {}, "key3": {}},
+			map[string]struct{}{"key1": {}, "key2": {}, "key3": {}},
+		},
+		{
+			"nil map",
+			nil,
+			map[string]struct{}{"key1": {}},
+			map[string]struct{}{"key1": {}},
+		},
+		{
+			"both nil",
+			nil,
+			nil,
+			map[string]struct{}{},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.merged, Merge(tc.m1, tc.m2))
+		})
+	}
+}