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

Make id zero reserved as no ID #67

Open
codefromthecrypt opened this issue Dec 22, 2016 · 3 comments
Open

Make id zero reserved as no ID #67

codefromthecrypt opened this issue Dec 22, 2016 · 3 comments

Comments

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Dec 22, 2016

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:

  • In zipkin parent id being absent means it is the root span, ex you leave the field out of json (or set it to null) vs setting it to 0
  • In finagle's fixed-width encoding, there was a special comparison of when span id == trace id == parent id, it is the root span. This is interesting because it doesn't require interpreting the value ids, rather comparing them

Here are some reasons to make zero special:

  • In some languages zero means false, so dodging ID zero can avoid some confusion when people write custom zipkin code.
  • in fixed-width binary encoding, you cannot represent null without a flag. If zero implies no ID, you don't need a flag.
  • In memory, the parent id is bigger since we need a reference to indicate if it is null or not. If zero meant unset, we don't need to box the u64.

Here is the work needed if we make zero special:

  • we'd coerce zero to null where we accept span IDs
  • all instrumentation need to avoid generating zero (ex while (id == 0) id = randomId )
  • we have to be explicit that by zero we really mean no bits :) numeric interpretation of IDs has led to confusion before, especially encoding.

Thoughts? cc @openzipkin/instrumentation-owners @mansu @bogdandrutu @openzipkin/core

@mansu
Copy link

mansu commented Dec 22, 2016

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?

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Dec 22, 2016 via email

@basvanbeek
Copy link
Member

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants