Skip to content

Commit

Permalink
fix breaking change in prom metrics validation scheme (#2781) (#2783)
Browse files Browse the repository at this point in the history
* fix breaking change in prom metrics validation scheme

* add test and changelog

* punctuation
  • Loading branch information
thampiotr authored Feb 20, 2025
1 parent 1a5d6c7 commit 658f837
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 26 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------

Expand Down
1 change: 1 addition & 0 deletions docs/sources/reference/cli/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
87 changes: 61 additions & 26 deletions internal/alloycli/cmd_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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{} {
Expand Down
72 changes: 72 additions & 0 deletions internal/alloycli/cmd_run_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

0 comments on commit 658f837

Please sign in to comment.