Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include trace flags in otlp marshaller #6167

Merged
merged 11 commits into from
Jan 31, 2024
2 changes: 1 addition & 1 deletion dependencyManagement/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ val DEPENDENCIES = listOf(
"eu.rekawek.toxiproxy:toxiproxy-java:2.1.7",
"io.github.netmikey.logunit:logunit-jul:2.0.0",
"io.jaegertracing:jaeger-client:1.8.1",
"io.opentelemetry.proto:opentelemetry-proto:1.0.0-alpha",
"io.opentelemetry.proto:opentelemetry-proto:1.1.0-alpha",
"io.opentelemetry.contrib:opentelemetry-aws-xray-propagator:1.29.0-alpha",
"io.opentracing:opentracing-api:0.33.0",
"io.opentracing:opentracing-noop:0.33.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ protected MarshalerWithSize(int size) {
this.size = size;
}

/** Vendored {@link Byte#toUnsignedInt(byte)} to support Android. */
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
protected static int toUnsignedInt(byte x) {
return ((int) x) & 0xff;
}

@Override
public final int getBinarySerializedSize() {
return size;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,4 @@ static ProtoEnumInfo toProtoSeverityNumber(Severity severity) {
// NB: Should not be possible with aligned versions.
return SeverityNumber.SEVERITY_NUMBER_UNSPECIFIED;
}

/** Vendored {@link Byte#toUnsignedInt(byte)} to support Android. */
private static int toUnsignedInt(byte x) {
return ((int) x) & 0xff;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static io.opentelemetry.api.trace.propagation.internal.W3CTraceContextEncoding.encodeTraceState;

import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.exporter.internal.marshal.MarshalerUtil;
import io.opentelemetry.exporter.internal.marshal.MarshalerWithSize;
Expand All @@ -26,6 +27,7 @@ final class SpanLinkMarshaler extends MarshalerWithSize {
private final byte[] traceStateUtf8;
private final KeyValueMarshaler[] attributeMarshalers;
private final int droppedAttributesCount;
private final TraceFlags traceFlags;

static SpanLinkMarshaler[] createRepeated(List<LinkData> links) {
if (links.isEmpty()) {
Expand All @@ -51,6 +53,7 @@ static SpanLinkMarshaler create(LinkData link) {
return new SpanLinkMarshaler(
link.getSpanContext().getTraceId(),
link.getSpanContext().getSpanId(),
link.getSpanContext().getTraceFlags(),
traceStateUtf8,
KeyValueMarshaler.createForAttributes(link.getAttributes()),
link.getTotalAttributeCount() - link.getAttributes().size());
Expand All @@ -59,14 +62,21 @@ static SpanLinkMarshaler create(LinkData link) {
private SpanLinkMarshaler(
String traceId,
String spanId,
TraceFlags traceFlags,
byte[] traceStateUtf8,
KeyValueMarshaler[] attributeMarshalers,
int droppedAttributesCount) {
super(
calculateSize(
traceId, spanId, traceStateUtf8, attributeMarshalers, droppedAttributesCount));
traceId,
spanId,
traceFlags,
traceStateUtf8,
attributeMarshalers,
droppedAttributesCount));
this.traceId = traceId;
this.spanId = spanId;
this.traceFlags = traceFlags;
this.traceStateUtf8 = traceStateUtf8;
this.attributeMarshalers = attributeMarshalers;
this.droppedAttributesCount = droppedAttributesCount;
Expand All @@ -79,11 +89,13 @@ public void writeTo(Serializer output) throws IOException {
output.serializeString(Span.Link.TRACE_STATE, traceStateUtf8);
output.serializeRepeatedMessage(Span.Link.ATTRIBUTES, attributeMarshalers);
output.serializeUInt32(Span.Link.DROPPED_ATTRIBUTES_COUNT, droppedAttributesCount);
output.serializeFixed32(Span.Link.FLAGS, toUnsignedInt(traceFlags.asByte()));
}

private static int calculateSize(
String traceId,
String spanId,
TraceFlags flags,
byte[] traceStateUtf8,
KeyValueMarshaler[] attributeMarshalers,
int droppedAttributesCount) {
Expand All @@ -93,6 +105,7 @@ private static int calculateSize(
size += MarshalerUtil.sizeBytes(Span.Link.TRACE_STATE, traceStateUtf8);
size += MarshalerUtil.sizeRepeatedMessage(Span.Link.ATTRIBUTES, attributeMarshalers);
size += MarshalerUtil.sizeUInt32(Span.Link.DROPPED_ATTRIBUTES_COUNT, droppedAttributesCount);
size += MarshalerUtil.sizeFixed32(Span.Link.FLAGS, toUnsignedInt(flags.asByte()));
return size;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static io.opentelemetry.api.trace.propagation.internal.W3CTraceContextEncoding.encodeTraceState;

import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.exporter.internal.marshal.MarshalerUtil;
import io.opentelemetry.exporter.internal.marshal.MarshalerWithSize;
Expand Down Expand Up @@ -37,6 +38,7 @@ final class SpanMarshaler extends MarshalerWithSize {
private final SpanLinkMarshaler[] spanLinkMarshalers;
private final int droppedLinksCount;
private final SpanStatusMarshaler spanStatusMarshaler;
private final TraceFlags flags;

// Because SpanMarshaler is always part of a repeated field, it cannot return "null".
static SpanMarshaler create(SpanData spanData) {
Expand Down Expand Up @@ -72,7 +74,8 @@ static SpanMarshaler create(SpanData spanData) {
spanData.getTotalRecordedEvents() - spanData.getEvents().size(),
spanLinkMarshalers,
spanData.getTotalRecordedLinks() - spanData.getLinks().size(),
SpanStatusMarshaler.create(spanData.getStatus()));
SpanStatusMarshaler.create(spanData.getStatus()),
spanData.getSpanContext().getTraceFlags());
}

private SpanMarshaler(
Expand All @@ -90,7 +93,8 @@ private SpanMarshaler(
int droppedEventsCount,
SpanLinkMarshaler[] spanLinkMarshalers,
int droppedLinksCount,
SpanStatusMarshaler spanStatusMarshaler) {
SpanStatusMarshaler spanStatusMarshaler,
TraceFlags flags) {
super(
calculateSize(
traceId,
Expand All @@ -107,7 +111,8 @@ private SpanMarshaler(
droppedEventsCount,
spanLinkMarshalers,
droppedLinksCount,
spanStatusMarshaler));
spanStatusMarshaler,
flags));
this.traceId = traceId;
this.spanId = spanId;
this.traceStateUtf8 = traceStateUtf8;
Expand All @@ -123,6 +128,7 @@ private SpanMarshaler(
this.spanLinkMarshalers = spanLinkMarshalers;
this.droppedLinksCount = droppedLinksCount;
this.spanStatusMarshaler = spanStatusMarshaler;
this.flags = flags;
}

@Override
Expand All @@ -148,6 +154,7 @@ public void writeTo(Serializer output) throws IOException {
output.serializeUInt32(Span.DROPPED_LINKS_COUNT, droppedLinksCount);

output.serializeMessage(Span.STATUS, spanStatusMarshaler);
output.serializeFixed32(Span.FLAGS, toUnsignedInt(flags.asByte()));
}

private static int calculateSize(
Expand All @@ -165,7 +172,8 @@ private static int calculateSize(
int droppedEventsCount,
SpanLinkMarshaler[] spanLinkMarshalers,
int droppedLinksCount,
SpanStatusMarshaler spanStatusMarshaler) {
SpanStatusMarshaler spanStatusMarshaler,
TraceFlags flags) {
int size = 0;
size += MarshalerUtil.sizeTraceId(Span.TRACE_ID, traceId);
size += MarshalerUtil.sizeSpanId(Span.SPAN_ID, spanId);
Expand All @@ -188,6 +196,7 @@ private static int calculateSize(
size += MarshalerUtil.sizeUInt32(Span.DROPPED_LINKS_COUNT, droppedLinksCount);

size += MarshalerUtil.sizeMessage(Span.STATUS, spanStatusMarshaler);
size += MarshalerUtil.sizeFixed32(Span.FLAGS, toUnsignedInt(flags.asByte()));
return size;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.exporter.internal.otlp.traces;

import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static java.util.Collections.singletonList;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.testing.trace.TestSpanData;
import io.opentelemetry.sdk.trace.data.EventData;
import io.opentelemetry.sdk.trace.data.LinkData;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.data.StatusData;
import java.io.ByteArrayOutputStream;
import java.nio.charset.StandardCharsets;
import java.util.List;
import org.junit.jupiter.api.Test;

class SpanMarshalerTest {

@Test
void marshalToJson() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

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);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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. 🙃

String traceId = "abcabcabcabcabcabcabcabcabcabcab";
String spanId = "1234123412341234";
SpanContext spanContext =
SpanContext.create(traceId, spanId, TraceFlags.getSampled(), TraceState.getDefault());
long now = 1706048791269000000L;
long later = 1706048791274000000L;
EventData event = EventData.create(now + 42L, "reboot", Attributes.of(stringKey("e1"), "v1"));
List<EventData> events = singletonList(event);
LinkData link = LinkData.create(spanContext, Attributes.of(stringKey("link1"), "lv1"));
List<LinkData> links = singletonList(link);
SpanData spanData =
TestSpanData.builder()
.setHasEnded(true)
.setSpanContext(spanContext)
.setParentSpanContext(SpanContext.getInvalid())
.setName("testSpan")
.setKind(SpanKind.SERVER)
.setStartEpochNanos(now)
.setEndEpochNanos(later)
.setStatus(StatusData.create(StatusCode.OK, "radical"))
.setAttributes(Attributes.of(stringKey("flim"), "flam"))
.setInstrumentationScopeInfo(
InstrumentationScopeInfo.builder("testInst")
.setVersion("1.0")
.setSchemaUrl("http://url")
.setAttributes(Attributes.builder().put("foo", "bar").build())
.build())
.setResource(Resource.builder().put("rez", 42).setSchemaUrl("http://url").build())
.setEvents(events)
.setLinks(links)
.build();
SpanMarshaler spanMarshaler = SpanMarshaler.create(spanData);

ByteArrayOutputStream out = new ByteArrayOutputStream();
spanMarshaler.writeJsonTo(out);
String result = out.toString(StandardCharsets.UTF_8.name());

String expected =
"{\"traceId\":\"abcabcabcabcabcabcabcabcabcabcab\",\"spanId\":\"1234123412341234\","
+ "\"name\":\"testSpan\",\"kind\":2,"
+ "\"startTimeUnixNano\":\"1706048791269000000\",\"endTimeUnixNano\":\"1706048791274000000\","
+ "\"attributes\":["
+ "{\"key\":\"flim\",\"value\":{\"stringValue\":\"flam\"}}"
+ "],"
+ "\"droppedAttributesCount\":-1,"
+ "\"events\":[{\"timeUnixNano\":\"1706048791269000042\",\"name\":\"reboot\",\"attributes\":[{\"key\":\"e1\",\"value\":{\"stringValue\":\"v1\"}}]}],\"droppedEventsCount\":-1,"
+ "\"links\":[{\"traceId\":\"abcabcabcabcabcabcabcabcabcabcab\",\"spanId\":\"1234123412341234\",\"attributes\":[{\"key\":\"link1\",\"value\":{\"stringValue\":\"lv1\"}}],\"flags\":1}],\"droppedLinksCount\":-1,"
+ "\"status\":{\"message\":\"radical\",\"code\":1},"
+ "\"flags\":1}";
assertThat(result).isEqualTo(expected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ void toProtoSpan() {
Span.Link.newBuilder()
.setTraceId(ByteString.copyFrom(TRACE_ID_BYTES))
.setSpanId(ByteString.copyFrom(SPAN_ID_BYTES))
.setFlags(SPAN_CONTEXT.getTraceFlags().asByte())
.setTraceState(encodeTraceState(SPAN_CONTEXT.getTraceState()))
.build());
assertThat(span.getDroppedLinksCount()).isEqualTo(1); // 2 - 1
Expand Down Expand Up @@ -314,6 +315,7 @@ void toProtoSpanLink_WithoutAttributes() {
Span.Link.newBuilder()
.setTraceId(ByteString.copyFrom(TRACE_ID_BYTES))
.setSpanId(ByteString.copyFrom(SPAN_ID_BYTES))
.setFlags(SPAN_CONTEXT.getTraceFlags().asByte())
.setTraceState(TRACE_STATE_VALUE)
.build());
}
Expand All @@ -330,6 +332,7 @@ void toProtoSpanLink_WithAttributes() {
Span.Link.newBuilder()
.setTraceId(ByteString.copyFrom(TRACE_ID_BYTES))
.setSpanId(ByteString.copyFrom(SPAN_ID_BYTES))
.setFlags(SPAN_CONTEXT.getTraceFlags().asByte())
.setTraceState(TRACE_STATE_VALUE)
.addAttributes(
KeyValue.newBuilder()
Expand Down
Loading