diff --git a/CHANGELOG.md b/CHANGELOG.md index b92051b7038..f547afa70e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ * [ENHANCEMENT] OTLP: Add experimental metric `cortex_distributor_otlp_array_lengths` to better understand the layout of OTLP packets in practice. #13525 * [ENHANCEMENT] Ruler: gRPC errors without details are classified as `operator` errors, and rule evaluation failures (such as duplicate labelsets) are classified as `user` errors. #13586 * [ENHANCEMENT] Server: The `/metrics` endpoint now supports metrics filtering by providing one or more `name[]` query parameters. #13746 +* [ENHANCEMENT] Distributor: Improved the performance of configuration retrieval in the validation middleware. #13807 * [ENHANCEMENT] Ingester: Make sharded active-series requests matching all series faster. #13491 * [BUGFIX] Compactor: Fix potential concurrent map writes. #13053 * [BUGFIX] Query-frontend: Fix issue where queries sometimes fail with `failed to receive query result stream message: rpc error: code = Canceled desc = context canceled` if remote execution is enabled. #13084 diff --git a/pkg/distributor/distributor.go b/pkg/distributor/distributor.go index e8c19fbf90c..1b909cd5b49 100644 --- a/pkg/distributor/distributor.go +++ b/pkg/distributor/distributor.go @@ -933,7 +933,7 @@ func (d *Distributor) checkSample(ctx context.Context, userID, cluster, replica // Returns an error explaining the first validation finding. // May alter timeseries data in-place. // The returned error MUST NOT retain label strings - they point into a gRPC buffer which is re-used. -func (d *Distributor) validateSamples(now model.Time, ts *mimirpb.PreallocTimeseries, userID, group string) error { +func (d *Distributor) validateSamples(now model.Time, ts *mimirpb.PreallocTimeseries, userID, group string, cfg sampleValidationConfig) error { if len(ts.Samples) == 0 { return nil } @@ -942,7 +942,7 @@ func (d *Distributor) validateSamples(now model.Time, ts *mimirpb.PreallocTimese if len(ts.Samples) == 1 { delta := now - model.Time(ts.Samples[0].TimestampMs) d.sampleDelay.WithLabelValues(userID).Observe(float64(delta) / 1000) - return validateSample(d.sampleValidationMetrics, now, d.limits, userID, group, ts.Labels, ts.Samples[0], cat) + return validateSample(d.sampleValidationMetrics, now, cfg, userID, group, ts.Labels, ts.Samples[0], cat) } timestamps := make(map[int64]struct{}, min(len(ts.Samples), 100)) @@ -960,7 +960,7 @@ func (d *Distributor) validateSamples(now model.Time, ts *mimirpb.PreallocTimese delta := now - model.Time(s.TimestampMs) d.sampleDelay.WithLabelValues(userID).Observe(float64(delta) / 1000) - if err := validateSample(d.sampleValidationMetrics, now, d.limits, userID, group, ts.Labels, s, cat); err != nil { + if err := validateSample(d.sampleValidationMetrics, now, cfg, userID, group, ts.Labels, s, cat); err != nil { return err } @@ -980,7 +980,7 @@ func (d *Distributor) validateSamples(now model.Time, ts *mimirpb.PreallocTimese // Returns an error explaining the first validation finding. // May alter timeseries data in-place. // The returned error MUST NOT retain label strings - they point into a gRPC buffer which is re-used. -func (d *Distributor) validateHistograms(now model.Time, ts *mimirpb.PreallocTimeseries, userID, group string) error { +func (d *Distributor) validateHistograms(now model.Time, ts *mimirpb.PreallocTimeseries, userID, group string, cfg sampleValidationConfig) error { if len(ts.Histograms) == 0 { return nil } @@ -990,7 +990,7 @@ func (d *Distributor) validateHistograms(now model.Time, ts *mimirpb.PreallocTim delta := now - model.Time(ts.Histograms[0].Timestamp) d.sampleDelay.WithLabelValues(userID).Observe(float64(delta) / 1000) - updated, err := validateSampleHistogram(d.sampleValidationMetrics, now, d.limits, userID, group, ts.Labels, &ts.Histograms[0], cat) + updated, err := validateSampleHistogram(d.sampleValidationMetrics, now, cfg, userID, group, ts.Labels, &ts.Histograms[0], cat) if err != nil { return err } @@ -1015,7 +1015,7 @@ func (d *Distributor) validateHistograms(now model.Time, ts *mimirpb.PreallocTim delta := now - model.Time(ts.Histograms[idx].Timestamp) d.sampleDelay.WithLabelValues(userID).Observe(float64(delta) / 1000) - updated, err := validateSampleHistogram(d.sampleValidationMetrics, now, d.limits, userID, group, ts.Labels, &ts.Histograms[idx], cat) + updated, err := validateSampleHistogram(d.sampleValidationMetrics, now, cfg, userID, group, ts.Labels, &ts.Histograms[idx], cat) if err != nil { return err } @@ -1078,21 +1078,21 @@ func (d *Distributor) validateExemplars(ts *mimirpb.PreallocTimeseries, userID s // Returns an error explaining the first validation finding. Non-nil error means the timeseries should be removed from the request. // The returned error MUST NOT retain label strings - they point into a gRPC buffer which is re-used. // It uses the passed nowt time to observe the delay of sample timestamps. -func (d *Distributor) validateSeries(nowt time.Time, ts *mimirpb.PreallocTimeseries, userID, group string, skipLabelValidation, skipLabelCountValidation bool, minExemplarTS, maxExemplarTS int64, valueTooLongSummaries *labelValueTooLongSummaries) error { +func (d *Distributor) validateSeries(nowt time.Time, ts *mimirpb.PreallocTimeseries, userID, group string, cfg validationConfig, skipLabelValidation, skipLabelCountValidation bool, minExemplarTS, maxExemplarTS int64, valueTooLongSummaries *labelValueTooLongSummaries) error { cat := d.costAttributionMgr.SampleTracker(userID) - if err := validateLabels(d.sampleValidationMetrics, d.limits, userID, group, ts.Labels, skipLabelValidation, skipLabelCountValidation, cat, nowt, valueTooLongSummaries); err != nil { + if err := validateLabels(d.sampleValidationMetrics, cfg.labels, userID, group, ts.Labels, skipLabelValidation, skipLabelCountValidation, cat, nowt, valueTooLongSummaries); err != nil { return err } now := model.TimeFromUnixNano(nowt.UnixNano()) totalSamplesAndHistograms := len(ts.Samples) + len(ts.Histograms) - if err := d.validateSamples(now, ts, userID, group); err != nil { + if err := d.validateSamples(now, ts, userID, group, cfg.samples); err != nil { return err } - if err := d.validateHistograms(now, ts, userID, group); err != nil { + if err := d.validateHistograms(now, ts, userID, group, cfg.samples); err != nil { return err } @@ -1328,6 +1328,7 @@ func (d *Distributor) prePushValidationMiddleware(next PushFunc) PushFunc { d.activeUsers.UpdateUserTimestamp(userID, now) pushReq.group = d.activeGroups.UpdateActiveGroupTimestamp(userID, validation.GroupLabel(d.limits, userID, req.Timeseries), now) + cfg := newValidationConfig(userID, d.limits) // A WriteRequest can only contain series or metadata but not both. This might change in the future. validatedMetadata := 0 @@ -1393,7 +1394,7 @@ func (d *Distributor) prePushValidationMiddleware(next PushFunc) PushFunc { // Note that validateSeries may drop some data in ts. rawSamples := len(ts.Samples) rawHistograms := len(ts.Histograms) - validationErr := d.validateSeries(now, &req.Timeseries[tsIdx], userID, pushReq.group, skipLabelValidation, skipLabelCountValidation, minExemplarTS, maxExemplarTS, &valueTooLongSummaries) + validationErr := d.validateSeries(now, &req.Timeseries[tsIdx], userID, pushReq.group, cfg, skipLabelValidation, skipLabelCountValidation, minExemplarTS, maxExemplarTS, &valueTooLongSummaries) if countDroppedNativeHistograms { droppedNativeHistograms += len(ts.Histograms) @@ -1467,7 +1468,7 @@ func (d *Distributor) prePushValidationMiddleware(next PushFunc) PushFunc { } for mIdx, m := range req.Metadata { - if validationErr := cleanAndValidateMetadata(d.metadataValidationMetrics, d.limits, userID, m); validationErr != nil { + if validationErr := cleanAndValidateMetadata(d.metadataValidationMetrics, cfg.metadata, userID, m); validationErr != nil { if firstPartialErr == nil { // The series are never retained by validationErr. This is guaranteed by the way the latter is built. firstPartialErr = newValidationError(validationErr) diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index bcf4fc643e8..10fbbfff31e 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -1652,9 +1652,10 @@ func TestDistributor_ValidateSeries(t *testing.T) { require.Len(t, ds, 1) require.Len(t, regs, 1) + cfg := newValidationConfig("user", ds[0].limits) now := mtime.Now() for _, ts := range tc.req.Timeseries { - err := ds[0].validateSeries(now, &ts, "user", "test-group", true, true, 0, 0, nil) + err := ds[0].validateSeries(now, &ts, "user", "test-group", cfg, true, true, 0, 0, nil) require.NoError(t, err) } @@ -1826,11 +1827,12 @@ func BenchmarkDistributor_SampleDuplicateTimestamp(b *testing.B) { for name, tc := range testCases { b.Run(name, func(b *testing.B) { timeseries := tc.setup(b.N) + cfg := newValidationConfig("user", ds[0].limits) b.ReportAllocs() b.ResetTimer() for n := 0; n < b.N; n++ { for _, ts := range timeseries[n] { - err := ds[0].validateSeries(now, &ts, "user", "test-group", true, true, 0, 0, nil) + err := ds[0].validateSeries(now, &ts, "user", "test-group", cfg, true, true, 0, 0, nil) if err != nil { b.Fatal(err) } @@ -2072,8 +2074,9 @@ func TestDistributor_ExemplarValidation(t *testing.T) { require.Len(t, ds, 1) require.Len(t, regs, 1) + cfg := newValidationConfig("user", ds[0].limits) for _, ts := range tc.req.Timeseries { - err := ds[0].validateSeries(now, &ts, "user", "test-group", false, false, tc.minExemplarTS, tc.maxExemplarTS, nil) + err := ds[0].validateSeries(now, &ts, "user", "test-group", cfg, false, false, tc.minExemplarTS, tc.maxExemplarTS, nil) assert.NoError(t, err) } @@ -2179,8 +2182,9 @@ func TestDistributor_HistogramReduction(t *testing.T) { require.Len(t, ds, 1) require.Len(t, regs, 1) + cfg := newValidationConfig("user", ds[0].limits) for _, ts := range tc.req.Timeseries { - err := ds[0].validateSeries(now, &ts, "user", "test-group", false, false, 0, 0, nil) + err := ds[0].validateSeries(now, &ts, "user", "test-group", cfg, false, false, 0, 0, nil) if tc.expectedError != nil { require.ErrorAs(t, err, &tc.expectedError) } else { diff --git a/pkg/distributor/validate.go b/pkg/distributor/validate.go index a2236b3f4a0..584f26b8885 100644 --- a/pkg/distributor/validate.go +++ b/pkg/distributor/validate.go @@ -194,13 +194,62 @@ type labelValueTooLongSummary struct { sampleValue string } +type validationConfig struct { + samples sampleValidationConfig + labels labelValidationConfig + metadata metadataValidationConfig +} + +// newValidationConfig builds a validationConfig based on the passed overrides. +// TODO: This could still be more efficient, as each overrides call performs an atomic pointer retrieval and a lookup in a map, +// TODO: but it's already better than the previous implementation, which was doing this per-sample and per-series. +func newValidationConfig(userID string, overrides *validation.Overrides) validationConfig { + return validationConfig{ + samples: sampleValidationConfig{ + creationGracePeriod: overrides.CreationGracePeriod(userID), + pastGracePeriod: overrides.PastGracePeriod(userID), + maxNativeHistogramBuckets: overrides.MaxNativeHistogramBuckets(userID), + reduceNativeHistogramOverMaxBuckets: overrides.ReduceNativeHistogramOverMaxBuckets(userID), + outOfOrderTimeWindow: overrides.OutOfOrderTimeWindow(userID), + }, + labels: labelValidationConfig{ + maxLabelNamesPerSeries: overrides.MaxLabelNamesPerSeries(userID), + maxLabelNamesPerInfoSeries: overrides.MaxLabelNamesPerInfoSeries(userID), + maxLabelNameLength: overrides.MaxLabelNameLength(userID), + maxLabelValueLength: overrides.MaxLabelValueLength(userID), + labelValueLengthOverLimitStrategy: overrides.LabelValueLengthOverLimitStrategy(userID), + nameValidationScheme: overrides.NameValidationScheme(userID), + }, + metadata: metadataValidationConfig{ + enforceMetadataMetricName: overrides.EnforceMetadataMetricName(userID), + maxMetadataLength: overrides.MaxMetadataLength(userID), + }, + } +} + // sampleValidationConfig helps with getting required config to validate sample. -type sampleValidationConfig interface { - CreationGracePeriod(userID string) time.Duration - PastGracePeriod(userID string) time.Duration - MaxNativeHistogramBuckets(userID string) int - ReduceNativeHistogramOverMaxBuckets(userID string) bool - OutOfOrderTimeWindow(userID string) time.Duration +type sampleValidationConfig struct { + creationGracePeriod time.Duration + pastGracePeriod time.Duration + maxNativeHistogramBuckets int + reduceNativeHistogramOverMaxBuckets bool + outOfOrderTimeWindow time.Duration +} + +// labelValidationConfig helps with getting required config to validate labels. +type labelValidationConfig struct { + maxLabelNamesPerSeries int + maxLabelNamesPerInfoSeries int + maxLabelNameLength int + maxLabelValueLength int + labelValueLengthOverLimitStrategy validation.LabelValueLengthOverLimitStrategy + nameValidationScheme model.ValidationScheme +} + +// metadataValidationConfig helps with getting required config to validate metadata. +type metadataValidationConfig struct { + enforceMetadataMetricName bool + maxMetadataLength int } // sampleValidationMetrics is a collection of metrics used during sample validation. @@ -312,14 +361,14 @@ func newExemplarValidationMetrics(r prometheus.Registerer) *exemplarValidationMe // The returned error MUST NOT retain label strings - they point into a gRPC buffer which is re-used. // It uses the passed 'now' time to measure the relative time of the sample. func validateSample(m *sampleValidationMetrics, now model.Time, cfg sampleValidationConfig, userID, group string, ls []mimirpb.UnsafeMutableLabel, s mimirpb.Sample, cat *costattribution.SampleTracker) error { - if model.Time(s.TimestampMs) > now.Add(cfg.CreationGracePeriod(userID)) { + if model.Time(s.TimestampMs) > now.Add(cfg.creationGracePeriod) { m.tooFarInFuture.WithLabelValues(userID, group).Inc() cat.IncrementDiscardedSamples(ls, 1, reasonTooFarInFuture, now.Time()) unsafeMetricName, _ := extract.UnsafeMetricNameFromLabelAdapters(ls) return fmt.Errorf(sampleTimestampTooNewMsgFormat, s.TimestampMs, unsafeMetricName) } - if cfg.PastGracePeriod(userID) > 0 && model.Time(s.TimestampMs) < now.Add(-cfg.PastGracePeriod(userID)).Add(-cfg.OutOfOrderTimeWindow(userID)) { + if cfg.pastGracePeriod > 0 && model.Time(s.TimestampMs) < now.Add(-cfg.pastGracePeriod).Add(-cfg.outOfOrderTimeWindow) { m.tooFarInPast.WithLabelValues(userID, group).Inc() cat.IncrementDiscardedSamples(ls, 1, reasonTooFarInPast, now.Time()) unsafeMetricName, _ := extract.UnsafeMetricNameFromLabelAdapters(ls) @@ -333,14 +382,14 @@ func validateSample(m *sampleValidationMetrics, now model.Time, cfg sampleValida // The returned error MUST NOT retain label strings - they point into a gRPC buffer which is re-used. // It uses the passed 'now' time to measure the relative time of the sample. func validateSampleHistogram(m *sampleValidationMetrics, now model.Time, cfg sampleValidationConfig, userID, group string, ls []mimirpb.UnsafeMutableLabel, s *mimirpb.Histogram, cat *costattribution.SampleTracker) (bool, error) { - if model.Time(s.Timestamp) > now.Add(cfg.CreationGracePeriod(userID)) { + if model.Time(s.Timestamp) > now.Add(cfg.creationGracePeriod) { cat.IncrementDiscardedSamples(ls, 1, reasonTooFarInFuture, now.Time()) m.tooFarInFuture.WithLabelValues(userID, group).Inc() unsafeMetricName, _ := extract.UnsafeMetricNameFromLabelAdapters(ls) return false, fmt.Errorf(sampleTimestampTooNewMsgFormat, s.Timestamp, unsafeMetricName) } - if cfg.PastGracePeriod(userID) > 0 && model.Time(s.Timestamp) < now.Add(-cfg.PastGracePeriod(userID)).Add(-cfg.OutOfOrderTimeWindow(userID)) { + if cfg.pastGracePeriod > 0 && model.Time(s.Timestamp) < now.Add(-cfg.pastGracePeriod).Add(-cfg.outOfOrderTimeWindow) { cat.IncrementDiscardedSamples(ls, 1, reasonTooFarInPast, now.Time()) m.tooFarInPast.WithLabelValues(userID, group).Inc() unsafeMetricName, _ := extract.UnsafeMetricNameFromLabelAdapters(ls) @@ -354,7 +403,7 @@ func validateSampleHistogram(m *sampleValidationMetrics, now model.Time, cfg sam return false, fmt.Errorf(invalidSchemaNativeHistogramMsgFormat, s.Schema) } - if bucketLimit := cfg.MaxNativeHistogramBuckets(userID); bucketLimit > 0 { + if bucketLimit := cfg.maxNativeHistogramBuckets; bucketLimit > 0 { bucketCount := s.BucketCount() if bucketCount > bucketLimit { if s.Schema == mimirpb.NativeHistogramsWithCustomBucketsSchema { @@ -363,7 +412,7 @@ func validateSampleHistogram(m *sampleValidationMetrics, now model.Time, cfg sam m.maxNativeHistogramBuckets.WithLabelValues(userID, group).Inc() return false, fmt.Errorf(nativeHistogramCustomBucketsNotReducibleMsgFormat, s.Timestamp, mimirpb.FromLabelAdaptersToString(ls), bucketCount, bucketLimit) } - if !cfg.ReduceNativeHistogramOverMaxBuckets(userID) { + if !cfg.reduceNativeHistogramOverMaxBuckets { cat.IncrementDiscardedSamples(ls, 1, reasonMaxNativeHistogramBuckets, now.Time()) m.maxNativeHistogramBuckets.WithLabelValues(userID, group).Inc() return false, fmt.Errorf(maxNativeHistogramBucketsMsgFormat, s.Timestamp, mimirpb.FromLabelAdaptersToString(ls), bucketCount, bucketLimit) @@ -445,16 +494,6 @@ func validateExemplarTimestamp(m *exemplarValidationMetrics, userID string, minT return true } -// labelValidationConfig helps with getting required config to validate labels. -type labelValidationConfig interface { - MaxLabelNamesPerSeries(userID string) int - MaxLabelNamesPerInfoSeries(userID string) int - MaxLabelNameLength(userID string) int - MaxLabelValueLength(userID string) int - LabelValueLengthOverLimitStrategy(userID string) validation.LabelValueLengthOverLimitStrategy - NameValidationScheme(userID string) model.ValidationScheme -} - func removeNonASCIIChars(in string) (out string) { foundNonASCII := false @@ -485,7 +524,7 @@ func validateLabels(m *sampleValidationMetrics, cfg labelValidationConfig, userI return errors.New(noMetricNameMsgFormat) } - validationScheme := cfg.NameValidationScheme(userID) + validationScheme := cfg.nameValidationScheme if !validationScheme.IsValidMetricName(unsafeMetricName) { cat.IncrementDiscardedSamples(ls, 1, reasonInvalidMetricName, ts) @@ -493,25 +532,25 @@ func validateLabels(m *sampleValidationMetrics, cfg labelValidationConfig, userI return fmt.Errorf(invalidMetricNameMsgFormat, removeNonASCIIChars(unsafeMetricName)) } - if !skipLabelCountValidation && len(ls) > cfg.MaxLabelNamesPerSeries(userID) { + if !skipLabelCountValidation && len(ls) > cfg.maxLabelNamesPerSeries { if strings.HasSuffix(unsafeMetricName, "_info") { - if len(ls) > cfg.MaxLabelNamesPerInfoSeries(userID) { + if len(ls) > cfg.maxLabelNamesPerInfoSeries { m.maxLabelNamesPerInfoSeries.WithLabelValues(userID, group).Inc() cat.IncrementDiscardedSamples(ls, 1, reasonMaxLabelNamesPerInfoSeries, ts) metric, ellipsis := getMetricAndEllipsis(ls) - return fmt.Errorf(tooManyInfoLabelsMsgFormat, len(ls), cfg.MaxLabelNamesPerInfoSeries(userID), metric, ellipsis) + return fmt.Errorf(tooManyInfoLabelsMsgFormat, len(ls), cfg.maxLabelNamesPerInfoSeries, metric, ellipsis) } } else { m.maxLabelNamesPerSeries.WithLabelValues(userID, group).Inc() cat.IncrementDiscardedSamples(ls, 1, reasonMaxLabelNamesPerSeries, ts) metric, ellipsis := getMetricAndEllipsis(ls) - return fmt.Errorf(tooManyLabelsMsgFormat, len(ls), cfg.MaxLabelNamesPerSeries(userID), metric, ellipsis) + return fmt.Errorf(tooManyLabelsMsgFormat, len(ls), cfg.maxLabelNamesPerSeries, metric, ellipsis) } } - maxLabelNameLength := cfg.MaxLabelNameLength(userID) - maxLabelValueLength := cfg.MaxLabelValueLength(userID) - labelValueLengthOverLimitStrategy := cfg.LabelValueLengthOverLimitStrategy(userID) + maxLabelNameLength := cfg.maxLabelNameLength + maxLabelValueLength := cfg.maxLabelValueLength + labelValueLengthOverLimitStrategy := cfg.labelValueLengthOverLimitStrategy lastLabelName := "" for i, l := range ls { @@ -594,20 +633,14 @@ func newMetadataValidationMetrics(r prometheus.Registerer) *metadataValidationMe } } -// metadataValidationConfig helps with getting required config to validate metadata. -type metadataValidationConfig interface { - EnforceMetadataMetricName(userID string) bool - MaxMetadataLength(userID string) int -} - // cleanAndValidateMetadata returns an err if a metric metadata is invalid. func cleanAndValidateMetadata(m *metadataValidationMetrics, cfg metadataValidationConfig, userID string, metadata *mimirpb.MetricMetadata) error { - if cfg.EnforceMetadataMetricName(userID) && metadata.GetMetricFamilyName() == "" { + if cfg.enforceMetadataMetricName && metadata.GetMetricFamilyName() == "" { m.missingMetricName.WithLabelValues(userID).Inc() return errors.New(metadataMetricNameMissingMsgFormat) } - maxMetadataValueLength := cfg.MaxMetadataLength(userID) + maxMetadataValueLength := cfg.maxMetadataLength if len(metadata.Help) > maxMetadataValueLength { newlen := 0 diff --git a/pkg/distributor/validate_test.go b/pkg/distributor/validate_test.go index 17d508b448d..cf6fe41cf67 100644 --- a/pkg/distributor/validate_test.go +++ b/pkg/distributor/validate_test.go @@ -12,6 +12,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "reflect" "strings" "sync" "testing" @@ -44,52 +45,6 @@ import ( "github.com/grafana/mimir/pkg/util/validation" ) -type validateLabelsCfg struct { - maxLabelNamesPerSeries int - maxLabelNamesPerInfoSeries int - maxLabelNameLength int - maxLabelValueLength int - validationScheme model.ValidationScheme - labelValueLengthOverLimitStrategy validation.LabelValueLengthOverLimitStrategy -} - -func (v validateLabelsCfg) MaxLabelNamesPerSeries(_ string) int { - return v.maxLabelNamesPerSeries -} - -func (v validateLabelsCfg) MaxLabelNamesPerInfoSeries(_ string) int { - return v.maxLabelNamesPerInfoSeries -} - -func (v validateLabelsCfg) MaxLabelNameLength(_ string) int { - return v.maxLabelNameLength -} - -func (v validateLabelsCfg) MaxLabelValueLength(_ string) int { - return v.maxLabelValueLength -} - -func (v validateLabelsCfg) NameValidationScheme(_ string) model.ValidationScheme { - return v.validationScheme -} - -func (v validateLabelsCfg) LabelValueLengthOverLimitStrategy(_ string) validation.LabelValueLengthOverLimitStrategy { - return v.labelValueLengthOverLimitStrategy -} - -type validateMetadataCfg struct { - enforceMetadataMetricName bool - maxMetadataLength int -} - -func (vm validateMetadataCfg) EnforceMetadataMetricName(_ string) bool { - return vm.enforceMetadataMetricName -} - -func (vm validateMetadataCfg) MaxMetadataLength(_ string) int { - return vm.maxMetadataLength -} - func TestValidateLabels(t *testing.T) { t.Parallel() @@ -929,7 +884,7 @@ func TestValidateMetadata(t *testing.T) { m := newMetadataValidationMetrics(reg) userID := "testUser" - var cfg validateMetadataCfg + var cfg metadataValidationConfig cfg.enforceMetadataMetricName = true cfg.maxMetadataLength = 22 @@ -1014,11 +969,11 @@ func TestValidateMetadata(t *testing.T) { func TestValidateLabelDuplication(t *testing.T) { ts := time.Now() - var cfg validateLabelsCfg + var cfg labelValidationConfig cfg.maxLabelNameLength = 10 cfg.maxLabelNamesPerSeries = 10 cfg.maxLabelValueLength = 10 - cfg.validationScheme = model.LegacyValidation + cfg.nameValidationScheme = model.LegacyValidation userID := "testUser" actual := validateLabels(newSampleValidationMetrics(nil), cfg, userID, "", []mimirpb.LabelAdapter{ @@ -1072,10 +1027,10 @@ func TestValidateLabel_UseAfterRelease(t *testing.T) { require.NoError(t, err) // Call validateLabels to get a LabelValueTooLongError. - cfg := validateLabelsCfg{ - maxLabelNameLength: 25, - maxLabelValueLength: 5, - validationScheme: model.UTF8Validation, + cfg := labelValidationConfig{ + maxLabelNameLength: 25, + maxLabelValueLength: 5, + nameValidationScheme: model.UTF8Validation, } const userID = "testUser" limits := testutils.NewMockCostAttributionLimits(0, []string{userID, "team"}) @@ -1104,31 +1059,6 @@ func TestValidateLabel_UseAfterRelease(t *testing.T) { require.EqualError(t, lengthErr, "received a series whose label value length of 37 exceeds the limit of 5, label: '__name__', value: 'value_longer_than_maxLabelValueLength' (truncated) series: 'value_longer_than_maxLabelValueLength' (err-mimir-label-value-too-long). To adjust the related per-tenant limit, configure -validation.max-length-label-value, or contact your service administrator.") } -type sampleValidationCfg struct { - maxNativeHistogramBuckets int - reduceNativeHistogramOverMaxBuckets bool -} - -func (c sampleValidationCfg) CreationGracePeriod(_ string) time.Duration { - return 0 -} - -func (c sampleValidationCfg) PastGracePeriod(_ string) time.Duration { - return 0 -} - -func (c sampleValidationCfg) OutOfOrderTimeWindow(_ string) time.Duration { - return 0 -} - -func (c sampleValidationCfg) MaxNativeHistogramBuckets(_ string) int { - return c.maxNativeHistogramBuckets -} - -func (c sampleValidationCfg) ReduceNativeHistogramOverMaxBuckets(_ string) bool { - return c.reduceNativeHistogramOverMaxBuckets -} - func TestMaxNativeHistorgramBuckets(t *testing.T) { // All will have 2 buckets, one negative and one positive testCases := map[string]mimirpb.Histogram{ @@ -1243,8 +1173,9 @@ func TestMaxNativeHistorgramBuckets(t *testing.T) { for _, limit := range []int{0, 1, 2} { for name, h := range testCases { t.Run(fmt.Sprintf("limit-%d-%s", limit, name), func(t *testing.T) { - var cfg sampleValidationCfg - cfg.maxNativeHistogramBuckets = limit + cfg := sampleValidationConfig{ + maxNativeHistogramBuckets: limit, + } ls := []mimirpb.LabelAdapter{{Name: model.MetricNameLabel, Value: "a"}, {Name: "a", Value: "a"}} _, err := validateSampleHistogram(metrics, model.Now(), cfg, "user-1", "group-1", ls, &h, nil) @@ -1292,7 +1223,7 @@ func TestInvalidNativeHistogramSchema(t *testing.T) { registry := prometheus.NewRegistry() metrics := newSampleValidationMetrics(registry) - cfg := sampleValidationCfg{} + cfg := sampleValidationConfig{} hist := &mimirpb.Histogram{} labels := []mimirpb.LabelAdapter{{Name: model.MetricNameLabel, Value: "a"}, {Name: "a", Value: "a"}} for testName, testCase := range testCases { @@ -1312,7 +1243,7 @@ func TestInvalidNativeHistogramSchema(t *testing.T) { func TestNativeHistogramDownScaling(t *testing.T) { testCases := map[string]struct { - cfg sampleValidationCfg + cfg sampleValidationConfig schema int32 offset int32 deltas []int64 // We're just using consecutive positive deltas. @@ -1327,13 +1258,13 @@ func TestNativeHistogramDownScaling(t *testing.T) { expectedDeltas: []int64{1, 2, 3}, }, "downscaling not allowed": { - cfg: sampleValidationCfg{maxNativeHistogramBuckets: 2}, + cfg: sampleValidationConfig{maxNativeHistogramBuckets: 2}, schema: 3, deltas: []int64{1, 2, 3}, expectedError: fmt.Errorf("received a native histogram sample with too many buckets"), }, "downscaling allowed": { - cfg: sampleValidationCfg{maxNativeHistogramBuckets: 2, reduceNativeHistogramOverMaxBuckets: true}, + cfg: sampleValidationConfig{maxNativeHistogramBuckets: 2, reduceNativeHistogramOverMaxBuckets: true}, schema: 3, offset: 1, deltas: []int64{1, 2, 10}, @@ -1342,7 +1273,7 @@ func TestNativeHistogramDownScaling(t *testing.T) { expectedUpdate: true, }, "downscaling allowed but impossible": { - cfg: sampleValidationCfg{maxNativeHistogramBuckets: 2, reduceNativeHistogramOverMaxBuckets: true}, + cfg: sampleValidationConfig{maxNativeHistogramBuckets: 2, reduceNativeHistogramOverMaxBuckets: true}, schema: 3, offset: 0, // This means we would have to join bucket around the boundary of 1.0, but that will never happen. deltas: []int64{1, 2, 10}, @@ -1355,14 +1286,14 @@ func TestNativeHistogramDownScaling(t *testing.T) { expectedDeltas: []int64{1, 2, 3}, }, "valid nhcb and bucket limit is set": { - cfg: sampleValidationCfg{maxNativeHistogramBuckets: 100, reduceNativeHistogramOverMaxBuckets: true}, + cfg: sampleValidationConfig{maxNativeHistogramBuckets: 100, reduceNativeHistogramOverMaxBuckets: true}, schema: -53, deltas: []int64{1, 2, 3}, expectedError: nil, expectedDeltas: []int64{1, 2, 3}, }, "downscaling not possible for nhcb": { - cfg: sampleValidationCfg{maxNativeHistogramBuckets: 2, reduceNativeHistogramOverMaxBuckets: true}, + cfg: sampleValidationConfig{maxNativeHistogramBuckets: 2, reduceNativeHistogramOverMaxBuckets: true}, schema: -53, offset: 1, deltas: []int64{1, 2, 3}, @@ -1504,3 +1435,42 @@ func stringContainsAll(s string, parts []string) bool { } return true } + +// TestNewValidationConfigFieldCompleteness ensures that we don't forget to populate a new field we may add to validationConfig. +func TestNewValidationConfigFieldCompleteness(t *testing.T) { + t.Parallel() + + // 1. Create limits with prepareDefaultLimits() + limits := prepareDefaultLimits() + + // 2. Set fields that default to zero to non-zero values + limits.PastGracePeriod = model.Duration(5 * time.Minute) + limits.MaxNativeHistogramBuckets = 100 + limits.OutOfOrderTimeWindow = model.Duration(30 * time.Minute) + require.NoError(t, limits.LabelValueLengthOverLimitStrategy.Set("truncate")) + + // 3. Create overrides + overrides := validation.NewOverrides(*limits, nil) + + // 4. Call newValidationConfig + cfg := newValidationConfig("test-user", overrides) + + // 5. Use reflection to verify all fields are non-zero + assertNoZeroFields(t, reflect.ValueOf(cfg), "validationConfig") +} + +func assertNoZeroFields(t *testing.T, v reflect.Value, path string) { + t.Helper() + + switch v.Kind() { + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + fieldName := v.Type().Field(i).Name + fieldPath := path + "." + fieldName + assertNoZeroFields(t, field, fieldPath) + } + default: + require.False(t, v.IsZero(), "field %s is zero", path) + } +}