Skip to content

Commit 7a83210

Browse files
PaurushGargPaurushGarg
authored
Add min/max schema Validation for NativeHistogram (#6766)
Signed-off-by: Paurush Garg <paurushg@amazon.com> Signed-off-by: Paurush Garg <62579325+PaurushGarg@users.noreply.github.com>
1 parent ff91465 commit 7a83210

File tree

5 files changed

+114
-56
lines changed

5 files changed

+114
-56
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Changelog
22

33
## master / unreleased
4+
* [ENHANCEMENT] Distributor: Add min/max schema validation for NativeHistograms. #6766
45
* [ENHANCEMENT] Ingester: Handle runtime errors in query path #6769
56
* [CHANGE] Ingester: Remove EnableNativeHistograms config flag and instead gate keep through new per-tenant limit at ingestion. #6718
67
* [CHANGE] StoreGateway/Alertmanager: Add default 5s connection timeout on client. #6603

‎pkg/distributor/distributor.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,6 @@ func (d *Distributor) validateSeries(ts cortexpb.PreallocTimeseries, userID stri
655655
if err := validation.ValidateSampleTimestamp(d.validateMetrics, limits, userID, ts.Labels, h.TimestampMs); err != nil {
656656
return emptyPreallocSeries, err
657657
}
658-
// TODO(yeya24): add max schema validation for native histogram if needed.
659658
convertedHistogram, err := validation.ValidateNativeHistogram(d.validateMetrics, limits, userID, ts.Labels, h)
660659
if err != nil {
661660
return emptyPreallocSeries, err

‎pkg/util/validation/errors.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77

88
"github.com/prometheus/common/model"
9+
"github.com/prometheus/prometheus/model/histogram"
910

1011
"github.com/cortexproject/cortex/pkg/cortexpb"
1112
)
@@ -243,6 +244,24 @@ func (e *histogramBucketLimitExceededError) Error() string {
243244
return fmt.Sprintf("native histogram bucket count exceeded for metric (limit: %d) metric: %.200q", e.limit, formatLabelSet(e.series))
244245
}
245246

247+
// nativeHistogramSchemaInvalidError is a ValidationError implementation for samples with native histogram
248+
// exceeding the valid schema range.
249+
type nativeHistogramSchemaInvalidError struct {
250+
series []cortexpb.LabelAdapter
251+
receivedSchema int
252+
}
253+
254+
func newNativeHistogramSchemaInvalidError(series []cortexpb.LabelAdapter, receivedSchema int) ValidationError {
255+
return &nativeHistogramSchemaInvalidError{
256+
series: series,
257+
receivedSchema: receivedSchema,
258+
}
259+
}
260+
261+
func (e *nativeHistogramSchemaInvalidError) Error() string {
262+
return fmt.Sprintf("invalid native histogram schema %d for metric: %.200q. supported schema from %d to %d", e.receivedSchema, formatLabelSet(e.series), histogram.ExponentialSchemaMin, histogram.ExponentialSchemaMax)
263+
}
264+
246265
// formatLabelSet formats label adapters as a metric name with labels, while preserving
247266
// label order, and keeping duplicates. If there are multiple "__name__" labels, only
248267
// first one is used as metric name, other ones will be included as regular labels.

‎pkg/util/validation/validate.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const (
5757

5858
// Native Histogram specific validation reasons
5959
nativeHistogramBucketCountLimitExceeded = "native_histogram_buckets_exceeded"
60+
nativeHistogramInvalidSchema = "native_histogram_invalid_schema"
6061

6162
// RateLimited is one of the values for the reason to discard samples.
6263
// Declared here to avoid duplication in ingester and distributor.
@@ -337,6 +338,13 @@ func ValidateMetadata(validateMetrics *ValidateMetrics, cfg *Limits, userID stri
337338
}
338339

339340
func ValidateNativeHistogram(validateMetrics *ValidateMetrics, limits *Limits, userID string, ls []cortexpb.LabelAdapter, histogramSample cortexpb.Histogram) (cortexpb.Histogram, error) {
341+
342+
// schema validation for native histogram
343+
if histogramSample.Schema < histogram.ExponentialSchemaMin || histogramSample.Schema > histogram.ExponentialSchemaMax {
344+
validateMetrics.DiscardedSamples.WithLabelValues(nativeHistogramInvalidSchema, userID).Inc()
345+
return cortexpb.Histogram{}, newNativeHistogramSchemaInvalidError(ls, int(histogramSample.Schema))
346+
}
347+
340348
if limits.MaxNativeHistogramBuckets == 0 {
341349
return histogramSample, nil
342350
}

‎pkg/util/validation/validate_test.go‎

Lines changed: 86 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -344,85 +344,116 @@ func TestValidateNativeHistogram(t *testing.T) {
344344
histogramWithSchemaMin.Schema = histogram.ExponentialSchemaMin
345345
floatHistogramWithSchemaMin := tsdbutil.GenerateTestFloatHistogram(0)
346346
floatHistogramWithSchemaMin.Schema = histogram.ExponentialSchemaMin
347+
348+
belowMinRangeSchemaHistogram := tsdbutil.GenerateTestFloatHistogram(0)
349+
belowMinRangeSchemaHistogram.Schema = -5
350+
exceedMaxRangeSchemaFloatHistogram := tsdbutil.GenerateTestFloatHistogram(0)
351+
exceedMaxRangeSchemaFloatHistogram.Schema = 20
352+
347353
for _, tc := range []struct {
348-
name string
349-
bucketLimit int
350-
resolutionReduced bool
351-
histogram cortexpb.Histogram
352-
expectedHistogram cortexpb.Histogram
353-
expectedErr error
354+
name string
355+
bucketLimit int
356+
resolutionReduced bool
357+
histogram cortexpb.Histogram
358+
expectedHistogram cortexpb.Histogram
359+
expectedErr error
360+
discardedSampleLabelValue string
354361
}{
355362
{
356-
name: "no limit, histogram",
357-
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
358-
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
363+
name: "no limit, histogram",
364+
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
365+
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
366+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
367+
},
368+
{
369+
name: "no limit, float histogram",
370+
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
371+
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
372+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
373+
},
374+
{
375+
name: "within limit, histogram",
376+
bucketLimit: 8,
377+
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
378+
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
379+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
359380
},
360381
{
361-
name: "no limit, float histogram",
362-
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
363-
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
382+
name: "within limit, float histogram",
383+
bucketLimit: 8,
384+
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
385+
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
386+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
364387
},
365388
{
366-
name: "within limit, histogram",
367-
bucketLimit: 8,
368-
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
369-
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
389+
name: "exceed limit and reduce resolution for 1 level, histogram",
390+
bucketLimit: 6,
391+
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
392+
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy().ReduceResolution(0)),
393+
resolutionReduced: true,
394+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
370395
},
371396
{
372-
name: "within limit, float histogram",
373-
bucketLimit: 8,
374-
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
375-
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
397+
name: "exceed limit and reduce resolution for 1 level, float histogram",
398+
bucketLimit: 6,
399+
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
400+
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy().ReduceResolution(0)),
401+
resolutionReduced: true,
402+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
376403
},
377404
{
378-
name: "exceed limit and reduce resolution for 1 level, histogram",
379-
bucketLimit: 6,
380-
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
381-
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy().ReduceResolution(0)),
382-
resolutionReduced: true,
405+
name: "exceed limit and reduce resolution for 2 levels, histogram",
406+
bucketLimit: 4,
407+
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
408+
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy().ReduceResolution(-1)),
409+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
383410
},
384411
{
385-
name: "exceed limit and reduce resolution for 1 level, float histogram",
386-
bucketLimit: 6,
387-
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
388-
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy().ReduceResolution(0)),
389-
resolutionReduced: true,
412+
name: "exceed limit and reduce resolution for 2 levels, float histogram",
413+
bucketLimit: 4,
414+
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
415+
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy().ReduceResolution(-1)),
416+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
390417
},
391418
{
392-
name: "exceed limit and reduce resolution for 2 levels, histogram",
393-
bucketLimit: 4,
394-
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
395-
expectedHistogram: cortexpb.HistogramToHistogramProto(0, h.Copy().ReduceResolution(-1)),
419+
name: "exceed limit but cannot reduce resolution further, histogram",
420+
bucketLimit: 1,
421+
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
422+
expectedErr: newHistogramBucketLimitExceededError(lbls, 1),
423+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
396424
},
397425
{
398-
name: "exceed limit and reduce resolution for 2 levels, float histogram",
399-
bucketLimit: 4,
400-
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
401-
expectedHistogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy().ReduceResolution(-1)),
426+
name: "exceed limit but cannot reduce resolution further, float histogram",
427+
bucketLimit: 1,
428+
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
429+
expectedErr: newHistogramBucketLimitExceededError(lbls, 1),
430+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
402431
},
403432
{
404-
name: "exceed limit but cannot reduce resolution further, histogram",
405-
bucketLimit: 1,
406-
histogram: cortexpb.HistogramToHistogramProto(0, h.Copy()),
407-
expectedErr: newHistogramBucketLimitExceededError(lbls, 1),
433+
name: "exceed limit but cannot reduce resolution further with min schema, histogram",
434+
bucketLimit: 4,
435+
histogram: cortexpb.HistogramToHistogramProto(0, histogramWithSchemaMin.Copy()),
436+
expectedErr: newHistogramBucketLimitExceededError(lbls, 4),
437+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
408438
},
409439
{
410-
name: "exceed limit but cannot reduce resolution further, float histogram",
411-
bucketLimit: 1,
412-
histogram: cortexpb.FloatHistogramToHistogramProto(0, fh.Copy()),
413-
expectedErr: newHistogramBucketLimitExceededError(lbls, 1),
440+
name: "exceed limit but cannot reduce resolution further with min schema, float histogram",
441+
bucketLimit: 4,
442+
histogram: cortexpb.FloatHistogramToHistogramProto(0, floatHistogramWithSchemaMin.Copy()),
443+
expectedErr: newHistogramBucketLimitExceededError(lbls, 4),
444+
discardedSampleLabelValue: nativeHistogramBucketCountLimitExceeded,
414445
},
415446
{
416-
name: "exceed limit but cannot reduce resolution further with min schema, histogram",
417-
bucketLimit: 4,
418-
histogram: cortexpb.HistogramToHistogramProto(0, histogramWithSchemaMin.Copy()),
419-
expectedErr: newHistogramBucketLimitExceededError(lbls, 4),
447+
name: "exceed min schema limit",
448+
histogram: cortexpb.FloatHistogramToHistogramProto(0, belowMinRangeSchemaHistogram.Copy()),
449+
expectedErr: newNativeHistogramSchemaInvalidError(lbls, int(belowMinRangeSchemaHistogram.Schema)),
450+
discardedSampleLabelValue: nativeHistogramInvalidSchema,
420451
},
421452
{
422-
name: "exceed limit but cannot reduce resolution further with min schema, float histogram",
423-
bucketLimit: 4,
424-
histogram: cortexpb.FloatHistogramToHistogramProto(0, floatHistogramWithSchemaMin.Copy()),
425-
expectedErr: newHistogramBucketLimitExceededError(lbls, 4),
453+
name: "exceed max schema limit",
454+
histogram: cortexpb.FloatHistogramToHistogramProto(0, exceedMaxRangeSchemaFloatHistogram.Copy()),
455+
expectedErr: newNativeHistogramSchemaInvalidError(lbls, int(exceedMaxRangeSchemaFloatHistogram.Schema)),
456+
discardedSampleLabelValue: nativeHistogramInvalidSchema,
426457
},
427458
} {
428459
t.Run(tc.name, func(t *testing.T) {
@@ -433,7 +464,7 @@ func TestValidateNativeHistogram(t *testing.T) {
433464
actualHistogram, actualErr := ValidateNativeHistogram(validateMetrics, limits, userID, lbls, tc.histogram)
434465
if tc.expectedErr != nil {
435466
require.Equal(t, tc.expectedErr, actualErr)
436-
require.Equal(t, float64(1), testutil.ToFloat64(validateMetrics.DiscardedSamples.WithLabelValues(nativeHistogramBucketCountLimitExceeded, userID)))
467+
require.Equal(t, float64(1), testutil.ToFloat64(validateMetrics.DiscardedSamples.WithLabelValues(tc.discardedSampleLabelValue, userID)))
437468
// Should never increment if error was returned
438469
require.Equal(t, float64(0), testutil.ToFloat64(validateMetrics.HistogramSamplesReducedResolution.WithLabelValues(userID)))
439470

0 commit comments

Comments
 (0)