Skip to content

Commit ec63c2b

Browse files
committed
Resolving comments
Signed-off-by: Paurush Garg <[email protected]>
1 parent 2585034 commit ec63c2b

File tree

5 files changed

+46
-47
lines changed

5 files changed

+46
-47
lines changed

docs/configuration/config-file-reference.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3489,9 +3489,9 @@ The `limits_config` configures default and per-tenant limits imposed by Cortex s
34893489
# CLI flag: -validation.max-labels-size-bytes
34903490
[max_labels_size_bytes: <int> | default = 0]
34913491
3492-
# Maximum size in bytes of a native histograms sample. 0 to disable the limit.
3493-
# CLI flag: -validation.max-native-histograms-sample-size-bytes
3494-
[max_native_histograms_sample_size_bytes: <int> | default = 0]
3492+
# Maximum size in bytes of a native histogram sample. 0 to disable the limit.
3493+
# CLI flag: -validation.max-native-histogram-sample-size-bytes
3494+
[max_native_histogram_sample_size_bytes: <int> | default = 0]
34953495
34963496
# Maximum length accepted for metric metadata. Metadata refers to Metric Name,
34973497
# HELP and UNIT.

pkg/util/validation/errors.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ func (e *nativeHistogramSchemaInvalidError) Error() string {
262262
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)
263263
}
264264

265-
// nativeHistogramSchemaInvalidError is a ValidationError implementation for samples with native histogram
266-
// exceeding the valid schema range.
265+
// nativeHistogramSampleSizeBytesExceededError is a ValidationError implementation for samples with native histogram
266+
// exceeding the sample size bytes limit
267267
type nativeHistogramSampleSizeBytesExceededError struct {
268268
nhSampleSizeBytes int
269269
series []cortexpb.LabelAdapter

pkg/util/validation/limits.go

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -123,30 +123,30 @@ type LimitsPerLabelSet struct {
123123
// limits via flags, or per-user limits via yaml config.
124124
type Limits struct {
125125
// Distributor enforced limits.
126-
IngestionRate float64 `yaml:"ingestion_rate" json:"ingestion_rate"`
127-
IngestionRateStrategy string `yaml:"ingestion_rate_strategy" json:"ingestion_rate_strategy"`
128-
IngestionBurstSize int `yaml:"ingestion_burst_size" json:"ingestion_burst_size"`
129-
AcceptHASamples bool `yaml:"accept_ha_samples" json:"accept_ha_samples"`
130-
AcceptMixedHASamples bool `yaml:"accept_mixed_ha_samples" json:"accept_mixed_ha_samples"`
131-
HAClusterLabel string `yaml:"ha_cluster_label" json:"ha_cluster_label"`
132-
HAReplicaLabel string `yaml:"ha_replica_label" json:"ha_replica_label"`
133-
HAMaxClusters int `yaml:"ha_max_clusters" json:"ha_max_clusters"`
134-
DropLabels flagext.StringSlice `yaml:"drop_labels" json:"drop_labels"`
135-
MaxLabelNameLength int `yaml:"max_label_name_length" json:"max_label_name_length"`
136-
MaxLabelValueLength int `yaml:"max_label_value_length" json:"max_label_value_length"`
137-
MaxLabelNamesPerSeries int `yaml:"max_label_names_per_series" json:"max_label_names_per_series"`
138-
MaxLabelsSizeBytes int `yaml:"max_labels_size_bytes" json:"max_labels_size_bytes"`
139-
MaxNativeHistogramsSampleSizeBytes int `yaml:"max_native_histograms_sample_size_bytes" json:"max_native_histograms_sample_size_bytes"`
140-
MaxMetadataLength int `yaml:"max_metadata_length" json:"max_metadata_length"`
141-
RejectOldSamples bool `yaml:"reject_old_samples" json:"reject_old_samples"`
142-
RejectOldSamplesMaxAge model.Duration `yaml:"reject_old_samples_max_age" json:"reject_old_samples_max_age"`
143-
CreationGracePeriod model.Duration `yaml:"creation_grace_period" json:"creation_grace_period"`
144-
EnforceMetadataMetricName bool `yaml:"enforce_metadata_metric_name" json:"enforce_metadata_metric_name"`
145-
EnforceMetricName bool `yaml:"enforce_metric_name" json:"enforce_metric_name"`
146-
IngestionTenantShardSize int `yaml:"ingestion_tenant_shard_size" json:"ingestion_tenant_shard_size"`
147-
MetricRelabelConfigs []*relabel.Config `yaml:"metric_relabel_configs,omitempty" json:"metric_relabel_configs,omitempty" doc:"nocli|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."`
148-
MaxNativeHistogramBuckets int `yaml:"max_native_histogram_buckets" json:"max_native_histogram_buckets"`
149-
PromoteResourceAttributes []string `yaml:"promote_resource_attributes" json:"promote_resource_attributes"`
126+
IngestionRate float64 `yaml:"ingestion_rate" json:"ingestion_rate"`
127+
IngestionRateStrategy string `yaml:"ingestion_rate_strategy" json:"ingestion_rate_strategy"`
128+
IngestionBurstSize int `yaml:"ingestion_burst_size" json:"ingestion_burst_size"`
129+
AcceptHASamples bool `yaml:"accept_ha_samples" json:"accept_ha_samples"`
130+
AcceptMixedHASamples bool `yaml:"accept_mixed_ha_samples" json:"accept_mixed_ha_samples"`
131+
HAClusterLabel string `yaml:"ha_cluster_label" json:"ha_cluster_label"`
132+
HAReplicaLabel string `yaml:"ha_replica_label" json:"ha_replica_label"`
133+
HAMaxClusters int `yaml:"ha_max_clusters" json:"ha_max_clusters"`
134+
DropLabels flagext.StringSlice `yaml:"drop_labels" json:"drop_labels"`
135+
MaxLabelNameLength int `yaml:"max_label_name_length" json:"max_label_name_length"`
136+
MaxLabelValueLength int `yaml:"max_label_value_length" json:"max_label_value_length"`
137+
MaxLabelNamesPerSeries int `yaml:"max_label_names_per_series" json:"max_label_names_per_series"`
138+
MaxLabelsSizeBytes int `yaml:"max_labels_size_bytes" json:"max_labels_size_bytes"`
139+
MaxNativeHistogramSampleSizeBytes int `yaml:"max_native_histogram_sample_size_bytes" json:"max_native_histogram_sample_size_bytes"`
140+
MaxMetadataLength int `yaml:"max_metadata_length" json:"max_metadata_length"`
141+
RejectOldSamples bool `yaml:"reject_old_samples" json:"reject_old_samples"`
142+
RejectOldSamplesMaxAge model.Duration `yaml:"reject_old_samples_max_age" json:"reject_old_samples_max_age"`
143+
CreationGracePeriod model.Duration `yaml:"creation_grace_period" json:"creation_grace_period"`
144+
EnforceMetadataMetricName bool `yaml:"enforce_metadata_metric_name" json:"enforce_metadata_metric_name"`
145+
EnforceMetricName bool `yaml:"enforce_metric_name" json:"enforce_metric_name"`
146+
IngestionTenantShardSize int `yaml:"ingestion_tenant_shard_size" json:"ingestion_tenant_shard_size"`
147+
MetricRelabelConfigs []*relabel.Config `yaml:"metric_relabel_configs,omitempty" json:"metric_relabel_configs,omitempty" doc:"nocli|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."`
148+
MaxNativeHistogramBuckets int `yaml:"max_native_histogram_buckets" json:"max_native_histogram_buckets"`
149+
PromoteResourceAttributes []string `yaml:"promote_resource_attributes" json:"promote_resource_attributes"`
150150

151151
// Ingester enforced limits.
152152
// Series
@@ -254,7 +254,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
254254
f.IntVar(&l.MaxLabelValueLength, "validation.max-length-label-value", 2048, "Maximum length accepted for label value. This setting also applies to the metric name")
255255
f.IntVar(&l.MaxLabelNamesPerSeries, "validation.max-label-names-per-series", 30, "Maximum number of label names per series.")
256256
f.IntVar(&l.MaxLabelsSizeBytes, "validation.max-labels-size-bytes", 0, "Maximum combined size in bytes of all labels and label values accepted for a series. 0 to disable the limit.")
257-
f.IntVar(&l.MaxNativeHistogramsSampleSizeBytes, "validation.max-native-histograms-sample-size-bytes", 0, "Maximum size in bytes of a native histograms sample. 0 to disable the limit.")
257+
f.IntVar(&l.MaxNativeHistogramSampleSizeBytes, "validation.max-native-histogram-sample-size-bytes", 0, "Maximum size in bytes of a native histogram sample. 0 to disable the limit.")
258258
f.IntVar(&l.MaxMetadataLength, "validation.max-metadata-length", 1024, "Maximum length accepted for metric metadata. Metadata refers to Metric Name, HELP and UNIT.")
259259
f.BoolVar(&l.RejectOldSamples, "validation.reject-old-samples", false, "Reject old samples.")
260260
_ = l.RejectOldSamplesMaxAge.Set("14d")

pkg/util/validation/validate.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,10 +341,9 @@ func ValidateMetadata(validateMetrics *ValidateMetrics, cfg *Limits, userID stri
341341
func ValidateNativeHistogram(validateMetrics *ValidateMetrics, limits *Limits, userID string, ls []cortexpb.LabelAdapter, histogramSample cortexpb.Histogram) (cortexpb.Histogram, error) {
342342

343343
// sample size validation for native histogram
344-
nhSampleSize := histogramSample.Size()
345-
if limits.MaxNativeHistogramsSampleSizeBytes > 0 && nhSampleSize > limits.MaxNativeHistogramsSampleSizeBytes {
344+
if limits.MaxNativeHistogramSampleSizeBytes > 0 && histogramSample.Size() > limits.MaxNativeHistogramSampleSizeBytes {
346345
validateMetrics.DiscardedSamples.WithLabelValues(nativeHistogramSampleSizeBytesExceeded, userID).Inc()
347-
return cortexpb.Histogram{}, newNativeHistogramSampleSizeBytesExceededError(ls, histogramSample.Size(), limits.MaxNativeHistogramsSampleSizeBytes)
346+
return cortexpb.Histogram{}, newNativeHistogramSampleSizeBytesExceededError(ls, histogramSample.Size(), limits.MaxNativeHistogramSampleSizeBytes)
348347
}
349348

350349
// schema validation for native histogram

pkg/util/validation/validate_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,14 @@ func TestValidateNativeHistogram(t *testing.T) {
352352
exceedMaxSampleSizeBytesLimitFloatHistogram := tsdbutil.GenerateTestFloatHistogram(100)
353353

354354
for _, tc := range []struct {
355-
name string
356-
bucketLimit int
357-
resolutionReduced bool
358-
histogram cortexpb.Histogram
359-
expectedHistogram cortexpb.Histogram
360-
expectedErr error
361-
discardedSampleLabelValue string
362-
maxNativeHistogramsSampleSizeBytesLimit int
355+
name string
356+
bucketLimit int
357+
resolutionReduced bool
358+
histogram cortexpb.Histogram
359+
expectedHistogram cortexpb.Histogram
360+
expectedErr error
361+
discardedSampleLabelValue string
362+
maxNativeHistogramSampleSizeBytesLimit int
363363
}{
364364
{
365365
name: "no limit, histogram",
@@ -458,19 +458,19 @@ func TestValidateNativeHistogram(t *testing.T) {
458458
discardedSampleLabelValue: nativeHistogramInvalidSchema,
459459
},
460460
{
461-
name: "exceed max sample size bytes limit",
462-
histogram: cortexpb.FloatHistogramToHistogramProto(0, exceedMaxSampleSizeBytesLimitFloatHistogram.Copy()),
463-
expectedErr: newNativeHistogramSampleSizeBytesExceededError(lbls, 126, 100),
464-
discardedSampleLabelValue: nativeHistogramSampleSizeBytesExceeded,
465-
maxNativeHistogramsSampleSizeBytesLimit: 100,
461+
name: "exceed max sample size bytes limit",
462+
histogram: cortexpb.FloatHistogramToHistogramProto(0, exceedMaxSampleSizeBytesLimitFloatHistogram.Copy()),
463+
expectedErr: newNativeHistogramSampleSizeBytesExceededError(lbls, 126, 100),
464+
discardedSampleLabelValue: nativeHistogramSampleSizeBytesExceeded,
465+
maxNativeHistogramSampleSizeBytesLimit: 100,
466466
},
467467
} {
468468
t.Run(tc.name, func(t *testing.T) {
469469
reg := prometheus.NewRegistry()
470470
validateMetrics := NewValidateMetrics(reg)
471471
limits := new(Limits)
472472
limits.MaxNativeHistogramBuckets = tc.bucketLimit
473-
limits.MaxNativeHistogramsSampleSizeBytes = tc.maxNativeHistogramsSampleSizeBytesLimit
473+
limits.MaxNativeHistogramSampleSizeBytes = tc.maxNativeHistogramSampleSizeBytesLimit
474474
actualHistogram, actualErr := ValidateNativeHistogram(validateMetrics, limits, userID, lbls, tc.histogram)
475475
if tc.expectedErr != nil {
476476
require.Equal(t, tc.expectedErr, actualErr)

0 commit comments

Comments
 (0)