diff --git a/.github/workflows/test-build-deploy.yml b/.github/workflows/test-build-deploy.yml index 9d510d58c9d..92653428b29 100644 --- a/.github/workflows/test-build-deploy.yml +++ b/.github/workflows/test-build-deploy.yml @@ -213,7 +213,7 @@ jobs: docker pull quay.io/cortexproject/cortex:v1.18.1 elif [ "$TEST_TAGS" = "integration_query_fuzz" ]; then docker pull quay.io/cortexproject/cortex:v1.18.1 - docker pull quay.io/prometheus/prometheus:v3.5.0 + docker pull quay.io/prometheus/prometheus:v3.6.0 fi docker pull memcached:1.6.1 docker pull redis:7.0.4-alpine diff --git a/CHANGELOG.md b/CHANGELOG.md index a5da0c1cf29..027f52728e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ * [FEATURE] Querier: Support for configuring query optimizers and enabling XFunctions in the Thanos engine. #6873 * [FEATURE] Query Frontend: Add support /api/v1/format_query API for formatting queries. #6893 * [FEATURE] Query Frontend: Add support for /api/v1/parse_query API (experimental) to parse a PromQL expression and return it as a JSON-formatted AST (abstract syntax tree). #6978 +* [ENHANCEMENT] Upgrade the Prometheus version to 3.6.0 and add a `-name-validation-scheme` flag to support UTF-8. #7040 #7056 * [ENHANCEMENT] Distributor: Emit an error with a 400 status code when empty labels are found before the relabelling or label dropping process. #7052 * [ENHANCEMENT] Parquet Storage: Add support for additional sort columns during Parquet file generation #7003 * [ENHANCEMENT] Modernizes the entire codebase by using go modernize tool. #7005 diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index b8f97501de3..3eb66084940 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -68,6 +68,11 @@ Where default_value is the value to use if the environment variable is undefined # CLI flag: -http.prefix [http_prefix: | default = "/api/prom"] +# Name validation scheme for metric names and label names, Support values are: +# legacy, utf8. +# CLI flag: -name-validation-scheme +[name_validation_scheme: | default = legacy] + resource_monitor: # Comma-separated list of resources to monitor. Supported values are cpu and # heap, which tracks metrics from github.com/prometheus/procfs and @@ -4344,11 +4349,6 @@ query_rejection: # list of rule groups to disable [disabled_rule_groups: | default = []] - -# Name validation scheme for metric names and label names, Support values are: -# legacy, utf8. -# CLI flag: -validation.name-validation-scheme -[name_validation_scheme: | default = legacy] ``` ### `memberlist_config` diff --git a/integration/e2e/images/images.go b/integration/e2e/images/images.go index 1ef0e8bbdec..19f9370edf5 100644 --- a/integration/e2e/images/images.go +++ b/integration/e2e/images/images.go @@ -11,5 +11,5 @@ var ( Minio = "minio/minio:RELEASE.2024-05-28T17-19-04Z" Consul = "consul:1.8.4" ETCD = "gcr.io/etcd-development/etcd:v3.4.7" - Prometheus = "quay.io/prometheus/prometheus:v3.5.0" + Prometheus = "quay.io/prometheus/prometheus:v3.6.0" ) diff --git a/integration/utf8_test.go b/integration/utf8_test.go index a7bdffee93b..17b0348bfec 100644 --- a/integration/utf8_test.go +++ b/integration/utf8_test.go @@ -4,11 +4,17 @@ package integration import ( + "context" "fmt" "path/filepath" "testing" "time" + amlabels "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/alertmanager/types" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/model/rulefmt" "github.com/prometheus/prometheus/prompb" "github.com/stretchr/testify/require" @@ -17,52 +23,156 @@ import ( "github.com/cortexproject/cortex/integration/e2ecortex" ) -func Test_RulerExternalLabels_UTF8Validation(t *testing.T) { +const utf8AlertmanagerConfig = `route: + receiver: dummy + group_by: [group.test.🙂] + routes: + - matchers: + - foo.🙂=bar.🙂 +receivers: + - name: dummy` + +func Test_Alertmanager_UTF8(t *testing.T) { s, err := e2e.NewScenario(networkName) require.NoError(t, err) defer s.Close() - // Start dependencies. - minio := e2edb.NewMinio(9000, bucketName) + flags := mergeFlags(AlertmanagerFlags(), AlertmanagerS3Flags(), + map[string]string{ + "-name-validation-scheme": "utf8", + }, + ) + + minio := e2edb.NewMinio(9000, alertsBucketName) require.NoError(t, s.StartAndWaitReady(minio)) + alertmanager := e2ecortex.NewAlertmanager( + "alertmanager", + flags, + "", + ) + + require.NoError(t, s.StartAndWaitReady(alertmanager)) + + c, err := e2ecortex.NewClient("", "", alertmanager.HTTPEndpoint(), "", "user-1") + require.NoError(t, err) + + ctx := context.Background() + + err = c.SetAlertmanagerConfig(ctx, utf8AlertmanagerConfig, map[string]string{}) + require.NoError(t, err) + + require.NoError(t, alertmanager.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_alertmanager_config_last_reload_successful"}, e2e.WaitMissingMetrics)) + require.NoError(t, alertmanager.WaitSumMetricsWithOptions(e2e.Greater(0), []string{"cortex_alertmanager_config_hash"}, e2e.WaitMissingMetrics)) + + silenceId, err := c.CreateSilence(ctx, types.Silence{ + Matchers: amlabels.Matchers{ + {Name: "silences.name.🙂", Value: "silences.value.🙂"}, + }, + Comment: "test silences", + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Minute), + }) + require.NoError(t, err) + require.NotEmpty(t, silenceId) + require.NoError(t, alertmanager.WaitSumMetrics(e2e.Equals(1), "cortex_alertmanager_silences")) + + err = c.SendAlertToAlermanager(ctx, &model.Alert{ + Labels: model.LabelSet{ + "alert.name.🙂": "alert.value.🙂", + }, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Minute), + }) + require.NoError(t, err) + require.NoError(t, alertmanager.WaitSumMetrics(e2e.Equals(1), "cortex_alertmanager_alerts_received_total")) +} + +func Test_Ruler_UTF8(t *testing.T) { + s, err := e2e.NewScenario(networkName) + require.NoError(t, err) + defer s.Close() + + // Start dependencies. + consul := e2edb.NewConsul() + minio := e2edb.NewMinio(9000, rulestoreBucketName, bucketName) + require.NoError(t, s.StartAndWaitReady(consul, minio)) + runtimeConfigYamlFile := ` overrides: - 'user-2': - name_validation_scheme: utf8 + 'user-1': ruler_external_labels: test.utf8.metric: 😄 ` require.NoError(t, writeFileToSharedDir(s, runtimeConfigFile, []byte(runtimeConfigYamlFile))) filePath := filepath.Join(e2e.ContainerSharedDir, runtimeConfigFile) - // Start Cortex components. - require.NoError(t, copyFileToSharedDir(s, "docs/configuration/single-process-config-blocks.yaml", cortexConfigFile)) + flags := mergeFlags( + BlocksStorageFlags(), + RulerFlags(), + map[string]string{ + "-runtime-config.file": filePath, + "-runtime-config.backend": "filesystem", + "-name-validation-scheme": "utf8", - flags := map[string]string{ - "-auth.enabled": "true", - "-runtime-config.file": filePath, - "-runtime-config.backend": "filesystem", - // ingester - "-blocks-storage.s3.access-key-id": e2edb.MinioAccessKey, - "-blocks-storage.s3.secret-access-key": e2edb.MinioSecretKey, - "-blocks-storage.s3.bucket-name": bucketName, - "-blocks-storage.s3.endpoint": fmt.Sprintf("%s-minio-9000:9000", networkName), - "-blocks-storage.s3.insecure": "true", - // alert manager - "-alertmanager.web.external-url": "http://localhost/alertmanager", - "-alertmanager-storage.backend": "local", - "-alertmanager-storage.local.path": filepath.Join(e2e.ContainerSharedDir, "alertmanager_configs"), - } + "-ring.store": "consul", + "-consul.hostname": consul.NetworkHTTPEndpoint(), + // ingester + "-blocks-storage.s3.access-key-id": e2edb.MinioAccessKey, + "-blocks-storage.s3.secret-access-key": e2edb.MinioSecretKey, + "-blocks-storage.s3.bucket-name": bucketName, + "-blocks-storage.s3.endpoint": fmt.Sprintf("%s-minio-9000:9000", networkName), + "-blocks-storage.s3.insecure": "true", + // alert manager + "-alertmanager.web.external-url": "http://localhost/alertmanager", + "-alertmanager-storage.backend": "local", + "-alertmanager-storage.local.path": filepath.Join(e2e.ContainerSharedDir, "alertmanager_configs"), + }, + ) // make alert manager config dir require.NoError(t, writeFileToSharedDir(s, "alertmanager_configs", []byte{})) - // The external labels validation should be success - cortex := e2ecortex.NewSingleBinaryWithConfigFile("cortex-1", cortexConfigFile, flags, "", 9009, 9095) + // Start Cortex. + cortex := e2ecortex.NewSingleBinary("cortex", flags, "") require.NoError(t, s.StartAndWaitReady(cortex)) + + groupLabels := map[string]string{ + "group.label.😄": "group.value", + } + ruleLabels := map[string]string{ + "rule.label.😄": "rule.value", + } + + interval, _ := model.ParseDuration("1s") + + ruleGroup := rulefmt.RuleGroup{ + Name: "rule.utf8.😄", + Interval: interval, + Rules: []rulefmt.Rule{{ + Alert: "alert.rule.😄", + Expr: "up", + Labels: ruleLabels, + }, { + Record: "record.rule.😄", + Expr: "up", + Labels: ruleLabels, + }}, + Labels: groupLabels, + } + + c, err := e2ecortex.NewClient("", "", "", cortex.HTTPEndpoint(), "user-1") + require.NoError(t, err) + + err = c.SetRuleGroup(ruleGroup, "namespace") + require.NoError(t, err) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_ruler_managers_total"}), e2e.WithLabelMatchers(labels.MustNewMatcher(labels.MatchEqual, "user", "user-1"))) + require.NoError(t, cortex.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_ruler_rule_groups_in_store"}, e2e.WithLabelMatchers( + labels.MustNewMatcher(labels.MatchEqual, "user", "user-1")), + e2e.WaitMissingMetrics, + )) } -func Test_Distributor_UTF8ValidationPerTenant(t *testing.T) { +func Test_PushQuery_UTF8(t *testing.T) { s, err := e2e.NewScenario(networkName) require.NoError(t, err) defer s.Close() @@ -71,22 +181,12 @@ func Test_Distributor_UTF8ValidationPerTenant(t *testing.T) { minio := e2edb.NewMinio(9000, bucketName) require.NoError(t, s.StartAndWaitReady(minio)) - runtimeConfigYamlFile := ` -overrides: - 'user-2': - name_validation_scheme: utf8 -` - - require.NoError(t, writeFileToSharedDir(s, runtimeConfigFile, []byte(runtimeConfigYamlFile))) - filePath := filepath.Join(e2e.ContainerSharedDir, runtimeConfigFile) - // Start Cortex components. require.NoError(t, copyFileToSharedDir(s, "docs/configuration/single-process-config-blocks.yaml", cortexConfigFile)) flags := map[string]string{ "-auth.enabled": "true", - "-runtime-config.file": filePath, - "-runtime-config.backend": "filesystem", + "-name-validation-scheme": "utf8", // ingester "-blocks-storage.s3.access-key-id": e2edb.MinioAccessKey, "-blocks-storage.s3.secret-access-key": e2edb.MinioSecretKey, @@ -104,34 +204,62 @@ overrides: cortex := e2ecortex.NewSingleBinaryWithConfigFile("cortex-1", cortexConfigFile, flags, "", 9009, 9095) require.NoError(t, s.StartAndWaitReady(cortex)) - // user-1 uses legacy validation - user1Client, err := e2ecortex.NewClient(cortex.HTTPEndpoint(), "", "", "", "user-1") - require.NoError(t, err) - - // user-2 uses utf8 validation - user2Client, err := e2ecortex.NewClient(cortex.HTTPEndpoint(), "", "", "", "user-2") + c, err := e2ecortex.NewClient(cortex.HTTPEndpoint(), cortex.HTTPEndpoint(), "", "", "user-1") require.NoError(t, err) now := time.Now() - utf8Series, _ := generateSeries("series_1", now, prompb.Label{Name: "test.utf8.metric", Value: "😄"}) + utf8Series, _ := generateSeries("series.1", now, prompb.Label{Name: "test.utf8.metric", Value: "😄"}) legacySeries, _ := generateSeries("series_2", now, prompb.Label{Name: "job", Value: "test"}) - res, err := user1Client.Push(legacySeries) + metadata := []prompb.MetricMetadata{ + { + MetricFamilyName: "metadata.name", + Help: "metadata.help", + Unit: "metadata.unit", + }, + } + + res, err := c.Push(legacySeries, metadata...) require.NoError(t, err) require.Equal(t, 200, res.StatusCode) - // utf8Series push should be fail for user-1 - res, err = user1Client.Push(utf8Series) + // utf8Series push should be success + res, err = c.Push(utf8Series) require.NoError(t, err) - require.Equal(t, 400, res.StatusCode) + require.Equal(t, 200, res.StatusCode) - res, err = user2Client.Push(legacySeries) + // utf8 querying + // c.f. https://prometheus.io/docs/guides/utf8/#querying + query := `{"series.1", "test.utf8.metric"="😄"}` + queryResult, err := c.Query(query, now) require.NoError(t, err) require.Equal(t, 200, res.StatusCode) + require.Equal(t, model.ValVector, queryResult.Type()) + vec := queryResult.(model.Vector) + require.Equal(t, 1, len(vec)) - // utf8Series push should be success for user-2 - res, err = user2Client.Push(utf8Series) + // label names + start := now + end := now.Add(time.Minute * 5) + labelNames, err := c.LabelNames(start, end) require.NoError(t, err) - require.Equal(t, 200, res.StatusCode) + require.Equal(t, []string{"__name__", "job", "test.utf8.metric"}, labelNames) + + // series + series, err := c.Series([]string{`{"test.utf8.metric"="😄"}`}, start, end) + require.NoError(t, err) + require.Equal(t, 1, len(series)) + require.Equal(t, `{__name__="series.1", test.utf8.metric="😄"}`, series[0].String()) + + // label values + labelValues, err := c.LabelValues("test.utf8.metric", start, end, nil) + require.NoError(t, err) + require.Equal(t, 1, len(labelValues)) + require.Equal(t, model.LabelValue("😄"), labelValues[0]) + + // metadata + metadataResult, err := c.Metadata("metadata.name", "") + require.NoError(t, err) + require.Equal(t, 1, len(metadataResult)) } diff --git a/pkg/cortex/configinit/init.go b/pkg/cortex/configinit/init.go deleted file mode 100644 index bfb180797bc..00000000000 --- a/pkg/cortex/configinit/init.go +++ /dev/null @@ -1,8 +0,0 @@ -package configinit - -import "github.com/prometheus/common/model" - -func init() { - // nolint:staticcheck - model.NameValidationScheme = model.LegacyValidation -} diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index e7575abdcee..8ff285d6d8e 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -15,6 +15,7 @@ import ( "github.com/go-kit/log/level" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" prom_storage "github.com/prometheus/prometheus/storage" "github.com/weaveworks/common/server" "github.com/weaveworks/common/signals" @@ -31,7 +32,6 @@ import ( "github.com/cortexproject/cortex/pkg/configs" configAPI "github.com/cortexproject/cortex/pkg/configs/api" "github.com/cortexproject/cortex/pkg/configs/db" - _ "github.com/cortexproject/cortex/pkg/cortex/configinit" "github.com/cortexproject/cortex/pkg/cortex/storage" "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/distributor" @@ -91,10 +91,12 @@ var ( // Config is the root config for Cortex. type Config struct { - Target flagext.StringSliceCSV `yaml:"target"` - AuthEnabled bool `yaml:"auth_enabled"` - PrintConfig bool `yaml:"-"` - HTTPPrefix string `yaml:"http_prefix"` + Target flagext.StringSliceCSV `yaml:"target"` + AuthEnabled bool `yaml:"auth_enabled"` + PrintConfig bool `yaml:"-"` + HTTPPrefix string `yaml:"http_prefix"` + NameValidationScheme model.ValidationScheme `yaml:"name_validation_scheme"` + ResourceMonitor configs.ResourceMonitor `yaml:"resource_monitor"` ExternalQueryable prom_storage.Queryable `yaml:"-"` @@ -145,6 +147,8 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { "Use '-modules' command line flag to get a list of available modules, and to see which modules are included in 'all'.") f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.") + _ = c.NameValidationScheme.Set(model.LegacyValidation.String()) + f.Var(&c.NameValidationScheme, "name-validation-scheme", fmt.Sprintf("Name validation scheme for metric names and label names, Support values are: %s.", strings.Join([]string{model.LegacyValidation.String(), model.UTF8Validation.String()}, ", "))) f.BoolVar(&c.PrintConfig, "print.config", false, "Print the config and exit.") f.StringVar(&c.HTTPPrefix, "http.prefix", "/api/prom", "HTTP path prefix for Cortex API.") @@ -186,6 +190,12 @@ func (c *Config) Validate(log log.Logger) error { return err } + switch c.NameValidationScheme { + case model.LegacyValidation, model.UTF8Validation: + default: + return fmt.Errorf("unsupported name validation scheme: %s", c.NameValidationScheme) + } + if c.HTTPPrefix != "" && !strings.HasPrefix(c.HTTPPrefix, "/") { return errInvalidHTTPPrefix } @@ -205,7 +215,7 @@ func (c *Config) Validate(log log.Logger) error { if err := c.BlocksStorage.Validate(); err != nil { return errors.Wrap(err, "invalid TSDB config") } - if err := c.LimitsConfig.Validate(c.Distributor.ShardByAllLabels, c.Ingester.ActiveSeriesMetricsEnabled); err != nil { + if err := c.LimitsConfig.Validate(c.NameValidationScheme, c.Distributor.ShardByAllLabels, c.Ingester.ActiveSeriesMetricsEnabled); err != nil { return errors.Wrap(err, "invalid limits config") } if err := c.ResourceMonitor.Validate(); err != nil { @@ -372,6 +382,9 @@ func New(cfg Config) (*Cortex, error) { Cfg: cfg, } + // set name validation scheme + model.NameValidationScheme = cfg.NameValidationScheme //nolint:staticcheck + cortex.setupThanosTracing() cortex.setupGRPCHeaderForwarding() cortex.setupRequestSigning() diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 6cac224319d..a044a62ae35 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/weaveworks/common/server" @@ -45,6 +46,7 @@ func TestCortex(t *testing.T) { // these values are set as defaults but since we aren't registering them, we // need to include the defaults here. These were hardcoded in a previous version // of weaveworks server. + NameValidationScheme: model.LegacyValidation, Server: server.Config{ GRPCListenNetwork: server.DefaultNetwork, HTTPListenNetwork: server.DefaultNetwork, @@ -217,6 +219,24 @@ func TestConfigValidation(t *testing.T) { }, expectedError: nil, }, + { + name: "should pass utf8 name validation scheme", + getTestConfig: func() *Config { + configuration := newDefaultConfig() + configuration.NameValidationScheme = model.UTF8Validation + return configuration + }, + expectedError: nil, + }, + { + name: "should fail unset name validation scheme", + getTestConfig: func() *Config { + configuration := newDefaultConfig() + configuration.NameValidationScheme = model.UnsetValidation + return configuration + }, + expectedError: fmt.Errorf("unsupported name validation scheme: unset"), + }, } { t.Run(tc.name, func(t *testing.T) { err := tc.getTestConfig().Validate(nil) @@ -233,9 +253,10 @@ func TestGrpcAuthMiddleware(t *testing.T) { prepareGlobalMetricsRegistry(t) cfg := Config{ - AuthEnabled: true, // We must enable this to enable Auth middleware for gRPC server. - Server: getServerConfig(t), - Target: []string{API}, // Something innocent that doesn't require much config. + AuthEnabled: true, // We must enable this to enable Auth middleware for gRPC server. + Server: getServerConfig(t), + Target: []string{API}, // Something innocent that doesn't require much config. + NameValidationScheme: model.LegacyValidation, } msch := &mockGrpcServiceHandler{} diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index 013dbb90834..35511562150 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -13,7 +13,10 @@ import ( "github.com/opentracing-contrib/go-stdlib/nethttp" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" + "github.com/prometheus/alertmanager/featurecontrol" + "github.com/prometheus/alertmanager/matcher/compat" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/rules" prom_storage "github.com/prometheus/prometheus/storage" @@ -225,6 +228,7 @@ func (t *Cortex) initOverridesExporter() (services.Service, error) { func (t *Cortex) initDistributorService() (serv services.Service, err error) { t.Cfg.Distributor.DistributorRing.ListenPort = t.Cfg.Server.GRPCListenPort t.Cfg.Distributor.ShuffleShardingLookbackPeriod = t.Cfg.Querier.ShuffleShardingIngestersLookbackPeriod + t.Cfg.Distributor.NameValidationScheme = t.Cfg.NameValidationScheme t.Cfg.IngesterClient.GRPCClientConfig.SignWriteRequestsEnabled = t.Cfg.Distributor.SignWriteRequestsEnabled // Check whether the distributor can join the distributors ring, which is @@ -723,6 +727,22 @@ func (t *Cortex) initConfig() (serv services.Service, err error) { func (t *Cortex) initAlertManager() (serv services.Service, err error) { t.Cfg.Alertmanager.ShardingRing.ListenPort = t.Cfg.Server.GRPCListenPort + var featureControlMode string + switch t.Cfg.NameValidationScheme { + case model.LegacyValidation: + featureControlMode = featurecontrol.FeatureClassicMode + case model.UTF8Validation: + featureControlMode = featurecontrol.FeatureUTF8StrictMode + default: + return nil, fmt.Errorf("invalid validation scheme: %s", t.Cfg.NameValidationScheme) + } + + features, err := featurecontrol.NewFlags(util_log.SLogger, featureControlMode) + if err != nil { + return + } + compat.InitFromFlags(util_log.SLogger, features) + // Initialise the store. store, err := alertstore.NewAlertStore(context.Background(), t.Cfg.AlertmanagerStorage, t.Overrides, util_log.Logger, prometheus.DefaultRegisterer) if err != nil { diff --git a/pkg/cortex/runtime_config.go b/pkg/cortex/runtime_config.go index 974e22f04bd..2b72b9e5e1e 100644 --- a/pkg/cortex/runtime_config.go +++ b/pkg/cortex/runtime_config.go @@ -87,7 +87,7 @@ func (l runtimeConfigLoader) load(r io.Reader) (any, error) { // refer to https://github.com/cortexproject/cortex/issues/6741#issuecomment-3067244929 if overrides != nil { for _, ul := range overrides.TenantLimits { - if err := ul.Validate(l.cfg.Distributor.ShardByAllLabels, l.cfg.Ingester.ActiveSeriesMetricsEnabled); err != nil { + if err := ul.Validate(l.cfg.NameValidationScheme, l.cfg.Distributor.ShardByAllLabels, l.cfg.Ingester.ActiveSeriesMetricsEnabled); err != nil { return nil, err } } diff --git a/pkg/distributor/distributor.go b/pkg/distributor/distributor.go index 4766afdd9e0..cfdb5f47fb1 100644 --- a/pkg/distributor/distributor.go +++ b/pkg/distributor/distributor.go @@ -185,6 +185,9 @@ type Config struct { // OTLPConfig OTLPConfig OTLPConfig `yaml:"otlp"` + + // Inject from global config + NameValidationScheme model.ValidationScheme `yaml:"-"` } type InstanceLimits struct { @@ -629,7 +632,7 @@ func (d *Distributor) checkSample(ctx context.Context, userID, cluster, replica func (d *Distributor) validateSeries(ts cortexpb.PreallocTimeseries, userID string, skipLabelNameValidation bool, limits *validation.Limits) (cortexpb.PreallocTimeseries, validation.ValidationError) { d.labelsHistogram.Observe(float64(len(ts.Labels))) - if err := validation.ValidateLabels(d.validateMetrics, limits, userID, ts.Labels, skipLabelNameValidation); err != nil { + if err := validation.ValidateLabels(d.validateMetrics, limits, userID, ts.Labels, skipLabelNameValidation, d.cfg.NameValidationScheme); err != nil { return emptyPreallocSeries, err } diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index f68cd116aab..6757585bad8 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -36,7 +36,6 @@ import ( "google.golang.org/grpc/status" promchunk "github.com/cortexproject/cortex/pkg/chunk/encoding" - _ "github.com/cortexproject/cortex/pkg/cortex/configinit" "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ha" "github.com/cortexproject/cortex/pkg/ingester" @@ -3079,6 +3078,7 @@ type prepConfig struct { errFail error tokens [][]uint32 useStreamPush bool + nameValidationScheme model.ValidationScheme } type prepState struct { @@ -3194,6 +3194,10 @@ func prepare(tb testing.TB, cfg prepConfig) ([]*Distributor, []*mockIngester, [] distributorCfg.InstanceLimits.MaxInflightClientRequests = cfg.maxInflightClientRequests distributorCfg.InstanceLimits.MaxIngestionRate = cfg.maxIngestionRate distributorCfg.UseStreamPush = cfg.useStreamPush + distributorCfg.NameValidationScheme = model.LegacyValidation + if cfg.nameValidationScheme == model.UTF8Validation { + distributorCfg.NameValidationScheme = cfg.nameValidationScheme + } if cfg.shuffleShardEnabled { distributorCfg.ShardingStrategy = util.ShardingStrategyShuffle @@ -4107,6 +4111,7 @@ func TestDistributor_Push_Relabel(t *testing.T) { inputSeries []labels.Labels expectedSeries labels.Labels metricRelabelConfigs []*relabel.Config + nameValidationScheme model.ValidationScheme } cases := []testcase{ @@ -4160,11 +4165,12 @@ func TestDistributor_Push_Relabel(t *testing.T) { limits.MetricRelabelConfigs = tc.metricRelabelConfigs ds, ingesters, _, _ := prepare(t, prepConfig{ - numIngesters: 2, - happyIngesters: 2, - numDistributors: 1, - shardByAllLabels: true, - limits: &limits, + numIngesters: 2, + happyIngesters: 2, + numDistributors: 1, + shardByAllLabels: true, + limits: &limits, + nameValidationScheme: tc.nameValidationScheme, }) // Push the series to the distributor diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index 1f2bc794a1a..16b956bef1b 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -245,8 +245,6 @@ type Limits struct { AlertmanagerMaxSilencesCount int `yaml:"alertmanager_max_silences_count" json:"alertmanager_max_silences_count"` AlertmanagerMaxSilencesSizeBytes int `yaml:"alertmanager_max_silences_size_bytes" json:"alertmanager_max_silences_size_bytes"` DisabledRuleGroups DisabledRuleGroups `yaml:"disabled_rule_groups" json:"disabled_rule_groups" doc:"nocli|description=list of rule groups to disable"` - - NameValidationScheme model.ValidationScheme `yaml:"name_validation_scheme" json:"name_validation_scheme"` } // RegisterFlags adds the flags required to config this to the given FlagSet @@ -356,14 +354,11 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) { f.IntVar(&l.AlertmanagerMaxAlertsSizeBytes, "alertmanager.max-alerts-size-bytes", 0, "Maximum total size of alerts that a single user can have, alert size is the sum of the bytes of its labels, annotations and generatorURL. Inserting more alerts will fail with a log message and metric increment. 0 = no limit.") f.IntVar(&l.AlertmanagerMaxSilencesCount, "alertmanager.max-silences-count", 0, "Maximum number of silences that a single user can have, including expired silences. 0 = no limit.") f.IntVar(&l.AlertmanagerMaxSilencesSizeBytes, "alertmanager.max-silences-size-bytes", 0, "Maximum size of individual silences that a single user can have. 0 = no limit.") - - _ = l.NameValidationScheme.Set(model.LegacyValidation.String()) - f.Var(&l.NameValidationScheme, "validation.name-validation-scheme", fmt.Sprintf("Name validation scheme for metric names and label names, Support values are: %s.", strings.Join([]string{model.LegacyValidation.String(), model.UTF8Validation.String()}, ", "))) } // Validate the limits config and returns an error if the validation // doesn't pass -func (l *Limits) Validate(shardByAllLabels bool, activeSeriesMetricsEnabled bool) error { +func (l *Limits) Validate(nameValidationScheme model.ValidationScheme, shardByAllLabels bool, activeSeriesMetricsEnabled bool) error { // The ingester.max-global-series-per-user metric is not supported // if shard-by-all-labels is disabled if l.MaxGlobalSeriesPerUser > 0 && !shardByAllLabels { @@ -381,16 +376,6 @@ func (l *Limits) Validate(shardByAllLabels bool, activeSeriesMetricsEnabled bool return errMaxLocalNativeHistogramSeriesPerUserValidation } - var nameValidationScheme model.ValidationScheme - switch l.NameValidationScheme { - case model.LegacyValidation, model.UTF8Validation: - nameValidationScheme = l.NameValidationScheme - case model.UnsetValidation: - nameValidationScheme = model.LegacyValidation - default: - return fmt.Errorf("unsupported name validation scheme: %s", l.NameValidationScheme) - } - if err := l.RulerExternalLabels.Validate(func(l labels.Label) error { if !nameValidationScheme.IsValidLabelName(l.Name) { return fmt.Errorf("%w: %q", errInvalidLabelName, l.Name) diff --git a/pkg/util/validation/limits_test.go b/pkg/util/validation/limits_test.go index 2438e6ae2e8..39c0204df6e 100644 --- a/pkg/util/validation/limits_test.go +++ b/pkg/util/validation/limits_test.go @@ -49,6 +49,7 @@ func TestLimits_Validate(t *testing.T) { shardByAllLabels bool activeSeriesMetricsEnabled bool expected error + nameValidationScheme model.ValidationScheme }{ "max-global-series-per-user disabled and shard-by-all-labels=false": { limits: Limits{MaxGlobalSeriesPerUser: 0}, @@ -124,23 +125,29 @@ func TestLimits_Validate(t *testing.T) { expected: errInvalidLabelValue, }, "utf8: external-labels utf8 label name and value": { - limits: Limits{NameValidationScheme: model.UTF8Validation, RulerExternalLabels: labels.FromStrings("test.utf8.metric", "😄")}, - expected: nil, + limits: Limits{RulerExternalLabels: labels.FromStrings("test.utf8.metric", "😄")}, + expected: nil, + nameValidationScheme: model.UTF8Validation, }, "utf8: external-labels invalid label name": { - limits: Limits{NameValidationScheme: model.UTF8Validation, RulerExternalLabels: labels.FromStrings("test.\xc5.metric", "😄")}, - expected: errInvalidLabelName, + limits: Limits{RulerExternalLabels: labels.FromStrings("test.\xc5.metric", "😄")}, + expected: errInvalidLabelName, + nameValidationScheme: model.UTF8Validation, }, "utf8: external-labels invalid label value": { - limits: Limits{NameValidationScheme: model.UTF8Validation, RulerExternalLabels: labels.FromStrings("test.utf8.metric", "test.\xc5.value")}, - expected: errInvalidLabelValue, + limits: Limits{RulerExternalLabels: labels.FromStrings("test.utf8.metric", "test.\xc5.value")}, + expected: errInvalidLabelValue, + nameValidationScheme: model.UTF8Validation, }, } for testName, testData := range tests { - t.Run(testName, func(t *testing.T) { - assert.ErrorIs(t, testData.limits.Validate(testData.shardByAllLabels, testData.activeSeriesMetricsEnabled), testData.expected) + nameValidationScheme := model.LegacyValidation + if testData.nameValidationScheme == model.UTF8Validation { + nameValidationScheme = testData.nameValidationScheme + } + assert.ErrorIs(t, testData.limits.Validate(nameValidationScheme, testData.shardByAllLabels, testData.activeSeriesMetricsEnabled), testData.expected) }) } } diff --git a/pkg/util/validation/validate.go b/pkg/util/validation/validate.go index eb3a460fbe5..436cf3dfab3 100644 --- a/pkg/util/validation/validate.go +++ b/pkg/util/validation/validate.go @@ -277,7 +277,7 @@ func ValidateExemplar(validateMetrics *ValidateMetrics, userID string, ls []cort // ValidateLabels returns an err if the labels are invalid. // The returned error may retain the provided series labels. -func ValidateLabels(validateMetrics *ValidateMetrics, limits *Limits, userID string, ls []cortexpb.LabelAdapter, skipLabelNameValidation bool) ValidationError { +func ValidateLabels(validateMetrics *ValidateMetrics, limits *Limits, userID string, ls []cortexpb.LabelAdapter, skipLabelNameValidation bool, nameValidationScheme model.ValidationScheme) ValidationError { if limits.EnforceMetricName { unsafeMetricName, err := extract.UnsafeMetricNameFromLabelAdapters(ls) if err != nil { @@ -285,7 +285,7 @@ func ValidateLabels(validateMetrics *ValidateMetrics, limits *Limits, userID str return newNoMetricNameError() } - if !limits.NameValidationScheme.IsValidLabelName(unsafeMetricName) { + if !nameValidationScheme.IsValidLabelName(unsafeMetricName) { validateMetrics.DiscardedSamples.WithLabelValues(invalidMetricName, userID).Inc() return newInvalidMetricNameError(unsafeMetricName) } @@ -304,7 +304,7 @@ func ValidateLabels(validateMetrics *ValidateMetrics, limits *Limits, userID str labelsSizeBytes := 0 for _, l := range ls { - if !skipLabelNameValidation && !limits.NameValidationScheme.IsValidLabelName(l.Name) { + if !skipLabelNameValidation && !nameValidationScheme.IsValidLabelName(l.Name) { validateMetrics.DiscardedSamples.WithLabelValues(invalidLabel, userID).Inc() return newInvalidLabelError(ls, l.Name) } else if len(l.Name) > maxLabelNameLength { diff --git a/pkg/util/validation/validate_test.go b/pkg/util/validation/validate_test.go index a2e2825a38f..861934eb667 100644 --- a/pkg/util/validation/validate_test.go +++ b/pkg/util/validation/validate_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/require" "github.com/weaveworks/common/httpgrpc" - _ "github.com/cortexproject/cortex/pkg/cortex/configinit" "github.com/cortexproject/cortex/pkg/cortexpb" util_log "github.com/cortexproject/cortex/pkg/util/log" ) @@ -33,7 +32,6 @@ func TestValidateLabels_UTF8(t *testing.T) { cfg.MaxLabelNamesPerSeries = 2 cfg.MaxLabelsSizeBytes = 90 cfg.EnforceMetricName = true - cfg.NameValidationScheme = model.UTF8Validation tests := []struct { description string @@ -78,7 +76,7 @@ func TestValidateLabels_UTF8(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { - err := ValidateLabels(validateMetrics, cfg, userID, cortexpb.FromMetricsToLabelAdapters(test.metric), test.skipLabelNameValidation) + err := ValidateLabels(validateMetrics, cfg, userID, cortexpb.FromMetricsToLabelAdapters(test.metric), test.skipLabelNameValidation, model.UTF8Validation) assert.Equal(t, test.expectedErr, err, "wrong error") }) } @@ -96,7 +94,6 @@ func TestValidateLabels(t *testing.T) { cfg.MaxLabelNamesPerSeries = 2 cfg.MaxLabelsSizeBytes = 90 cfg.EnforceMetricName = true - cfg.NameValidationScheme = model.LegacyValidation cfg.LimitsPerLabelSet = []LimitsPerLabelSet{ { Limits: LimitsPerLabelSetEntry{MaxSeries: 0}, @@ -180,7 +177,7 @@ func TestValidateLabels(t *testing.T) { nil, }, } { - err := ValidateLabels(validateMetrics, cfg, userID, cortexpb.FromMetricsToLabelAdapters(c.metric), c.skipLabelNameValidation) + err := ValidateLabels(validateMetrics, cfg, userID, cortexpb.FromMetricsToLabelAdapters(c.metric), c.skipLabelNameValidation, model.LegacyValidation) assert.Equal(t, c.err, err, "wrong error") } @@ -335,7 +332,6 @@ func TestValidateLabelOrder(t *testing.T) { cfg.MaxLabelNameLength = 10 cfg.MaxLabelNamesPerSeries = 10 cfg.MaxLabelValueLength = 10 - cfg.NameValidationScheme = model.LegacyValidation reg := prometheus.NewRegistry() validateMetrics := NewValidateMetrics(reg) userID := "testUser" @@ -344,7 +340,7 @@ func TestValidateLabelOrder(t *testing.T) { {Name: model.MetricNameLabel, Value: "m"}, {Name: "b", Value: "b"}, {Name: "a", Value: "a"}, - }, false) + }, false, model.LegacyValidation) expected := newLabelsNotSortedError([]cortexpb.LabelAdapter{ {Name: model.MetricNameLabel, Value: "m"}, {Name: "b", Value: "b"}, @@ -364,7 +360,6 @@ func TestValidateLabelDuplication(t *testing.T) { cfg.MaxLabelNameLength = 10 cfg.MaxLabelNamesPerSeries = 10 cfg.MaxLabelValueLength = 10 - cfg.NameValidationScheme = model.LegacyValidation reg := prometheus.NewRegistry() validateMetrics := NewValidateMetrics(reg) userID := "testUser" @@ -372,7 +367,7 @@ func TestValidateLabelDuplication(t *testing.T) { actual := ValidateLabels(validateMetrics, cfg, userID, []cortexpb.LabelAdapter{ {Name: model.MetricNameLabel, Value: "a"}, {Name: model.MetricNameLabel, Value: "b"}, - }, false) + }, false, model.LegacyValidation) expected := newDuplicatedLabelError([]cortexpb.LabelAdapter{ {Name: model.MetricNameLabel, Value: "a"}, {Name: model.MetricNameLabel, Value: "b"}, @@ -383,7 +378,7 @@ func TestValidateLabelDuplication(t *testing.T) { {Name: model.MetricNameLabel, Value: "a"}, {Name: "a", Value: "a"}, {Name: "a", Value: "a"}, - }, false) + }, false, model.LegacyValidation) expected = newDuplicatedLabelError([]cortexpb.LabelAdapter{ {Name: model.MetricNameLabel, Value: "a"}, {Name: "a", Value: "a"}, diff --git a/schemas/cortex-config-schema.json b/schemas/cortex-config-schema.json index 159bfda0a28..4f3206c3f62 100644 --- a/schemas/cortex-config-schema.json +++ b/schemas/cortex-config-schema.json @@ -5184,12 +5184,6 @@ "description": "List of metric relabel configurations. Note that in most situations, it is more effective to use metrics relabeling directly in the Prometheus server, e.g. remote_write.write_relabel_configs.", "type": "string" }, - "name_validation_scheme": { - "default": "legacy", - "description": "Name validation scheme for metric names and label names, Support values are: legacy, utf8.", - "type": "number", - "x-cli-flag": "validation.name-validation-scheme" - }, "native_histogram_ingestion_burst_size": { "default": 0, "description": "Per-user allowed native histogram ingestion burst size (in number of samples)", @@ -8503,6 +8497,12 @@ "memberlist": { "$ref": "#/definitions/memberlist_config" }, + "name_validation_scheme": { + "default": "legacy", + "description": "Name validation scheme for metric names and label names, Support values are: legacy, utf8.", + "type": "number", + "x-cli-flag": "name-validation-scheme" + }, "parquet_converter": { "properties": { "conversion_interval": {