Skip to content

Verify native histograms with rate.Inf and zero burst#7662

Open
SungJin1212 wants to merge 1 commit into
cortexproject:masterfrom
SungJin1212:test/native-histogram-unlimited-rate-burst
Open

Verify native histograms with rate.Inf and zero burst#7662
SungJin1212 wants to merge 1 commit into
cortexproject:masterfrom
SungJin1212:test/native-histogram-unlimited-rate-burst

Conversation

@SungJin1212

Copy link
Copy Markdown
Member

What this PR does:
This PR verifies that native histogram ingestion works correctly when using rate.Inf and a zero burst rate.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@dosubot dosubot Bot added component/distributor go Pull requests that update Go code type/tests labels Jul 1, 2026
@SungJin1212 SungJin1212 mentioned this pull request Jul 1, 2026
2 tasks
{nhSamples: 1000, expectedError: nil, expectedNHDiscardedSampleMetricValue: 0},
{nhSamples: 5000, expectedError: nil, expectedNHDiscardedSampleMetricValue: 0},
},
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test for the globalStrategyNativeHistogram.Limit() case as well?

"global strategy: unlimited native histogram rate (Inf) with zero burst should always accept NH samples": {
distributors: 2,
ingestionRateStrategy: validation.GlobalIngestionRateStrategy,
ingestionRate: 100000,
ingestionBurstSize: 100000,
nativeHistogramIngestionRate: float64(rate.Inf),
nativeHistogramIngestionBurstSize: 0,
pushes: []testPush{
{nhSamples: 1000, expectedError: nil, expectedNHDiscardedSampleMetricValue: 0},
{nhSamples: 5000, expectedError: nil, expectedNHDiscardedSampleMetricValue: 0},
},
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment :D
I don't think it's necessary to add it here because it returns rate.Inf directly and never divides by the number of distributors when the nativeHistogramIngestionRateis rate.Inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants