Make Zipkin trace rate configurable #3968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hello @negz , could you fix the tests (failing in the CI) please (ref. https://semaphoreci.com/containous/traefik/branches/pull-request-3968/builds/3 )? Thanks for this PR 👏 |
|
@dduportal I'm starting to take a look now, but at first glance it looks pretty unlikely that my PR had anything to do with this test failure. Is there a chance it's a flaky test? |
|
the main issue: |
|
Just to update, I've been fighting the integration tests for quite some time now. I'm quite confident that even the broken Zipkin test is unrelated to my changes, but it's hard to prove that because I can't get it to pass on master on my laptop in the first place. |
|
To add some context, I'm seeing the following failure on master. So far I've run the test maybe 10 times. It's passed once but failed all the other times, despite no differences in how it was invoked. |
|
Your PR change a behavior due to: before: The set effective configuration for the tracing don't do what you think You can add this test in TestSetEffectiveConfigurationTracing to see the problem: {
desc: "tracing zipkin TEST",
tracing: &tracing.Tracing{
Backend: "zipkin",
Zipkin: &zipkin.Config{
HTTPEndpoint: "http://powpow:9411/api/v1/spans",
},
},
expected: &tracing.Tracing{
Backend: "zipkin",
Jaeger: nil,
Zipkin: &zipkin.Config{
HTTPEndpoint: "http://powpow:9411/api/v1/spans",
SameSpan: false,
ID128Bit: false,
Debug: false,
SampleRate: 0,
},
},
}, |
|
@ldez Thanks for the pointer! I had intended to make the default |
This maintains the previous behaviour of Traefik, which did not set a sampler and thus defaulted to always sampling.
ldez
added
status/2-needs-review
kind/enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Allows Zipkin traces to be sent for only a sample of between 0 and 100% of requests, expressed as a float between 0.0 and 1.0. Defaults to the current behaviour of sampling 100% of requests.
Fixes #3959.
Motivation
In large scale deployments it's not always practical or feasible to emit traces for 100% of incoming HTTP requests. For example at Planet Labs we typically sample 2% of requests for tracing in our service mesh, and a similar number at our (not-yet-Traefik) edge router.
More
Additional Notes
I couldn't resist fixing a typo and a few differing stylisations of the word Zipkin in the docs and docstrings, sorry! I did so before I saw the "don' reformat code" rule.