Include trace flags in otlp marshaller#6167
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6167 +/- ##
============================================
+ Coverage 91.00% 91.09% +0.08%
- Complexity 5646 5676 +30
============================================
Files 619 620 +1
Lines 16443 16545 +102
Branches 1663 1682 +19
============================================
+ Hits 14964 15071 +107
+ Misses 1017 1005 -12
- Partials 462 469 +7 ☔ View full report in Codecov by Sentry. |
| class SpanMarshalerTest { | ||
|
|
||
| @Test | ||
| void marshalToJson() throws Exception { |
There was a problem hiding this comment.
It seems like this test is largely duplicating what is being done in TraceRequestMarshalerTest here: https://github.com/open-telemetry/opentelemetry-java/blob/main/exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/traces/TraceRequestMarshalerTest.java#L114-L232
Can we just add an additional assertion in that test against the span flags and remove this file?
assertThat(span.getFlags() & SpanFlags.SPAN_FLAGS_TRACE_FLAGS_MASK_VALUE).isEqualTo(1);
There was a problem hiding this comment.
Sure, the SpanMarshaler was probably already covered (with tests) through the TraceMarshaller (since it does "real" testing through real collaborators, not mocking). I'll see if I can just get an assertion in there and pull this back. I think I misinterpreted the original ask. 🙃
…hod to Serializer and MarshalerUtil.
As of version 1.1.0, the OTLP protos now include a flags field, which includes the trace flags.
https://github.com/open-telemetry/opentelemetry-proto/releases/tag/v1.1.0
This change includes these flags in the marshalling of OTLP spans.