From 85e52e223729fdb7c99d6dce774d9dff0a86253c Mon Sep 17 00:00:00 2001 From: Carles Garcia Cabot Date: Fri, 6 Feb 2026 12:27:17 +0100 Subject: [PATCH] Fix metrics generator panic when write_relabel_configs is set (#6396) The Prometheus dependency upgrade from v0.304.2 to v0.307.3 (PR prometheus/prometheus#16928) moved label name validation from a global to a per-config NameValidationScheme field on relabel.Config. This field must be initialized by calling Validate() before use, otherwise relabeling panics with "Invalid name validation scheme requested: unset". Tempo constructs RemoteWriteConfig programmatically rather than going through Prometheus's config.Load() path, so Validate() was never called. Call RemoteWriteConfig.Validate(model.UTF8Validation) once at startup in storage.Config.Validate(), which initializes NameValidationScheme on all write relabel configs before any tenant storage instances are created. Also fix a pre-existing bug in watchOverrides where cached state was updated before ApplyConfig succeeded, preventing retries on failure. --- CHANGELOG.md | 1 + modules/generator/config.go | 4 ++ modules/generator/storage/config.go | 15 ++++++ modules/generator/storage/config_util_test.go | 46 +++++++++++++++++++ modules/generator/storage/instance.go | 7 +-- 5 files changed, 70 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fce9c5c105e..bd186254d36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * [BUGFIX] Correct instant query calculation for rate() [#6205](https://github.com/grafana/tempo/pull/6205) (@ruslan-mikhailov) * [BUGFIX] Fix live-store deadlock occurring after a complete block failure [#6338](https://github.com/grafana/tempo/pull/6338) (@ruslan-mikhailov) * [BUGFIX] generator: fix dimension_mappings and target_info_excluded_dimensions being unconditionally overwritten even when overrides were nil [#6390](https://github.com/grafana/tempo/pull/6390) (@carles-grafana) +* [BUGFIX] generator: fix panic when `write_relabel_configs` is configured on remote write endpoints [#6396](https://github.com/grafana/tempo/pull/6396) (@carles-grafana) ### 3.0 Cleanup diff --git a/modules/generator/config.go b/modules/generator/config.go index b4bb576b958..bff9b55d7df 100644 --- a/modules/generator/config.go +++ b/modules/generator/config.go @@ -131,6 +131,10 @@ func (cfg *Config) Validate() error { } } + if err := cfg.Storage.Validate(); err != nil { + return err + } + if !slices.Contains(validCodecs, cfg.Codec) { return fmt.Errorf("invalid codec: %s, valid choices are %s", cfg.Codec, validCodecs) } diff --git a/modules/generator/storage/config.go b/modules/generator/storage/config.go index e72c2ab9abb..0a5c3108ca0 100644 --- a/modules/generator/storage/config.go +++ b/modules/generator/storage/config.go @@ -2,8 +2,10 @@ package storage import ( "flag" + "fmt" "time" + "github.com/prometheus/common/model" prometheus_config "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/tsdb/agent" "github.com/prometheus/prometheus/util/compression" @@ -26,6 +28,19 @@ type Config struct { RemoteWrite []prometheus_config.RemoteWriteConfig `yaml:"remote_write,omitempty"` } +// Validate initializes and validates the remote write configurations. This must +// be called once at startup before any tenant storage instances are created, so +// that NameValidationScheme is set on all write relabel configs. Without this, +// relabeling panics with "Invalid name validation scheme requested: unset". +func (cfg *Config) Validate() error { + for i := range cfg.RemoteWrite { + if err := cfg.RemoteWrite[i].Validate(model.UTF8Validation); err != nil { + return fmt.Errorf("invalid remote write config %q: %w", cfg.RemoteWrite[i].Name, err) + } + } + return nil +} + func (cfg *Config) RegisterFlagsAndApplyDefaults(string, *flag.FlagSet) { cfg.Wal = agentDefaultOptions() diff --git a/modules/generator/storage/config_util_test.go b/modules/generator/storage/config_util_test.go index 4320e9911ae..a032a6b8992 100644 --- a/modules/generator/storage/config_util_test.go +++ b/modules/generator/storage/config_util_test.go @@ -7,8 +7,12 @@ import ( "testing" prometheus_common_config "github.com/prometheus/common/config" + "github.com/prometheus/common/model" prometheus_config "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/model/relabel" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/grafana/tempo/pkg/util" ) @@ -142,6 +146,48 @@ func Test_copyMap(t *testing.T) { assert.Equal(t, "", original["k3"]) } +func Test_generateTenantRemoteWriteConfigs_writeRelabelConfigs(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + + cfg := Config{ + RemoteWrite: []prometheus_config.RemoteWriteConfig{ + { + URL: &prometheus_common_config.URL{URL: urlMustParse("http://prometheus-1/api/prom/push")}, + Headers: map[string]string{}, + WriteRelabelConfigs: []*relabel.Config{ + { + SourceLabels: model.LabelNames{"deployment_environment"}, + Separator: relabel.DefaultRelabelConfig.Separator, + Regex: relabel.DefaultRelabelConfig.Regex, + Replacement: relabel.DefaultRelabelConfig.Replacement, + Action: relabel.Replace, + TargetLabel: "client_deployment_environment", + }, + }, + }, + }, + } + + // Validate once at startup, as the generator does in Config.Validate(). + require.NoError(t, cfg.Validate()) + + result := generateTenantRemoteWriteConfigs(cfg.RemoteWrite, "my-tenant", nil, false, logger, false) + + // Simulate what the Prometheus remote write queue manager does: run the + // relabel configs against a label set. Before the fix this panics with + // "Invalid name validation scheme requested: unset" because + // NameValidationScheme was never initialized on the relabel configs. + lbls := labels.FromStrings("deployment_environment", "production") + + var newLbls labels.Labels + var keep bool + assert.NotPanics(t, func() { + newLbls, keep = relabel.Process(lbls, result[0].WriteRelabelConfigs...) + }) + assert.True(t, keep) + assert.Equal(t, "production", newLbls.Get("client_deployment_environment")) +} + func urlMustParse(urlStr string) *url.URL { url, err := url.Parse(urlStr) if err != nil { diff --git a/modules/generator/storage/instance.go b/modules/generator/storage/instance.go index abce3a85a71..e4bdc156682 100644 --- a/modules/generator/storage/instance.go +++ b/modules/generator/storage/instance.go @@ -169,15 +169,16 @@ func (s *storageImpl) watchOverrides() { if !headersEqual(s.currentHeaders, newHeaders) || s.sendNativeHistograms != newSendNativeHistograms { s.logger.Info("updating remote write configuration") - s.currentHeaders = newHeaders - s.sendNativeHistograms = newSendNativeHistograms err := s.remote.ApplyConfig(&prometheus_config.Config{ RemoteWriteConfigs: generateTenantRemoteWriteConfigs(s.cfg.RemoteWrite, s.tenantID, newHeaders, s.cfg.RemoteWriteAddOrgIDHeader, s.logger, newSendNativeHistograms), }) if err != nil { metricStorageRemoteWriteUpdateFailed.WithLabelValues(s.tenantID).Inc() - s.logger.Info("Failed to update remote write configuration. Remote write will continue with configuration", "err", err.Error()) + s.logger.Warn("Failed to update remote write configuration. Remote write will continue with previous configuration", "err", err.Error()) + continue } + s.currentHeaders = newHeaders + s.sendNativeHistograms = newSendNativeHistograms } case <-s.closeCh: return