-
Notifications
You must be signed in to change notification settings - Fork 64
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
Make id zero reserved as no ID #67
Comments
I think one consequence of this change is that the span and trace ids will be numeric id now. To simplify the logic further, does it make sense to make the span id and trace id be positive numbers? |
I think one consequence of this change is that the span and trace ids will
be numeric id now. To simplify the logic further, why not make the span id
and trace id be positive numbers?
Using this to escalate into numeric interpretation of IDs is the opposite
of my intent. I don't want people ordering them or trying to encode them as
numbers etc. We've been there before and that's exactly why we use hex in
json and headers. The only reason we use longs is to get a fixed width
buffer of 64bits.
So what this proposes is that regardless of if you are using a uint64 or a
string of hex characters to hold span ids, if all bits are unset, it is the
same as null.
note I don't actually like this.. just raising the issue so that when the
next person asks, there's a place to hold the discussion.
|
It will mess up statistical distribution of a lot of Samplers which expect 0 to be a valid position. Not sure if this is of importance for people but yet another side effect. I'm not a huge fan of this idea. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Over a while, a few people have expressed interest in making span ID zero invalid. This mostly has impact to semantics of no ID and span id generators:
No ID applied to Zipkin:
Here are some reasons to make zero special:
Here is the work needed if we make zero special:
Thoughts? cc @openzipkin/instrumentation-owners @mansu @bogdandrutu @openzipkin/core
The text was updated successfully, but these errors were encountered: