Feature: Include trace flags in spans exported by the OTLPSpanExporter #4761
Conversation
...roto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py
Outdated
Show resolved
Hide resolved
...roto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py
Outdated
Show resolved
Hide resolved
...roto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py
Outdated
Show resolved
Hide resolved
xrmx
moved this from Ready for review
to Reviewed PRs that need fixes
in @xrmx's Python PR digest
|
@xrmx made changes as per your comments. thanks. |
xrmx
moved this from Reviewed PRs that need fixes
to Ready for review
in @xrmx's Python PR digest
exporter/opentelemetry-exporter-otlp-proto-common/tests/test_trace_encoder.py
Outdated
Show resolved
Hide resolved
exporter/opentelemetry-exporter-otlp-proto-common/tests/test_trace_encoder.py
Outdated
Show resolved
Hide resolved
exporter/opentelemetry-exporter-otlp-proto-common/tests/test_trace_encoder.py
Outdated
Show resolved
Hide resolved
|
@xrmx I have made the changes as u said. Can you take a lookg again? |
|
@xrmx can you trigger the workflows once? I have fixed the errors |
|
@xrmx can you trigger the workflows once? I have fixed the errors |
...roto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py
Outdated
Show resolved
Hide resolved
...roto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py
Outdated
Show resolved
Hide resolved
...roto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py
Outdated
Show resolved
Hide resolved
|
can you trigger the workflows once @xrmx ? |
|
Fixed all errors. Please trigger workflows again @xrmx |
| span = SDKSpan(name="root", context=span_context, parent=None) | ||
| pb = _encode_span(span) | ||
| assert (pb.flags & PB2SpanFlags.SPAN_FLAGS_TRACE_FLAGS_MASK) == 0x00 | ||
| assert (pb.flags & PB2SpanFlags.SPAN_FLAGS_TRACE_FLAGS_MASK) == 0x00 # pylint: disable=no-member |
There was a problem hiding this comment.
Can you make this per testcase instead so we can have this only once, i.e. put this comment in first line of this class?
| ) -> int: | ||
| # Lower 8 bits: W3C TraceFlags | ||
| # Handle TraceFlags objects, regular ints, and test mocks | ||
| # TraceFlags is an int subclass, but we handle Mock objects in tests |
There was a problem hiding this comment.
Again the code should not be aware of the mocks in tests, what tests are failing? Can we update the mocks to be speced against the class we need?
There was a problem hiding this comment.
for otlp-http-exporter the fix is small, need to add trace_flags to the mocked span context, here
There was a problem hiding this comment.
for otlp-grpc-exporter will have to fix the same, but in more places here
3 participants
Description
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: