Fail on invalid intrinsic dimension override keys

grafana · Dec 20, 2022 · 9088882 · 9088882
1 parent 45b6e36
commit 9088882
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 7 deletions.
diff --git a/modules/generator/config.go b/modules/generator/config.go
@@ -4,6 +4,8 @@ import (
 	"flag"
 	"time"
 
+	"github.com/pkg/errors"
+
 	"github.com/grafana/tempo/modules/generator/processor/servicegraphs"
 	"github.com/grafana/tempo/modules/generator/processor/spanmetrics"
 	"github.com/grafana/tempo/modules/generator/registry"
@@ -50,7 +52,7 @@ func (cfg *ProcessorConfig) RegisterFlagsAndApplyDefaults(prefix string, f *flag
 }
 
 // copyWithOverrides creates a copy of the config using values set in the overrides.
-func (cfg *ProcessorConfig) copyWithOverrides(o metricsGeneratorOverrides, userID string) ProcessorConfig {
+func (cfg *ProcessorConfig) copyWithOverrides(o metricsGeneratorOverrides, userID string) (ProcessorConfig, error) {
 	copyCfg := *cfg
 
 	if buckets := o.MetricsGeneratorProcessorServiceGraphsHistogramBuckets(userID); buckets != nil {
@@ -66,8 +68,11 @@ func (cfg *ProcessorConfig) copyWithOverrides(o metricsGeneratorOverrides, userI
 		copyCfg.SpanMetrics.Dimensions = dimensions
 	}
 	if dimensions := o.MetricsGeneratorProcessorSpanMetricsIntrinsicDimensions(userID); dimensions != nil {
-		copyCfg.SpanMetrics.IntrinsicDimensions.ApplyFromMap(dimensions)
+		err := copyCfg.SpanMetrics.IntrinsicDimensions.ApplyFromMap(dimensions)
+		if err != nil {
+			return ProcessorConfig{}, errors.Wrap(err, "fail to apply overrides")
+		}
 	}
 
-	return copyCfg
+	return copyCfg, nil
 }
diff --git a/modules/generator/config_test.go b/modules/generator/config_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/tempo/modules/generator/processor/servicegraphs"
 	"github.com/grafana/tempo/modules/generator/processor/spanmetrics"
@@ -31,7 +32,8 @@ func TestProcessorConfig_copyWithOverrides(t *testing.T) {
 			spanMetricsIntrinsicDimensions: map[string]bool{"status_code": true},
 		}
 
-		copied := original.copyWithOverrides(o, "tenant")
+		copied, err := original.copyWithOverrides(o, "tenant")
+		require.NoError(t, err)
 
 		assert.NotEqual(t, *original, copied)
 
@@ -53,8 +55,18 @@ func TestProcessorConfig_copyWithOverrides(t *testing.T) {
 	t.Run("empty overrides", func(t *testing.T) {
 		o := &mockOverrides{}
 
-		copied := original.copyWithOverrides(o, "tenant")
+		copied, err := original.copyWithOverrides(o, "tenant")
+		require.NoError(t, err)
 
 		assert.Equal(t, *original, copied)
 	})
+
+	t.Run("invalid overrides", func(t *testing.T) {
+		o := &mockOverrides{
+			spanMetricsIntrinsicDimensions: map[string]bool{"invalid": true},
+		}
+
+		_, err := original.copyWithOverrides(o, "tenant")
+		require.Error(t, err)
+	})
 }
diff --git a/modules/generator/instance.go b/modules/generator/instance.go
@@ -126,7 +126,10 @@ func (i *instance) watchOverrides() {
 
 func (i *instance) updateProcessors() error {
 	desiredProcessors := i.overrides.MetricsGeneratorProcessors(i.instanceID)
-	desiredCfg := i.cfg.Processor.copyWithOverrides(i.overrides, i.instanceID)
+	desiredCfg, err := i.cfg.Processor.copyWithOverrides(i.overrides, i.instanceID)
+	if err != nil {
+		return err
+	}
 
 	i.processorsMtx.RLock()
 	toAdd, toRemove, toReplace, err := i.diffProcessors(desiredProcessors, desiredCfg)

diff --git a/modules/generator/processor/spanmetrics/config.go b/modules/generator/processor/spanmetrics/config.go
@@ -3,6 +3,7 @@ package spanmetrics
 import (
 	"flag"
 
+	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -44,7 +45,7 @@ type IntrinsicDimensions struct {
 	StatusMessage bool `yaml:"status_message,omitempty"`
 }
 
-func (ic *IntrinsicDimensions) ApplyFromMap(dimensions map[string]bool) {
+func (ic *IntrinsicDimensions) ApplyFromMap(dimensions map[string]bool) error {
 	for label, active := range dimensions {
 		switch label {
 		case dimService:
@@ -57,6 +58,9 @@ func (ic *IntrinsicDimensions) ApplyFromMap(dimensions map[string]bool) {
 			ic.StatusCode = active
 		case dimStatusMessage:
 			ic.StatusMessage = active
+		default:
+			return errors.Errorf("%s is not a valid intrinsic dimension", label)
 		}
 	}
+	return nil
 }