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

Probability sampler is broken for low sample rates #378

Closed
akoshochrein opened this issue Nov 1, 2018 · 2 comments
Closed

Probability sampler is broken for low sample rates #378

akoshochrein opened this issue Nov 1, 2018 · 2 comments

Comments

@akoshochrein
Copy link

Hello everyone,

I've found an issue with the probability sampler not working for low sample rates. There are 2 piecens of code related to this:
https://github.com/census-instrumentation/opencensus-python/blob/master/opencensus/trace/samplers/probability.py#L56-L66
As the code says, it fetches the last 8 bytes of the trace id, that will later be used to compare to a constant value calculated using the sample rate. This algorithm is, in fact, flawless and propagates traces perfectly.
The issue comes from a bit deeper:
https://github.com/census-instrumentation/opencensus-python/blob/master/opencensus/trace/span_context.py#L165-L173
As a matter of fact, uuid4 is not an entirely random number according to the uuid specs.
Here's some empyrical data as proof:
https://gist.github.com/akoskaaa/a53c8b0201a8bffc1ae4df91e5965933

Source:
https://tools.ietf.org/html/rfc4122
https://en.wikipedia.org/wiki/Universally_unique_identifier

Solution could be to change the tracing id generation to a more random one. I've actually patched it in my systems with this function:

def patched_generate_trace_id():
    hexdigits = '0123456789abcdef'
    return ''.join([hexdigits[random.randint(0, 0xF)] for _ in range(32)])

Do you have any better ideas?

Also, I'd just like to say thanks to the owners and everyone who put time into making this happen. Super useful!

@reyang
Copy link
Contributor

reyang commented Apr 15, 2019

Should be fixed by #619, big thank you to @ewhauser!

@reyang reyang closed this as completed Apr 15, 2019
@akoshochrein
Copy link
Author

Huge thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants