Skip to content

fix: reject non-positive ingest sample rates#5253

Merged
simonswine merged 1 commit into
grafana:mainfrom
immanuwell:fix/ingest-samplerate-validation
Jun 19, 2026
Merged

fix: reject non-positive ingest sample rates#5253
simonswine merged 1 commit into
grafana:mainfrom
immanuwell:fix/ingest-samplerate-validation

Conversation

@immanuwell

Copy link
Copy Markdown
Contributor

tiny footgun, but real.

/ingest accepts sampleRate from the query string. Before this patch, sampleRate=-1 was parsed as an int and then cast to uint32, so it wrapped to 4294967295. For CPU and wall profiles that can drive the computed period down to 0, which is pretty cursed.

fix:

  • treat sampleRate <= 0 as invalid
  • fall back to the documented default 100
  • add tests for -1, 0, abc, and a valid positive value

repro:

  1. hit /ingest?...&sampleRate=-1
  2. before this patch, the handler stored 4294967295 as the sample rate
  3. after this patch, it falls back to 100

verified with go test ./pkg/ingester/pyroscope -count=1

@simonswine simonswine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This make perfect sense. Thanks for adding it

LGTM

@simonswine simonswine merged commit 3d08dbf into grafana:main Jun 19, 2026
33 checks passed
@korniltsev-grafanista

Copy link
Copy Markdown
Contributor

Were there any problems with sample rate 0? I noticed a ton of errors in my logs #5276

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants