-
Notifications
You must be signed in to change notification settings - Fork 825
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
JaegerRemoteSampler inconsistent Implementation #5504
Comments
@pavolloffay can you comment on this? |
@jack-berg @jkwatson - I would like to try solving this if thats ok :) |
correct it should be fixed in OTEL SDK |
I was debugging this over the weekend and to me it seems the issue is with jaeger not returning probablistic operation sampling strategies if the service configuration is ratelimiting. In theory it would be possible to call Jaeger twice, get strategies for a given service, then get defaults and merge them before starting to use them, but to me it looks like a mistake in Jaeger backend and this "calling jaeger twice" does not feel right. |
Just a small update - after updating jaeger to return the operation level default probablistic sampling strategies for ratelimiting service level sampling strategy configurations - the issue can't be reproduced anymore. |
…egies without them (#5277) ## Which problem is this PR solving? - Part 1 of [#5270] ## Description of the changes - See issue for description of the bug and of the two changed behaviors - Default `probabilistic` operation level strategies will now be returned if service strategy is of `ratelimiting` type - If the service itself has `probabilistic` strategy, its sampling rate takes priority for operation-level definition ## How was this change tested? - Via updated unit test - By building local all-in-one executable and using it as a otel instrumentation backend for simple java service https://github.com/kuujis/opentelemetry-java-5504 and using the below `sampling_strategies.json` from open-telemetry/opentelemetry-java#5504 ``` { "service_strategies": [ { "service": "foobar", "type": "ratelimiting", "param": 2 } ], "default_strategy": { "type": "probabilistic", "param": 0.2, "operation_strategies": [ { "operation": "metrics", "type": "probabilistic", "param": 0.0 } ] } } ``` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Kazimieras Pociunas <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
This should work out of the box with next release of Jaeger, provided flag |
…egies without them (jaegertracing#5277) ## Which problem is this PR solving? - Part 1 of [jaegertracing#5270] ## Description of the changes - See issue for description of the bug and of the two changed behaviors - Default `probabilistic` operation level strategies will now be returned if service strategy is of `ratelimiting` type - If the service itself has `probabilistic` strategy, its sampling rate takes priority for operation-level definition ## How was this change tested? - Via updated unit test - By building local all-in-one executable and using it as a otel instrumentation backend for simple java service https://github.com/kuujis/opentelemetry-java-5504 and using the below `sampling_strategies.json` from open-telemetry/opentelemetry-java#5504 ``` { "service_strategies": [ { "service": "foobar", "type": "ratelimiting", "param": 2 } ], "default_strategy": { "type": "probabilistic", "param": 0.2, "operation_strategies": [ { "operation": "metrics", "type": "probabilistic", "param": 0.0 } ] } } ``` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Kazimieras Pociunas <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Vamshi Maskuri <[email protected]>
The implementation of SDK Jaeger Remote Sampler Extension is not according to the description of the example of Jaeger Remote Sampler.
Having the remote sampling configuration above I would expect service
foobar
to be rate limited to 2 and all other services to be sampled with probability 0.2. As an exception operation/metrics
of all services (including foobar) never gets sampled.With the current implementation only rate limiting is configured for service
foobar
which is also applied to/metrics
operation.Also when operation_strategies are configured for a service then the default type of the service is always "probabilistic" even when "ratelimiting" is configured.
I've used the following libraries and versions:
io.opentelemetry:opentelemetry-sdk-extension-autoconfigure:1.26.0-alpha
io.opentelemetry:opentelemetry-sdk-extension-jaeger-remote-sampler:1.26.0-alpha
And activated the autoconfiguration with
export OTEL_TRACES_SAMPLER=parentbased_jaeger_remote
The text was updated successfully, but these errors were encountered: