Skip to content
This repository has been archived by the owner on Dec 23, 2023. It is now read-only.

Add documentation about binary encoding format that we are using and the trace context. #550

Closed

Conversation

bogdandrutu
Copy link
Contributor

TODO: Add tags encoding.

@bogdandrutu
Copy link
Contributor Author

cc @adriancole

@@ -0,0 +1,99 @@
# BINARY FORMAT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe put a see-also link somewhere to the trace context repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a good start. trace context needs a description. for example, that it is the parent and not intended to be shared across network boundaries.

@codecov-io
Copy link

codecov-io commented Aug 24, 2017

Codecov Report

Merging #550 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #550   +/-   ##
=======================================
  Coverage        90%    90%           
  Complexity      566    566           
=======================================
  Files           110    110           
  Lines          2361   2361           
  Branches        211    211           
=======================================
  Hits           2125   2125           
  Misses          180    180           
  Partials         56     56

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87e78c3...abd3387. Read the comment docs.

@bogdandrutu
Copy link
Contributor Author

102, 103, 104, 2, 1}

This corresponds to:
* `traceId` = {64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very confusing because it uses byte values which happen to be valid ASCII characters, leading one to think that they are perhaps hex digits or something. It would be equally valid, and more instructive, to have a value like:

traceId = { 00, 127, 128, 255, 10, 13, 00, 00, 01, 02, 03, 04, 64, 65, 66, 67 }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, can you make a quick PR to change this?

https://github.com/census-instrumentation/opencensus-specs


Is the ID of the whole trace forest. It is represented as a 16-bytes array,
e.g. (in hex), `4bf92f3577b34da6a3ce929d0e0e4736`. All bytes 0 is considered invalid.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explicitly mention that these bytes are (I assume) in big-endian order (== network order), as opposed to little endian?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no endianess implied here because trace-id is defined as a 16-byte array and span-id as a 8 byte array.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, you're saying that this sequence of 32 hex digits 4bf92f3577b34da6a3ce929d0e0e4736, is to be understood as a sequence of 16 bytes with values 0x4b, 0xf9, 0x2f, 0x35, 0x77, 0xb3, 0x4d, 0xa6, 0xa3, 0xce, 0x92, 0x9d, 0x0e, 0x0e, 0x47, 0x36, in that order, but not to be interpreted as the 128 bit number 0x4bf92f3577b34da6a3ce929d0e0e4736 aka 100985939111033328018442752961257817910 in base 10.

This is confusing to me, because we do actually internally treat the span id as a 64 bit integer. I.e. we treat spanId = {97, 98, 99, 100, 101, 102, 103, 104} from your example as equivalent to (converting to hex digits) spanId = { 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68 }, and then (concatenating the bytes, treating as big-endian and converting to decimal) spanId = 7017280452245743464 as from an HTTP header with value <traceid>/7017280452245743464;<options>

@bogdandrutu bogdandrutu deleted the binaryencoding branch August 30, 2017 05:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants