-
Notifications
You must be signed in to change notification settings - Fork 10
Wire up composite sampler #410
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
base: main
Are you sure you want to change the base?
Conversation
src/elasticotel/distro/config.py
Outdated
logger.debug("Cannot get sampler from tracer provider.") | ||
return ConfigUpdate() | ||
|
||
# FIXME: this needs to be updated for the consistent probability samplers |
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.
# FIXME: this needs to be updated for the consistent probability samplers |
os.environ.setdefault(OTEL_TRACES_SAMPLER, "parentbased_traceidratio") | ||
os.environ.setdefault(OTEL_TRACES_SAMPLER_ARG, str(DEFAULT_SAMPLING_RATE)) |
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.
We should also remove these from the documentation. Need to check priority of OTEL_TRACES_SAMPLER
env var against the configure
parameter. An option is to provide our own entry points and just switch to it there?
I consider EDOT to provide an opinionated agent. If customers need better configurability, let them feed that back rather than we try an anticipate needs that might not be requested |
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.
@xrmx based on the test logging, it looks like there was already an issue with loading entry points, not just my new one. I guess it's a test infrastructure issue, can you help with it? Note that I run locally using uv run pytest
and it works fine, so maybe pip specific.
WARNING opentelemetry.sdk._configuration:__init__.py:390 Using default sampler. Failed to initialize sampler, experimental_composite_parentbased_traceidratio: Requested component 'experimental_composite_parentbased_traceidratio' not found in entry point 'opentelemetry_traces_sampler'
ERROR opentelemetry.sdk.resources:__init__.py:224 Failed to load resource detector 'service_instance', skipping
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.11.13/x64/lib/python3.11/site-packages/opentelemetry/sdk/resources/__init__.py", line 214, in create
next(
StopIteration
ERROR opentelemetry.sdk.resources:__init__.py:224 Failed to load resource detector '_gcp', skipping
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.11.13/x64/lib/python3.11/site-packages/opentelemetry/sdk/resources/__init__.py", line 214, in create
next(
StopIteration
ERROR opentelemetry.sdk.resources:__init__.py:224 Failed to load resource detector 'process_runtime', skipping
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.11.13/x64/lib/python3.11/site-packages/opentelemetry/sdk/resources/__init__.py", line 214, in create
next(
StopIteration
What does this pull request do?
This wires up the new composite sampler to EDOT.
I followed the same approach as EDOT Java which enables the sampler unconditionally I believe
https://github.com/elastic/elastic-otel-java/blob/1254b84e16c5cf6829f2b84bf63c902b71626448/custom/src/main/java/co/elastic/otel/ElasticAutoConfigurationCustomizerProvider.java#L95
Happy to change to registering a OTEL_TRACES_SAMPLER name instead, presumably the behavior should be consistent between the two though - I can update Java if we change it.
/cc @xrmx @AlexanderWert @jackshirazi