Skip to content

refactor(loki): Add internal sampling library for secretfilter and process stage#5778

Merged
mikefat merged 2 commits into
mainfrom
mgordo/276
Mar 13, 2026
Merged

refactor(loki): Add internal sampling library for secretfilter and process stage#5778
mikefat merged 2 commits into
mainfrom
mgordo/276

Conversation

@mikefat
Copy link
Copy Markdown
Contributor

@mikefat mikefat commented Mar 12, 2026

Brief description of Pull Request

Based on a comment here, this PR refactors the code used for rate-based sampling and organizes it into an internal shared library. This cuts on code duplication and creates one source of truth for logic. This is currently used by loki.secretfilter and loki.process stage.sampling.

Pull Request Details

Changes:

  • internal/sampling: New package with Sampler struct. Implements the same probabilistic sampling as before (Jaeger-style algorithm) using stdlib only.
  • loki.secretfilter: Replaces local sampling (boundary/source, shouldProcessEntry) with *sampling.Sampler; Validate uses sampling.ValidateRate.
  • loki.process stage.sampling: Replaces local sampling and the jaeger-client-go dependency with *sampling.Sampler; config validation uses sampling.ValidateRate. Rate remains a required attribute (no default).
  • Tests: internal/sampling has new unit tests

Notes to the Reviewer

PR Checklist

  • Tests updated

@mikefat mikefat marked this pull request as ready for review March 12, 2026 23:10
@mikefat mikefat requested a review from a team as a code owner March 12, 2026 23:10
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Dependency Review

github.com/uber/jaeger-client-go v2.30.0+incompatible (direct) → v2.30.0+incompatible (indirect) — ✅ Safe
  • Summary

    • Version is unchanged; only the requirement type changed from direct to indirect.
    • No API or behavior change to review across versions; no code changes are required to adopt this state.
  • Changelog / Release Notes

  • Suggested Code Changes

    • None required. Because the version did not change and it’s now an indirect dependency, there are no code updates needed.

Notes

  • No net-new dependencies were introduced in go.mod in this diff.

Copy link
Copy Markdown
Contributor

@kalleep kalleep left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mikefat mikefat merged commit df7e9fb into main Mar 13, 2026
49 checks passed
@mikefat mikefat deleted the mgordo/276 branch March 13, 2026 15:04
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants