Skip to content
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 sampler configuration to instrumentation kind #514

Merged
merged 2 commits into from
Nov 10, 2021

Conversation

pavolloffay
Copy link
Member

@pavolloffay pavolloffay commented Nov 8, 2021

@pavolloffay pavolloffay requested review from a team and codeboten November 8, 2021 10:36
@pavolloffay
Copy link
Member Author

@anuraaga would you like to review this one?

// configure sampler only if it is configured in the CR
if idx == -1 && otelinst.Spec.Sampler.Type != "" {
idxSamplerArg := getIndexOfEnv(container.Env, envOTELTracesSamplerArg)
if idxSamplerArg == -1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm - given we know that there is no arg for the default sampler, parent_based_always_on, I think we can be a bit more agressive here and ignore the user's SAMPLER_ARG even if it's set. Or either way, it's probably worth to log a message as it seems like a user error

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's worth handling this case. If a user accidentally adds an argument for the always_on it's gonna by ignored by the SDK anyways.

@jpkrohling
Copy link
Member

This needs a rebase.

@pavolloffay
Copy link
Member Author

rebased

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@jpkrohling jpkrohling merged commit f673f4e into open-telemetry:main Nov 10, 2021
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this pull request Dec 12, 2021
* Add sampler configuration to instrumentation kind

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>

* Fix

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* Add sampler configuration to instrumentation kind

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>

* Fix

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
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.

4 participants