Skip to content

Commit

Permalink
config: remove expand-external-labels flag in release 3.0 (#14657)
Browse files Browse the repository at this point in the history 
remove expand-external-labels feature flag

and enabled env arg expansion for external labels by default.

Signed-off-by: jyz0309 <[email protected]>
  • Loading branch information
@jyz0309
jyz0309 authored Oct 17, 2024
1 parent 6ce21b8 commit 2cabd1b
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 70 deletions.
26 changes: 11 additions & 15 deletions cmd/prometheus/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,10 @@ type flagConfig struct {
memlimitRatio float64
// These options are extracted from featureList
// for ease of use.
enableExpandExternalLabels bool
enablePerStepStats bool
enableAutoGOMAXPROCS bool
enableAutoGOMEMLIMIT bool
enableConcurrentRuleEval bool
enablePerStepStats bool
enableAutoGOMAXPROCS bool
enableAutoGOMEMLIMIT bool
enableConcurrentRuleEval bool

prometheusURL string
corsRegexString string
Expand All @@ -220,9 +219,6 @@ func (c *flagConfig) setFeatureListOptions(logger *slog.Logger) error {
opts := strings.Split(f, ",")
for _, o := range opts {
switch o {
case "expand-external-labels":
c.enableExpandExternalLabels = true
logger.Info("Experimental expand-external-labels enabled")
case "exemplar-storage":
c.tsdb.EnableExemplarStorage = true
logger.Info("Experimental in-memory exemplar storage enabled")
Expand Down Expand Up @@ -595,7 +591,7 @@ func main() {

// Throw error for invalid config before starting other components.
var cfgFile *config.Config
if cfgFile, err = config.LoadFile(cfg.configFile, agentMode, false, promslog.NewNopLogger()); err != nil {
if cfgFile, err = config.LoadFile(cfg.configFile, agentMode, promslog.NewNopLogger()); err != nil {
absPath, pathErr := filepath.Abs(cfg.configFile)
if pathErr != nil {
absPath = cfg.configFile
Expand Down Expand Up @@ -1145,7 +1141,7 @@ func main() {
for {
select {
case <-hup:
if err := reloadConfig(cfg.configFile, cfg.enableExpandExternalLabels, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil {
if err := reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil {
logger.Error("Error reloading config", "err", err)
} else if cfg.enableAutoReload {
if currentChecksum, err := config.GenerateChecksum(cfg.configFile); err == nil {
Expand All @@ -1155,7 +1151,7 @@ func main() {
}
}
case rc := <-webHandler.Reload():
if err := reloadConfig(cfg.configFile, cfg.enableExpandExternalLabels, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil {
if err := reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil {
logger.Error("Error reloading config", "err", err)
rc <- err
} else {
Expand All @@ -1180,7 +1176,7 @@ func main() {
}
logger.Info("Configuration file change detected, reloading the configuration.")

if err := reloadConfig(cfg.configFile, cfg.enableExpandExternalLabels, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil {
if err := reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil {
logger.Error("Error reloading config", "err", err)
} else {
checksum = currentChecksum
Expand Down Expand Up @@ -1210,7 +1206,7 @@ func main() {
return nil
}

if err := reloadConfig(cfg.configFile, cfg.enableExpandExternalLabels, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, func(bool) {}, reloaders...); err != nil {
if err := reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, func(bool) {}, reloaders...); err != nil {
return fmt.Errorf("error loading config from %q: %w", cfg.configFile, err)
}

Expand Down Expand Up @@ -1437,7 +1433,7 @@ type reloader struct {
reloader func(*config.Config) error
}

func reloadConfig(filename string, expandExternalLabels, enableExemplarStorage bool, logger *slog.Logger, noStepSuqueryInterval *safePromQLNoStepSubqueryInterval, callback func(bool), rls ...reloader) (err error) {
func reloadConfig(filename string, enableExemplarStorage bool, logger *slog.Logger, noStepSuqueryInterval *safePromQLNoStepSubqueryInterval, callback func(bool), rls ...reloader) (err error) {
start := time.Now()
timingsLogger := logger
logger.Info("Loading configuration file", "filename", filename)
Expand All @@ -1453,7 +1449,7 @@ func reloadConfig(filename string, expandExternalLabels, enableExemplarStorage b
}
}()

conf, err := config.LoadFile(filename, agentMode, expandExternalLabels, logger)
conf, err := config.LoadFile(filename, agentMode, logger)
if err != nil {
return fmt.Errorf("couldn't load configuration (--config.file=%q): %w", filename, err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/promtool/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func checkFileExists(fn string) error {
func checkConfig(agentMode bool, filename string, checkSyntaxOnly bool) ([]string, error) {
fmt.Println("Checking", filename)

cfg, err := config.LoadFile(filename, agentMode, false, promslog.NewNopLogger())
cfg, err := config.LoadFile(filename, agentMode, promslog.NewNopLogger())
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/promtool/sd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type sdCheckResult struct {
func CheckSD(sdConfigFiles, sdJobName string, sdTimeout time.Duration, registerer prometheus.Registerer) int {
logger := promslog.New(&promslog.Config{})

cfg, err := config.LoadFile(sdConfigFiles, false, false, logger)
cfg, err := config.LoadFile(sdConfigFiles, false, logger)
if err != nil {
fmt.Fprintln(os.Stderr, "Cannot load config", err)
return failureExitCode
Expand Down
14 changes: 6 additions & 8 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const (
)

// Load parses the YAML input s into a Config.
func Load(s string, expandExternalLabels bool, logger *slog.Logger) (*Config, error) {
func Load(s string, logger *slog.Logger) (*Config, error) {
cfg := &Config{}
// If the entire config body is empty the UnmarshalYAML method is
// never called. We thus have to set the DefaultConfig at the entry
Expand All @@ -84,10 +84,6 @@ func Load(s string, expandExternalLabels bool, logger *slog.Logger) (*Config, er
return nil, err
}

if !expandExternalLabels {
return cfg, nil
}

b := labels.NewScratchBuilder(0)
cfg.GlobalConfig.ExternalLabels.Range(func(v labels.Label) {
newV := os.Expand(v.Value, func(s string) string {
Expand All @@ -106,17 +102,19 @@ func Load(s string, expandExternalLabels bool, logger *slog.Logger) (*Config, er
// Note newV can be blank. https://github.com/prometheus/prometheus/issues/11024
b.Add(v.Name, newV)
})
cfg.GlobalConfig.ExternalLabels = b.Labels()
if !b.Labels().IsEmpty() {
cfg.GlobalConfig.ExternalLabels = b.Labels()
}
return cfg, nil
}

// LoadFile parses the given YAML file into a Config.
func LoadFile(filename string, agentMode, expandExternalLabels bool, logger *slog.Logger) (*Config, error) {
func LoadFile(filename string, agentMode bool, logger *slog.Logger) (*Config, error) {
content, err := os.ReadFile(filename)
if err != nil {
return nil, err
}
cfg, err := Load(string(content), expandExternalLabels, logger)
cfg, err := Load(string(content), logger)
if err != nil {
return nil, fmt.Errorf("parsing YAML file %s: %w", filename, err)
}
Expand Down
65 changes: 31 additions & 34 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,7 @@ var expectedConf = &Config{
}

func TestYAMLRoundtrip(t *testing.T) {
want, err := LoadFile("testdata/roundtrip.good.yml", false, false, promslog.NewNopLogger())
want, err := LoadFile("testdata/roundtrip.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)

out, err := yaml.Marshal(want)
Expand All @@ -1514,7 +1514,7 @@ func TestYAMLRoundtrip(t *testing.T) {
}

func TestRemoteWriteRetryOnRateLimit(t *testing.T) {
want, err := LoadFile("testdata/remote_write_retry_on_rate_limit.good.yml", false, false, promslog.NewNopLogger())
want, err := LoadFile("testdata/remote_write_retry_on_rate_limit.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)

out, err := yaml.Marshal(want)
Expand All @@ -1529,7 +1529,7 @@ func TestRemoteWriteRetryOnRateLimit(t *testing.T) {

func TestOTLPSanitizeResourceAttributes(t *testing.T) {
t.Run("good config", func(t *testing.T) {
want, err := LoadFile(filepath.Join("testdata", "otlp_sanitize_resource_attributes.good.yml"), false, false, promslog.NewNopLogger())
want, err := LoadFile(filepath.Join("testdata", "otlp_sanitize_resource_attributes.good.yml"), false, promslog.NewNopLogger())
require.NoError(t, err)

out, err := yaml.Marshal(want)
Expand All @@ -1541,7 +1541,7 @@ func TestOTLPSanitizeResourceAttributes(t *testing.T) {
})

t.Run("bad config", func(t *testing.T) {
_, err := LoadFile(filepath.Join("testdata", "otlp_sanitize_resource_attributes.bad.yml"), false, false, promslog.NewNopLogger())
_, err := LoadFile(filepath.Join("testdata", "otlp_sanitize_resource_attributes.bad.yml"), false, promslog.NewNopLogger())
require.ErrorContains(t, err, `duplicated promoted OTel resource attribute "k8s.job.name"`)
require.ErrorContains(t, err, `empty promoted OTel resource attribute`)
})
Expand All @@ -1550,16 +1550,17 @@ func TestOTLPSanitizeResourceAttributes(t *testing.T) {
func TestLoadConfig(t *testing.T) {
// Parse a valid file that sets a global scrape timeout. This tests whether parsing
// an overwritten default field in the global config permanently changes the default.
_, err := LoadFile("testdata/global_timeout.good.yml", false, false, promslog.NewNopLogger())
_, err := LoadFile("testdata/global_timeout.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)

c, err := LoadFile("testdata/conf.good.yml", false, false, promslog.NewNopLogger())
c, err := LoadFile("testdata/conf.good.yml", false, promslog.NewNopLogger())

require.NoError(t, err)
require.Equal(t, expectedConf, c)
}

func TestScrapeIntervalLarger(t *testing.T) {
c, err := LoadFile("testdata/scrape_interval_larger.good.yml", false, false, promslog.NewNopLogger())
c, err := LoadFile("testdata/scrape_interval_larger.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
require.Len(t, c.ScrapeConfigs, 1)
for _, sc := range c.ScrapeConfigs {
Expand All @@ -1569,7 +1570,7 @@ func TestScrapeIntervalLarger(t *testing.T) {

// YAML marshaling must not reveal authentication credentials.
func TestElideSecrets(t *testing.T) {
c, err := LoadFile("testdata/conf.good.yml", false, false, promslog.NewNopLogger())
c, err := LoadFile("testdata/conf.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)

secretRe := regexp.MustCompile(`\\u003csecret\\u003e|<secret>`)
Expand All @@ -1586,31 +1587,31 @@ func TestElideSecrets(t *testing.T) {

func TestLoadConfigRuleFilesAbsolutePath(t *testing.T) {
// Parse a valid file that sets a rule files with an absolute path
c, err := LoadFile(ruleFilesConfigFile, false, false, promslog.NewNopLogger())
c, err := LoadFile(ruleFilesConfigFile, false, promslog.NewNopLogger())
require.NoError(t, err)
require.Equal(t, ruleFilesExpectedConf, c)
}

func TestKubernetesEmptyAPIServer(t *testing.T) {
_, err := LoadFile("testdata/kubernetes_empty_apiserver.good.yml", false, false, promslog.NewNopLogger())
_, err := LoadFile("testdata/kubernetes_empty_apiserver.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
}

func TestKubernetesWithKubeConfig(t *testing.T) {
_, err := LoadFile("testdata/kubernetes_kubeconfig_without_apiserver.good.yml", false, false, promslog.NewNopLogger())
_, err := LoadFile("testdata/kubernetes_kubeconfig_without_apiserver.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
}

func TestKubernetesSelectors(t *testing.T) {
_, err := LoadFile("testdata/kubernetes_selectors_endpoints.good.yml", false, false, promslog.NewNopLogger())
_, err := LoadFile("testdata/kubernetes_selectors_endpoints.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
_, err = LoadFile("testdata/kubernetes_selectors_node.good.yml", false, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/kubernetes_selectors_node.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
_, err = LoadFile("testdata/kubernetes_selectors_ingress.good.yml", false, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/kubernetes_selectors_ingress.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
_, err = LoadFile("testdata/kubernetes_selectors_pod.good.yml", false, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/kubernetes_selectors_pod.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
_, err = LoadFile("testdata/kubernetes_selectors_service.good.yml", false, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/kubernetes_selectors_service.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
}

Expand Down Expand Up @@ -2094,7 +2095,7 @@ func TestBadConfigs(t *testing.T) {
model.NameValidationScheme = model.UTF8Validation
}()
for _, ee := range expectedErrors {
_, err := LoadFile("testdata/"+ee.filename, false, false, promslog.NewNopLogger())
_, err := LoadFile("testdata/"+ee.filename, false, promslog.NewNopLogger())
require.ErrorContains(t, err, ee.errMsg,
"Expected error for %s to contain %q but got: %s", ee.filename, ee.errMsg, err)
}
Expand Down Expand Up @@ -2125,7 +2126,7 @@ func TestBadStaticConfigsYML(t *testing.T) {
}

func TestEmptyConfig(t *testing.T) {
c, err := Load("", false, promslog.NewNopLogger())
c, err := Load("", promslog.NewNopLogger())
require.NoError(t, err)
exp := DefaultConfig
require.Equal(t, exp, *c)
Expand All @@ -2135,38 +2136,34 @@ func TestExpandExternalLabels(t *testing.T) {
// Cleanup ant TEST env variable that could exist on the system.
os.Setenv("TEST", "")

c, err := LoadFile("testdata/external_labels.good.yml", false, false, promslog.NewNopLogger())
require.NoError(t, err)
testutil.RequireEqual(t, labels.FromStrings("bar", "foo", "baz", "foo${TEST}bar", "foo", "${TEST}", "qux", "foo$${TEST}", "xyz", "foo$$bar"), c.GlobalConfig.ExternalLabels)

c, err = LoadFile("testdata/external_labels.good.yml", false, true, promslog.NewNopLogger())
c, err := LoadFile("testdata/external_labels.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
testutil.RequireEqual(t, labels.FromStrings("bar", "foo", "baz", "foobar", "foo", "", "qux", "foo${TEST}", "xyz", "foo$bar"), c.GlobalConfig.ExternalLabels)

os.Setenv("TEST", "TestValue")
c, err = LoadFile("testdata/external_labels.good.yml", false, true, promslog.NewNopLogger())
c, err = LoadFile("testdata/external_labels.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)
testutil.RequireEqual(t, labels.FromStrings("bar", "foo", "baz", "fooTestValuebar", "foo", "TestValue", "qux", "foo${TEST}", "xyz", "foo$bar"), c.GlobalConfig.ExternalLabels)
}

func TestAgentMode(t *testing.T) {
_, err := LoadFile("testdata/agent_mode.with_alert_manager.yml", true, false, promslog.NewNopLogger())
_, err := LoadFile("testdata/agent_mode.with_alert_manager.yml", true, promslog.NewNopLogger())
require.ErrorContains(t, err, "field alerting is not allowed in agent mode")

_, err = LoadFile("testdata/agent_mode.with_alert_relabels.yml", true, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/agent_mode.with_alert_relabels.yml", true, promslog.NewNopLogger())
require.ErrorContains(t, err, "field alerting is not allowed in agent mode")

_, err = LoadFile("testdata/agent_mode.with_rule_files.yml", true, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/agent_mode.with_rule_files.yml", true, promslog.NewNopLogger())
require.ErrorContains(t, err, "field rule_files is not allowed in agent mode")

_, err = LoadFile("testdata/agent_mode.with_remote_reads.yml", true, false, promslog.NewNopLogger())
_, err = LoadFile("testdata/agent_mode.with_remote_reads.yml", true, promslog.NewNopLogger())
require.ErrorContains(t, err, "field remote_read is not allowed in agent mode")

c, err := LoadFile("testdata/agent_mode.without_remote_writes.yml", true, false, promslog.NewNopLogger())
c, err := LoadFile("testdata/agent_mode.without_remote_writes.yml", true, promslog.NewNopLogger())
require.NoError(t, err)
require.Empty(t, c.RemoteWriteConfigs)

c, err = LoadFile("testdata/agent_mode.good.yml", true, false, promslog.NewNopLogger())
c, err = LoadFile("testdata/agent_mode.good.yml", true, promslog.NewNopLogger())
require.NoError(t, err)
require.Len(t, c.RemoteWriteConfigs, 1)
require.Equal(
Expand All @@ -2177,7 +2174,7 @@ func TestAgentMode(t *testing.T) {
}

func TestEmptyGlobalBlock(t *testing.T) {
c, err := Load("global:\n", false, promslog.NewNopLogger())
c, err := Load("global:\n", promslog.NewNopLogger())
require.NoError(t, err)
exp := DefaultConfig
exp.Runtime = DefaultRuntimeConfig
Expand Down Expand Up @@ -2332,7 +2329,7 @@ func TestGetScrapeConfigs(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
c, err := LoadFile(tc.configFile, false, false, promslog.NewNopLogger())
c, err := LoadFile(tc.configFile, false, promslog.NewNopLogger())
require.NoError(t, err)

scfgs, err := c.GetScrapeConfigs()
Expand All @@ -2350,7 +2347,7 @@ func kubernetesSDHostURL() config.URL {
}

func TestScrapeConfigDisableCompression(t *testing.T) {
want, err := LoadFile("testdata/scrape_config_disable_compression.good.yml", false, false, promslog.NewNopLogger())
want, err := LoadFile("testdata/scrape_config_disable_compression.good.yml", false, promslog.NewNopLogger())
require.NoError(t, err)

out, err := yaml.Marshal(want)
Expand Down Expand Up @@ -2397,7 +2394,7 @@ func TestScrapeConfigNameValidationSettings(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
want, err := LoadFile(fmt.Sprintf("testdata/%s.yml", tc.inputFile), false, false, promslog.NewNopLogger())
want, err := LoadFile(fmt.Sprintf("testdata/%s.yml", tc.inputFile), false, promslog.NewNopLogger())
require.NoError(t, err)

out, err := yaml.Marshal(want)
Expand Down
6 changes: 5 additions & 1 deletion docs/configuration/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ global:
[ rule_query_offset: <duration> | default = 0s ]

# The labels to add to any time series or alerts when communicating with
# external systems (federation, remote storage, Alertmanager).
# external systems (federation, remote storage, Alertmanager).
# Environment variable references `${var}` or `$var` are replaced according
# to the values of the current environment variables.
# References to undefined variables are replaced by the empty string.
# The `$` character can be escaped by using `$$`.
external_labels:
[ <labelname>: <labelvalue> ... ]

Expand Down
9 changes: 0 additions & 9 deletions docs/feature_flags.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,6 @@ Their behaviour can change in future releases which will be communicated via the
You can enable them using the `--enable-feature` flag with a comma separated list of features.
They may be enabled by default in future versions.

## Expand environment variables in external labels

`--enable-feature=expand-external-labels`

Replace `${var}` or `$var` in the [`external_labels`](configuration/configuration.md#configuration-file)
values according to the values of the current environment variables. References
to undefined variables are replaced by the empty string.
The `$` character can be escaped by using `$$`.

## Exemplars storage

`--enable-feature=exemplar-storage`
Expand Down
2 changes: 1 addition & 1 deletion scrape/scrape_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3835,7 +3835,7 @@ scrape_configs:

mng, err := NewManager(&Options{DiscoveryReloadInterval: model.Duration(10 * time.Millisecond), EnableNativeHistogramsIngestion: true}, nil, nil, s, reg)
require.NoError(t, err)
cfg, err := config.Load(configStr, false, promslog.NewNopLogger())
cfg, err := config.Load(configStr, promslog.NewNopLogger())
require.NoError(t, err)
mng.ApplyConfig(cfg)
tsets := make(chan map[string][]*targetgroup.Group)
Expand Down

0 comments on commit 2cabd1b

Please sign in to comment.