Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions modules/generator/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@
}
}

if err := cfg.Storage.Validate(); err != nil {
return err
}

Check notice on line 136 in modules/generator/config.go

View workflow job for this annotation

GitHub Actions / Coverage Annotations

Uncovered lines

Lines 135-136 are not covered by tests

if !slices.Contains(validCodecs, cfg.Codec) {
return fmt.Errorf("invalid codec: %s, valid choices are %s", cfg.Codec, validCodecs)
}
Expand Down
15 changes: 15 additions & 0 deletions modules/generator/storage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

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"
Expand All @@ -26,6 +28,19 @@
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)
}

Check notice on line 39 in modules/generator/storage/config.go

View workflow job for this annotation

GitHub Actions / Coverage Annotations

Uncovered lines

Lines 38-39 are not covered by tests
}
return nil
}

func (cfg *Config) RegisterFlagsAndApplyDefaults(string, *flag.FlagSet) {
cfg.Wal = agentDefaultOptions()

Expand Down
46 changes: 46 additions & 0 deletions modules/generator/storage/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions modules/generator/storage/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,16 @@

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

Check notice on line 178 in modules/generator/storage/instance.go

View workflow job for this annotation

GitHub Actions / Coverage Annotations

Uncovered lines

Lines 177-178 are not covered by tests
}
s.currentHeaders = newHeaders
s.sendNativeHistograms = newSendNativeHistograms

Check notice on line 181 in modules/generator/storage/instance.go

View workflow job for this annotation

GitHub Actions / Coverage Annotations

Uncovered lines

Lines 180-181 are not covered by tests
}
case <-s.closeCh:
return
Expand Down
Loading