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