-
Notifications
You must be signed in to change notification settings - Fork 488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add probabilistic sampler processor #5151
Conversation
d2afa4a
to
00f0447
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this! I added a few notes regarding docs and testing.
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
component/otelcol/processor/probabilistic_sampler/probabilistic_sampler_test.go
Outdated
Show resolved
Hide resolved
component/otelcol/processor/probabilistic_sampler/probabilistic_sampler_test.go
Outdated
Show resolved
Hide resolved
00f0447
to
5fc1005
Compare
Thank you @ptodev for review! I fixed all of you mentioned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some doc suggestions
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
1bfa374
to
1f2f274
Compare
Thank you @clayton-cornell! I made changes to address all your suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, it looks great! I added a few minor suggestions. After @clayton-cornell approves, we could merge it.
component/otelcol/processor/probabilistic_sampler/probabilistic_sampler.go
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.processor.probabilistic_sampler.md
Outdated
Show resolved
Hide resolved
…robabilistic_sampler
…bilistic_sampler.md Co-authored-by: Clayton Cornell <[email protected]>
5142f4c
to
81d7f8e
Compare
Seems there are a few linter errors in the CI |
…c_sampler.go Co-authored-by: Paulin Todev <[email protected]>
81d7f8e
to
f7e42cf
Compare
forgot to import river, now linter passes |
PR Description
implements a flow component otelcol.processor.probabilistic_sampler that wraps over the upstream OpenTelemetry Collector Contrib probabilistic_sampler processor.
Which issue(s) this PR fixes
#3596
Notes to the Reviewer
PR Checklist