change: disable legacy overrides by default, add flag to opt in

CHANGELOG.md

@@ -55,6 +55,7 @@
 
 ### 3.0 Cleanup
 
+* [CHANGE] **BREAKING CHANGE** Disable legacy (flat, unscoped) overrides by default. Tempo will refuse to start if legacy overrides are detected. Set `enable_legacy_overrides: true` or `-config.enable-legacy-overrides=true` to opt back in temporarily. Legacy overrides will be removed in a future release. [#6741](https://github.com/grafana/tempo/pull/6741) (@electron0zero)
 * [CHANGE] **BREAKING CHANGE** Remove remaining app ingester config [#6667](https://github.com/grafana/tempo/pull/6667) (@javiermolinar)
 * [CHANGE] **BREAKING CHANGE** Remove span-metrics leftovers and lazy-init generator clients [#6618](https://github.com/grafana/tempo/pull/6618) (@javiermolinar)
 * [CHANGE] **BREAKING CHANGE** Decommission livestore MetricsGenerator query service [#6615](https://github.com/grafana/tempo/pull/6615) (@javiermolinar)

cmd/tempo-cli/cmd-migrate-overrides-config.go

@@ -39,6 +39,10 @@
 		return fmt.Errorf("failed to parse configFile %s: %w", cmd.ConfigFile, err)
 	}
 
+	// The migration command needs to read legacy overrides to convert them,
+	// so force-enable legacy overrides regardless of the config setting.
+	cfg.Overrides.EnableLegacyOverrides = true
+
 	o, err := overrides.NewOverrides(cfg.Overrides, nil, prometheus.DefaultRegisterer)
 	if err != nil {
 		return fmt.Errorf("failed to load overrides module: %w", err)
@@ -62,6 +66,7 @@
 	}
 
 	cfg.Overrides.Defaults = runtimeConfig.Defaults
+	cfg.Overrides.EnableLegacyOverrides = false // reset - migrated config doesn't need this
 	configBytes, err := yaml.Marshal(cfg)
 	if err != nil {
 		return fmt.Errorf("failed to marshal config: %w", err)

cmd/tempo/app/config.go

@@ -275,7 +275,8 @@ var (
 		Message: "Local backend will not correctly retrieve traces with a distributed deployment unless all components have access to the same disk. You should probably be using object storage as a backend.",
 	}
 	warnLegacyOverridesConfig = ConfigWarning{
-		Message: "Inline, unscoped overrides are deprecated. Please use the new overrides config format.",
+		Message: "DEPRECATED: Legacy (flat, unscoped) overrides format is in use and will be removed in a future release.",
+		Explain: "Migrate your overrides config to the new scoped format, or set -config.enable-legacy-overrides=true (or enable_legacy_overrides: true in YAML) to continue using legacy overrides temporarily.",
 	}
 
 	warnTracesAndUserConfigurableOverridesStorageConflict = ConfigWarning{

docs/sources/tempo/configuration/_index.md

@@ -2115,6 +2115,10 @@ overrides:
   # How frequent tenant-specific overrides are read from the configuration file.
   [per_tenant_override_period: <duration> | default = 10s]
 
+  # Enable the deprecated legacy overrides format.
+  # NOTE: This is disabled by default and will be removed in a future release.
+  [enable_legacy_overrides: <bool> | default = false]
+
   # User-configurable overrides configuration
   user_configurable_overrides:
 

docs/sources/tempo/configuration/manifest.md

@@ -768,6 +768,7 @@ overrides:
                 hedge_requests_up_to: 2
         api:
             check_for_conflicting_runtime_overrides: false
+    enable_legacy_overrides: false
 memberlist:
     node_name: ""
     randomize_node_name: true

modules/overrides/config.go

@@ -219,6 +219,11 @@ type Config struct {
 
 	UserConfigurableOverridesConfig UserConfigurableOverridesConfig `yaml:"user_configurable_overrides" json:"user_configurable_overrides"`
 
+	// EnableLegacyOverrides allows using the deprecated legacy (flat, unscoped) overrides format.
+	// Legacy overrides are disabled by default. Set this to true to opt back in while migrating
+	// to the new format. This option will be removed in a future release.
+	EnableLegacyOverrides bool `yaml:"enable_legacy_overrides" json:"enable_legacy_overrides"`
+
 	ConfigType ConfigType `yaml:"-" json:"-"`
 	ExpandEnv  bool       `yaml:"-" json:"-"`
 }
@@ -243,12 +248,15 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		PerTenantOverridePeriod model.Duration `yaml:"per_tenant_override_period"`
 
 		UserConfigurableOverridesConfig UserConfigurableOverridesConfig `yaml:"user_configurable_overrides"`
+
+		EnableLegacyOverrides bool `yaml:"enable_legacy_overrides"`
 	}
 	var legacyCfg legacyConfig
 	legacyCfg.DefaultOverrides = c.Defaults.toLegacy()
 	legacyCfg.PerTenantOverrideConfig = c.PerTenantOverrideConfig
 	legacyCfg.PerTenantOverridePeriod = c.PerTenantOverridePeriod
 	legacyCfg.UserConfigurableOverridesConfig = c.UserConfigurableOverridesConfig
+	legacyCfg.EnableLegacyOverrides = c.EnableLegacyOverrides
 
 	if legacyErr := unmarshal(&legacyCfg); legacyErr != nil {
 		return fmt.Errorf("failed to unmarshal config: %w; also failed in legacy format: %w", err, legacyErr)
@@ -258,6 +266,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	c.PerTenantOverrideConfig = legacyCfg.PerTenantOverrideConfig
 	c.PerTenantOverridePeriod = legacyCfg.PerTenantOverridePeriod
 	c.UserConfigurableOverridesConfig = legacyCfg.UserConfigurableOverridesConfig
+	c.EnableLegacyOverrides = legacyCfg.EnableLegacyOverrides
 	c.ConfigType = ConfigTypeLegacy
 	return nil
 }
@@ -291,6 +300,7 @@ func (c *Config) RegisterFlagsAndApplyDefaults(f *flag.FlagSet) {
 	f.StringVar(&c.PerTenantOverrideConfig, "config.per-user-override-config", "", "File name of per-user Overrides.")
 	_ = c.PerTenantOverridePeriod.Set("10s")
 	f.Var(&c.PerTenantOverridePeriod, "config.per-user-override-period", "Period with this to reload the Overrides.")
+	f.BoolVar(&c.EnableLegacyOverrides, "config.enable-legacy-overrides", false, "Enable the deprecated legacy overrides format. This is disabled by default and will be removed in a future release.")
 
 	c.UserConfigurableOverridesConfig.RegisterFlagsAndApplyDefaults(f)
 }

modules/overrides/overrides.go

@@ -1,8 +1,13 @@
 package overrides
 
 import (
+	"fmt"
+
+	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"go.opentelemetry.io/otel"
+
+	"github.com/grafana/tempo/pkg/util/log"
 )
 
 const wildcardTenant = "*"
@@ -21,6 +26,21 @@ var metricOverridesLimitsDesc = prometheus.NewDesc(
 // are defaulted to those values.  As such, the last call to NewOverrides will
 // become the new global defaults.
 func NewOverrides(cfg Config, validator Validator, registerer prometheus.Registerer) (Service, error) {
+	if cfg.ConfigType == ConfigTypeLegacy {
+		// always log a warning for ConfigTypeLegacy
+		level.Warn(log.Logger).Log(
+			"msg", "DEPRECATED: legacy overrides config format is in use. Legacy overrides are deprecated and will be removed in a future release. "+
+				"Please migrate your overrides config to the new overrides format.",
+		)
+
+		if !cfg.EnableLegacyOverrides {
+			return nil, fmt.Errorf(
+				"DEPRECATED: legacy overrides config format detected but legacy overrides are disabled by default. Legacy overrides will be removed in a future release. " +
+					"Migrate your overrides config to the new scoped format, or set -config.enable-legacy-overrides=true (or enable_legacy_overrides: true in YAML) to continue using legacy overrides temporarily")
+		}
+
+	}
+
 	o, err := newRuntimeConfigOverrides(cfg, validator, registerer)
 	if err != nil {
 		return nil, err

modules/overrides/overrides_test.go

@@ -0,0 +1,95 @@
+package overrides
+
+import (
+	"bytes"
+	"context"
+	"testing"
+
+	"github.com/grafana/dskit/services"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/require"
+	"go.yaml.in/yaml/v2"
+)
+
+func TestLegacyOverridesDisabledByDefault(t *testing.T) {
+	tests := []struct {
+		name                  string
+		configType            ConfigType
+		enableLegacyOverrides bool
+		expectErr             string
+	}{
+		{
+			name:       "rejects legacy when EnableLegacyOverrides is false",
+			configType: ConfigTypeLegacy,
+			expectErr:  "DEPRECATED: legacy overrides config format detected but legacy overrides are disabled by default",
+		},
+		{
+			name:                  "accepts legacy when EnableLegacyOverrides is true",
+			configType:            ConfigTypeLegacy,
+			enableLegacyOverrides: true,
+		},
+		{
+			name:       "accepts new config regardless of EnableLegacyOverrides",
+			configType: ConfigTypeNew,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := Config{
+				ConfigType:            tc.configType,
+				EnableLegacyOverrides: tc.enableLegacyOverrides,
+			}
+
+			o, err := NewOverrides(cfg, nil, prometheus.NewRegistry())
+			if tc.expectErr != "" {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.expectErr)
+				return
+			}
+
+			require.NoError(t, err)
+			require.NoError(t, services.StartAndAwaitRunning(context.TODO(), o))
+			require.NoError(t, services.StopAndAwaitTerminated(context.TODO(), o))
+		})
+	}
+}
+
+func TestPerTenantLegacyOverridesDisabledByDefault(t *testing.T) {
+	legacyOverrides := &perTenantLegacyOverrides{
+		TenantLimits: map[string]*LegacyOverrides{
+			"tenant1": {MaxBytesPerTrace: 1000},
+		},
+	}
+	buff, err := yaml.Marshal(legacyOverrides)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name         string
+		enableLegacy bool
+		expectErr    string
+	}{
+		{
+			name:      "rejects legacy per-tenant overrides when enableLegacy is false",
+			expectErr: "DEPRECATED: legacy overrides config format is in use. per-tenant overrides file uses the legacy format but legacy overrides are disabled by default",
+		},
+		{
+			name:         "accepts legacy per-tenant overrides when enableLegacy is true",
+			enableLegacy: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			loader := loadPerTenantOverrides(&mockValidator{}, ConfigTypeNew, false, tc.enableLegacy)
+			result, err := loader(bytes.NewReader(buff))
+			if tc.expectErr != "" {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.expectErr)
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, result)
+			}
+		})
+	}
+}

modules/overrides/runtime_config_overrides.go

@@ -68,7 +68,9 @@ func (o *perTenantOverrides) forUser(userID string) *Overrides {
 }
 
 // loadPerTenantOverrides is of type runtimeconfig.Loader
-func loadPerTenantOverrides(validator Validator, typ ConfigType, expandEnv bool) func(r io.Reader) (interface{}, error) {
+func loadPerTenantOverrides(validator Validator, typ ConfigType, expandEnv bool, enableLegacy bool) func(r io.Reader) (interface{}, error) {
+	// var is outside closure to ensure it's not recreated on each call
+	var lastLegacyWarn time.Time
 	return func(r io.Reader) (interface{}, error) {
 		overrides := &perTenantOverrides{}
 
@@ -92,6 +94,24 @@ func loadPerTenantOverrides(validator Validator, typ ConfigType, expandEnv bool)
 			return nil, err
 		}
 
+		if overrides.ConfigType == ConfigTypeLegacy {
+			// Log periodically (every 10 mins) and not every reload, so the warning stays visible in recent logs
+			// without spamming logs on every reload cycle.
+			if time.Since(lastLegacyWarn) >= 10*time.Minute {
+				level.Warn(log.Logger).Log(
+					"msg", "DEPRECATED: legacy overrides config format is in use. per-tenant overrides file uses the legacy format. Legacy overrides are deprecated and will be removed in a future release. "+
+						"Please migrate your per-tenant overrides to the new scoped format.",
+				)
+				lastLegacyWarn = time.Now()
+			}
+
+			if !enableLegacy {
+				return nil, fmt.Errorf(
+					"DEPRECATED: legacy overrides config format is in use. per-tenant overrides file uses the legacy format but legacy overrides are disabled by default. " +
+						"Migrate your per-tenant overrides to the new scoped format, or set -config.enable-legacy-overrides=true (or enable_legacy_overrides: true in YAML) to continue using legacy overrides temporarily")
+			}
+		}
+
 		if overrides.ConfigType != typ {
 			// TODO: Return error?
 			level.Warn(log.Logger).Log(
@@ -141,7 +161,7 @@ func newRuntimeConfigOverrides(cfg Config, validator Validator, registerer prome
 		runtimeCfg := runtimeconfig.Config{
 			LoadPath:     []string{cfg.PerTenantOverrideConfig},
 			ReloadPeriod: time.Duration(cfg.PerTenantOverridePeriod),
-			Loader:       loadPerTenantOverrides(validator, cfg.ConfigType, cfg.ExpandEnv),
+			Loader:       loadPerTenantOverrides(validator, cfg.ConfigType, cfg.ExpandEnv, cfg.EnableLegacyOverrides),
 		}
 		runtimeCfgMgr, err := runtimeconfig.New(runtimeCfg, "overrides", prometheus.WrapRegistererWithPrefix("tempo_", registerer), log.Logger)
 		if err != nil {

modules/overrides/runtime_config_overrides_test.go

@@ -27,7 +27,7 @@ import (
 func TestRuntimeConfigOverrides_loadPerTenantOverrides(t *testing.T) {
 	validator := &mockValidator{}
 
-	loader := loadPerTenantOverrides(validator, ConfigTypeNew, false)
+	loader := loadPerTenantOverrides(validator, ConfigTypeNew, false, false)
 
 	perTenantOverrides := perTenantOverrides{
 		TenantLimits: map[string]*Overrides{
@@ -205,7 +205,8 @@ func TestRuntimeConfigOverrides(t *testing.T) {
 
 		t.Run(fmt.Sprintf("%s (legacy)", tt.name), func(t *testing.T) {
 			cfg := Config{
-				Defaults: tt.defaultLimits,
+				Defaults:              tt.defaultLimits,
+				EnableLegacyOverrides: true, // need to enable it to test
 			}
 
 			if tt.perTenantOverrides != nil {

operations/jsonnet-compiled/jsonnetfile.lock.json

@@ -8,7 +8,7 @@
           "subdir": "ksonnet-util"
         }
       },
-      "version": "f53b1c9f855e11adce73b0aa6d6b08f511b89274",
+      "version": "2e9c7f4a276f6f6a9bc3f994f2b8804555f31bb9",
       "sum": "0y3AFX9LQSpfWTxWKSwoLgbt0Wc9nnCwhMH2szKzHv0="
     },
     {
@@ -18,7 +18,7 @@
           "subdir": "memcached"
         }
       },
-      "version": "f53b1c9f855e11adce73b0aa6d6b08f511b89274",
+      "version": "2e9c7f4a276f6f6a9bc3f994f2b8804555f31bb9",
       "sum": "zNTCDRaCqJUHjQSQRsxGR3jLrMP9tU7YNQb13dbm1bU="
     },
     {

operations/jsonnet-compiled/microservices-with-extras/gen/ConfigMap-tempo-overrides.yaml

@@ -3,10 +3,12 @@ data:
   overrides.yaml: |
     overrides:
         super_user:
-            ingestion_burst_size_bytes: 2e+07
-            ingestion_rate_limit_bytes: 2e+07
-            max_bytes_per_trace: 3e+07
-            max_traces_per_user: 100000
+            global:
+                max_bytes_per_trace: 3e+07
+            ingestion:
+                burst_size_bytes: 2e+07
+                max_traces_per_user: 100000
+                rate_limit_bytes: 2e+07
 kind: ConfigMap
 metadata:
   name: tempo-overrides

operations/jsonnet-compiled/microservices/gen/ConfigMap-tempo-overrides.yaml

@@ -3,10 +3,12 @@ data:
   overrides.yaml: |
     overrides:
         super_user:
-            ingestion_burst_size_bytes: 2e+07
-            ingestion_rate_limit_bytes: 2e+07
-            max_bytes_per_trace: 3e+07
-            max_traces_per_user: 100000
+            global:
+                max_bytes_per_trace: 3e+07
+            ingestion:
+                burst_size_bytes: 2e+07
+                max_traces_per_user: 100000
+                rate_limit_bytes: 2e+07
 kind: ConfigMap
 metadata:
   name: tempo-overrides

operations/jsonnet/microservices/config.libsonnet

@@ -182,10 +182,14 @@
     overrides_configmap_name: 'tempo-overrides',
     overrides+:: {
       super_user: {
-        max_traces_per_user: 100000,
-        ingestion_rate_limit_bytes: 200e5,  // ~20MB per sec
-        ingestion_burst_size_bytes: 200e5,  // ~20MB
-        max_bytes_per_trace: 300e5,  // ~30MB
+        ingestion: {
+          max_traces_per_user: 100000,
+          rate_limit_bytes: 200e5,  // ~20MB per sec
+          burst_size_bytes: 200e5,  // ~20MB
+        },
+        global: {
+          max_bytes_per_trace: 300e5,  // ~30MB
+        },
       },
     },
   },