Add WithSampler option
#468
Conversation
thomasgouveia
force-pushed
the
f-454-build-in-sampler-to-filter-traces-with-request-path
branch
3 times, most recently
from
b88895c to
bf6e610
Compare
There was a problem hiding this comment.
I don't see this as a scalable way to support custom sampling. It adds a one-off solution for a single sampler.
I would rather be in favor of supporting samplers like the exporter is supported. E.g. an "auto" package in contrib and a generic option here that accepts a sampler interface.
|
Hi @MrAlias, If I understand correctly, you want to :
LGTM in that case because it allows flexibility and customization for anyone who wants to build a custom auto instrumentation binary. |
|
+1 to I think the contrib addition is up for discussion. I think there are a few models for that addition. We would likely want to support environment variables there. For this though, I think adding a sampler option is a good start. 👍 |
thomasgouveia
force-pushed
the
f-454-build-in-sampler-to-filter-traces-with-request-path
branch
2 times, most recently
from
2a9e24d to
9b82733
Compare
|
@MrAlias I've just updated the PR with what we discussed. I've also updated the related issue. Let me know if something is missing! |
thomasgouveia
changed the title
Add a new `WithSampler` method allowing end-users to provide their own implementation of OpenTelemetry sampler directly through the package API. Signed-off-by: thomasgouveia <gouveia.thomas@outlook.fr>
thomasgouveia
force-pushed
the
f-454-build-in-sampler-to-filter-traces-with-request-path
branch
from
9b82733 to
803f528
Compare
This PR adds a new built-in sampler to filter traces based on the HTTP request path.
This feature could be handy when deploying into orchestration systems like Kubernetes that produce a trace for each liveness/readiness probe and could lead to higher storage costs.
Fixes #454