From 658f837d4733fee29533c81d4338480fe1a0ca63 Mon Sep 17 00:00:00 2001 From: Piotr <17101802+thampiotr@users.noreply.github.com> Date: Thu, 20 Feb 2025 17:12:39 +0000 Subject: [PATCH] fix breaking change in prom metrics validation scheme (#2781) (#2783) * fix breaking change in prom metrics validation scheme * add test and changelog * punctuation --- CHANGELOG.md | 9 ++++ docs/sources/reference/cli/run.md | 1 + internal/alloycli/cmd_run.go | 87 ++++++++++++++++++++++--------- internal/alloycli/cmd_run_test.go | 72 +++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 26 deletions(-) create mode 100644 internal/alloycli/cmd_run_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a17b9b7fe9..27d8b0c250 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ This document contains a historical list of changes between releases. Only changes that impact end-user behavior are listed; changes to documentation or internal API changes are not present. +v1.7.0-rc.2 +----------------- + +### Bugfixes + +- Fix an issue where Prometheus metric name validation scheme was set by default to UTF-8. It is now set back to the + previous "legacy" scheme. An experimental flag `--feature.prometheus.metric-validation-scheme` can be used to switch + it to `utf-8` to experiment with UTF-8 support. + v1.7.0-rc.1 ----------------- diff --git a/docs/sources/reference/cli/run.md b/docs/sources/reference/cli/run.md index 4cbe811690..a22b0c97ca 100644 --- a/docs/sources/reference/cli/run.md +++ b/docs/sources/reference/cli/run.md @@ -60,6 +60,7 @@ The following flags are supported: * `--config.extra-args`: Extra arguments from the original format used by the converter. * `--stability.level`: The minimum permitted stability level of functionality to run. Supported values: `experimental`, `public-preview`, `generally-available` (default `"generally-available"`). * `--feature.community-components.enabled`: Enable community components (default `false`). +* `--feature.prometheus.metric-validation-scheme`: Prometheus metric validation scheme to use. Supported values: `legacy`, `utf-8`. NOTE: this is an experimental flag and may be removed in future releases (default `"legacy"`). ## Update the configuration file diff --git a/internal/alloycli/cmd_run.go b/internal/alloycli/cmd_run.go index 7ef521ed19..e5b3c97e34 100644 --- a/internal/alloycli/cmd_run.go +++ b/internal/alloycli/cmd_run.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/ckit/advertise" "github.com/grafana/ckit/peer" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" "github.com/spf13/cobra" "github.com/spf13/pflag" "go.opentelemetry.io/otel" @@ -50,6 +51,11 @@ import ( _ "github.com/grafana/alloy/internal/component/all" ) +var ( + prometheusLegacyMetricValidationScheme = "legacy" + prometheusUTF8MetricValidationScheme = "utf-8" +) + func runCommand() *cobra.Command { r := &alloyRun{ inMemoryAddr: "alloy.internal:12345", @@ -64,6 +70,9 @@ func runCommand() *cobra.Command { clusterMaxJoinPeers: 5, clusterRejoinInterval: 60 * time.Second, disableSupportBundle: false, + // For backwards compatibility - use the LegacyValidation of Prometheus metrics name. This is a global variable + // setting that has changed upstream. See https://github.com/prometheus/common/pull/724. + prometheusMetricNameValidationScheme: prometheusLegacyMetricValidationScheme, } cmd := &cobra.Command{ @@ -155,38 +164,40 @@ depending on the nature of the reload error. cmd.Flags().StringVar(&r.storagePath, "storage.path", r.storagePath, "Base directory where components can store data") cmd.Flags().Var(&r.minStability, "stability.level", fmt.Sprintf("Minimum stability level of features to enable. Supported values: %s", strings.Join(featuregate.AllowedValues(), ", "))) cmd.Flags().BoolVar(&r.enableCommunityComps, "feature.community-components.enabled", r.enableCommunityComps, "Enable community components.") + cmd.Flags().StringVar(&r.prometheusMetricNameValidationScheme, "feature.prometheus.metric-validation-scheme", prometheusLegacyMetricValidationScheme, fmt.Sprintf("Prometheus metric validation scheme to use. Supported values: %q, %q. NOTE: this is an experimental flag and may be removed in future releases.", prometheusLegacyMetricValidationScheme, prometheusUTF8MetricValidationScheme)) addDeprecatedFlags(cmd) return cmd } type alloyRun struct { - inMemoryAddr string - httpListenAddr string - storagePath string - minStability featuregate.Stability - uiPrefix string - enablePprof bool - disableReporting bool - clusterEnabled bool - clusterNodeName string - clusterAdvAddr string - clusterJoinAddr string - clusterDiscoverPeers string - clusterAdvInterfaces []string - clusterRejoinInterval time.Duration - clusterMaxJoinPeers int - clusterName string - clusterEnableTLS bool - clusterTLSCAPath string - clusterTLSCertPath string - clusterTLSKeyPath string - clusterTLSServerName string - configFormat string - configBypassConversionErrors bool - configExtraArgs string - enableCommunityComps bool - disableSupportBundle bool + inMemoryAddr string + httpListenAddr string + storagePath string + minStability featuregate.Stability + uiPrefix string + enablePprof bool + disableReporting bool + clusterEnabled bool + clusterNodeName string + clusterAdvAddr string + clusterJoinAddr string + clusterDiscoverPeers string + clusterAdvInterfaces []string + clusterRejoinInterval time.Duration + clusterMaxJoinPeers int + clusterName string + clusterEnableTLS bool + clusterTLSCAPath string + clusterTLSCertPath string + clusterTLSKeyPath string + clusterTLSServerName string + configFormat string + configBypassConversionErrors bool + configExtraArgs string + enableCommunityComps bool + disableSupportBundle bool + prometheusMetricNameValidationScheme string } func (fr *alloyRun) Run(cmd *cobra.Command, configPath string) error { @@ -211,6 +222,10 @@ func (fr *alloyRun) Run(cmd *cobra.Command, configPath string) error { return fmt.Errorf("building tracer: %w", err) } + if err := fr.configurePrometheusMetricNameValidationScheme(l); err != nil { + return err + } + // Set the global tracer provider to catch global traces, but ideally things // use the tracer provider given to them so the appropriate attributes get // injected. @@ -435,6 +450,26 @@ func (fr *alloyRun) Run(cmd *cobra.Command, configPath string) error { } } +func (fr *alloyRun) configurePrometheusMetricNameValidationScheme(l log.Logger) error { + switch fr.prometheusMetricNameValidationScheme { + case prometheusLegacyMetricValidationScheme: + model.NameValidationScheme = model.LegacyValidation + case prometheusUTF8MetricValidationScheme: + if err := featuregate.CheckAllowed( + featuregate.StabilityExperimental, + fr.minStability, + "Prometheus utf-8 metric name validation scheme", + ); err != nil { + return err + } + level.Warn(l).Log("msg", "Using experimental UTF-8 Prometheus metric name validation scheme") + model.NameValidationScheme = model.UTF8Validation + default: + return fmt.Errorf("invalid prometheus metric name validation scheme: %q", fr.prometheusMetricNameValidationScheme) + } + return nil +} + // getEnabledComponentsFunc returns a function that gets the current enabled components func getEnabledComponentsFunc(f *alloy_runtime.Runtime) func() map[string]interface{} { return func() map[string]interface{} { diff --git a/internal/alloycli/cmd_run_test.go b/internal/alloycli/cmd_run_test.go new file mode 100644 index 0000000000..405085c90a --- /dev/null +++ b/internal/alloycli/cmd_run_test.go @@ -0,0 +1,72 @@ +package alloycli + +import ( + "testing" + + "github.com/go-kit/log" + "github.com/prometheus/common/model" + "github.com/stretchr/testify/require" + + "github.com/grafana/alloy/internal/featuregate" +) + +func TestConfigurePrometheusMetricNameValidationScheme(t *testing.T) { + tests := []struct { + name string + run alloyRun + expectedError string + expectedScheme model.ValidationScheme + }{ + { + name: "legacy validation scheme", + run: alloyRun{ + prometheusMetricNameValidationScheme: prometheusLegacyMetricValidationScheme, + minStability: featuregate.StabilityGenerallyAvailable, + }, + expectedScheme: model.LegacyValidation, + }, + { + name: "utf8 validation scheme with experimental stability", + run: alloyRun{ + prometheusMetricNameValidationScheme: prometheusUTF8MetricValidationScheme, + minStability: featuregate.StabilityExperimental, + }, + expectedScheme: model.UTF8Validation, + }, + { + name: "utf8 validation scheme with GA stability should fail", + run: alloyRun{ + prometheusMetricNameValidationScheme: prometheusUTF8MetricValidationScheme, + minStability: featuregate.StabilityGenerallyAvailable, + }, + expectedError: `Prometheus utf-8 metric name validation scheme is at stability level "experimental", which is below the minimum allowed stability level "generally-available"`, + }, + { + name: "invalid validation scheme", + run: alloyRun{ + prometheusMetricNameValidationScheme: "invalid", + minStability: featuregate.StabilityGenerallyAvailable, + }, + expectedError: `invalid prometheus metric name validation scheme: "invalid"`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Reset the global validation scheme before each test + defer func() { + model.NameValidationScheme = model.LegacyValidation + }() + + err := tc.run.configurePrometheusMetricNameValidationScheme(log.NewNopLogger()) + + if tc.expectedError != "" { + require.ErrorContains(t, err, tc.expectedError) + return + } + + require.NoError(t, err) + require.Equal(t, tc.expectedScheme, model.NameValidationScheme) + }) + } +}