otel compatible distributed tracing#496
Conversation
84c5773 to
915ea54
Compare
44c9b91 to
87070ea
Compare
87070ea to
48d105f
Compare
Abhijeet Prasad (AbhiPrasad)
left a comment
There was a problem hiding this comment.
I'm happy with this public API. Mind writing a spec doc for this so we can also be clear about the behavior difference between py/js and the otel powered sdks?
| _PERCENT_ENCODE = { | ||
| ",": "%2C", | ||
| ";": "%3B", | ||
| "=": "%3D", | ||
| " ": "%20", | ||
| "%": "%25", | ||
| } |
There was a problem hiding this comment.
this might not be enough to be spec compliant. Can we make sure this works w/ the baggage spec appropriately?
There was a problem hiding this comment.
agreed, I update the implementation to use urllib instead. also added some more round-trip unit test cases
| """ | ||
| members = [] | ||
| if existing and isinstance(existing, str): | ||
| capped = existing[:_MAX_BAGGAGE_LENGTH] if len(existing) > _MAX_BAGGAGE_LENGTH else existing |
There was a problem hiding this comment.
existing[:_MAX_BAGGAGE_LENGTH] is not safe, we might cut off a member mid value. Let's try to optimize for keeping the values around instead of sending partial values.
There was a problem hiding this comment.
yeah this is too aggressive. I updated the logic to back up to the last completed value when we blow the max size limits
There was a problem hiding this comment.
(BTW these limits come from the baggage spec if you're curious: https://www.w3.org/TR/baggage/#limits )
48d105f to
a1f514c
Compare
implements the `distributed-tracing` sdk spec - default span/trace id gen to use otel-friendly 16/8 byte hex IDs - new inject/extract tools for w3c headers
a1f514c to
c53efc2
Compare
|
Abhijeet Prasad (@AbhiPrasad) sweet! I've been working on this spec in tandem with this PR: braintrustdata/braintrust-spec#27 I just updated it to add test cases for encoding and size limits |
OLD WAY: https://www.braintrust.dev/docs/instrument/advanced-tracing#trace-distributed-systems
NOTE: the "old way" is still supported and backwards-compatible. Native SDKs can continue to link to each other in the manner, even if one is upgraded and the other is not. However if users with to link to otel sdks, they will have to change their code to use the new apis.
NEW WAY:
Other notes
root_span_idfield essentially becoming the trace id rather than the id of the root span (i.e.root_span_id!=rootSpan.id). This sucks, but it's already the case for otel sdks and py otel compat mode. So at least we're consistent.Examples