Feature: Include trace flags in spans exported by the OTLPSpanExporter - #5667
gavalur-sfdc wants to merge 2 commits into
Conversation
|
| def test_span_flags(self): | ||
| has_remote = int(JSONSpanFlags.SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK) | ||
| is_remote = int(JSONSpanFlags.SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK) | ||
| sampled = int(TraceFlags.SAMPLED) |
There was a problem hiding this comment.
For completeness, can we add test cases that include the random trace id flag.
|
Please add a changelog entry too. |
|
|
||
|
|
||
| def _span_flags(parent_span_context: SpanContext | None) -> int: | ||
| def _span_flags(span_context: SpanContext | None) -> int: |
There was a problem hiding this comment.
Probably ideal to not rename from parent_span_context to span_context since the context here represents the parent span.
There was a problem hiding this comment.
Not just the rename, but _span_flags is called with the parent span context down in _encode_span, not the current span, so this implementation would actually be buggy.
There was a problem hiding this comment.
The same comment by @lzchen also applies to the protobuf _span_flags function as well.
|
|
||
|
|
||
| def _span_flags(parent_span_context: SpanContext | None) -> int: | ||
| def _span_flags(span_context: SpanContext | None) -> int: |
There was a problem hiding this comment.
Probably ideal to not rename from parent_span_context to span_context since the context here represents the parent span.
Pull request dashboard statusWaiting on the author · refreshed 2026-09-18 01:51 UTC Two things need attention:
Status above doesn't look right?
|
4 participants
Description
Update OTLP JSON and protobuf span encoders to include a span context’s trace flags, such as the sampled flag, in exported span flags.
Fixes # (4666)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Contrib Repo Change?
Checklist: