From f493e26481f3808d056453b33de709099351cba9 Mon Sep 17 00:00:00 2001 From: Toshiaki Maki Date: Thu, 26 Sep 2024 12:40:24 +0900 Subject: [PATCH] Support adding static resource attributes on the encoder side (#18) This pull request adds a way to add static resource attributes on the encoder side. Related to #14, resource attributes are typically static and do not depend on a span. For example, Spring Boot provides `management.opentelemetry.resource-attributes.*` properties to add resource attributes for Otel SDK. This pr will provide the feature parity of this capability for Brave. https://github.com/spring-projects/spring-boot/blob/aba22547ee56606c815ecb1ef57cee82e9005ed9/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryAutoConfiguration.java#L74-L91 --- .../otel/brave/OtlpProtoV1Encoder.java | 53 +++++++- .../reporter/otel/brave/SpanTranslator.java | 33 +++-- .../otel/brave/ITOtlpProtoV1EncoderTest.java | 4 +- .../OtelToZipkinSpanTransformerTest.java | 122 +++++++++++++++--- .../otel/brave/SpanTranslatorTest.java | 3 +- 5 files changed, 173 insertions(+), 42 deletions(-) diff --git a/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/OtlpProtoV1Encoder.java b/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/OtlpProtoV1Encoder.java index 0a3f0ca..b975e04 100644 --- a/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/OtlpProtoV1Encoder.java +++ b/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/OtlpProtoV1Encoder.java @@ -4,18 +4,59 @@ */ package zipkin2.reporter.otel.brave; +import java.util.Collections; +import java.util.Map; + import brave.Tag; +import brave.Tags; import brave.handler.MutableSpan; import io.opentelemetry.proto.trace.v1.TracesData; import zipkin2.reporter.BytesEncoder; import zipkin2.reporter.Encoding; public final class OtlpProtoV1Encoder implements BytesEncoder { + public static OtlpProtoV1Encoder create() { + return newBuilder().build(); + } + + public static Builder newBuilder() { + return new Builder(); + } + + public static final class Builder { + Tag errorTag = Tags.ERROR; + + Map resourceAttributes = Collections.emptyMap(); + + /** The throwable parser. Defaults to {@link Tags#ERROR}. */ + public Builder errorTag(Tag errorTag) { + if (errorTag == null) { + throw new NullPointerException("errorTag == null"); + } + this.errorTag = errorTag; return this; + } + + /** static resource attributes added to a {@link io.opentelemetry.proto.resource.v1.Resource}. Defaults to empty map. */ + public Builder resourceAttributes(Map resourceAttributes) { + if (resourceAttributes == null) { + throw new NullPointerException("resourceAttributes == null"); + } + this.resourceAttributes = resourceAttributes; return this; + } + + public OtlpProtoV1Encoder build() { + return new OtlpProtoV1Encoder(this); + } + + Builder() { + + } + } + final SpanTranslator spanTranslator; - public OtlpProtoV1Encoder(Tag errorTag) { - if (errorTag == null) throw new NullPointerException("errorTag == null"); - this.spanTranslator = new SpanTranslator(errorTag); + private OtlpProtoV1Encoder(Builder builder) { + this.spanTranslator = new SpanTranslator(builder.errorTag, builder.resourceAttributes); } @Override @@ -23,13 +64,15 @@ public Encoding encoding() { return Encoding.PROTO3; } - @Override public int sizeInBytes(MutableSpan span) { + @Override + public int sizeInBytes(MutableSpan span) { // TODO: Create a proto size function to avoid allocations here TracesData convert = translate(span); return encoding().listSizeInBytes(convert.getSerializedSize()); } - @Override public byte[] encode(MutableSpan span) { + @Override + public byte[] encode(MutableSpan span) { return translate(span).toByteArray(); } diff --git a/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/SpanTranslator.java b/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/SpanTranslator.java index 0363cf1..e6b1197 100644 --- a/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/SpanTranslator.java +++ b/encoder-brave/src/main/java/zipkin2/reporter/otel/brave/SpanTranslator.java @@ -18,6 +18,7 @@ import io.opentelemetry.proto.common.v1.AnyValue; import io.opentelemetry.proto.common.v1.InstrumentationScope; import io.opentelemetry.proto.common.v1.KeyValue; +import io.opentelemetry.proto.resource.v1.Resource; import io.opentelemetry.proto.trace.v1.ResourceSpans; import io.opentelemetry.proto.trace.v1.ResourceSpans.Builder; import io.opentelemetry.proto.trace.v1.ScopeSpans; @@ -78,8 +79,11 @@ final class SpanTranslator { private final TagMapper tagMapper; - SpanTranslator(Tag errorTag) { + private final Map resourceAttributes; + + SpanTranslator(Tag errorTag, Map resourceAttributes) { this.tagMapper = new TagMapper(errorTag, TAG_TO_ATTRIBUTE); + this.resourceAttributes = resourceAttributes; } TracesData translate(MutableSpan braveSpan) { @@ -101,7 +105,7 @@ private Span.Builder builderForSingleSpan(MutableSpan span, Builder resourceSpan Span.Builder spanBuilder = Span.newBuilder() .setTraceId(span.traceId() != null ? ByteString.fromHex(span.traceId()) : INVALID_TRACE_ID) .setSpanId(span.id() != null ? ByteString.fromHex(span.id()) : INVALID_SPAN_ID) - .setName(span.name() == null || span.name().isEmpty() ? DEFAULT_SPAN_NAME : span.name()); + .setName(span.name() == null || span.name().isEmpty() ? DEFAULT_SPAN_NAME : span.name()); if (span.parentId() != null) { spanBuilder.setParentSpanId(ByteString.fromHex(span.parentId())); } @@ -110,11 +114,15 @@ private Span.Builder builderForSingleSpan(MutableSpan span, Builder resourceSpan spanBuilder.setStartTimeUnixNano(TimeUnit.MICROSECONDS.toNanos(start)); spanBuilder.setEndTimeUnixNano(TimeUnit.MICROSECONDS.toNanos(finish)); spanBuilder.setKind(translateKind(span.kind())); - String localServiceName = span.localServiceName(); - if (localServiceName == null || localServiceName.isEmpty()) { - localServiceName = DEFAULT_SERVICE_NAME; + Resource.Builder resourceBuilder = resourceSpansBuilder.getResourceBuilder(); + if (!this.resourceAttributes.containsKey(ServiceAttributes.SERVICE_NAME.getKey())) { + String localServiceName = span.localServiceName(); + if (localServiceName == null || localServiceName.isEmpty()) { + localServiceName = DEFAULT_SERVICE_NAME; + } + resourceBuilder.addAttributes(stringAttribute(ServiceAttributes.SERVICE_NAME.getKey(), localServiceName)); } - resourceSpansBuilder.getResourceBuilder().addAttributes(stringAttribute(ServiceAttributes.SERVICE_NAME.getKey(), localServiceName)); + resourceAttributes.forEach((k, v) -> resourceBuilder.addAttributes(stringAttribute(k, v))); maybeAddStringAttribute(spanBuilder, NetworkAttributes.NETWORK_LOCAL_ADDRESS.getKey(), span.localIp()); maybeAddIntAttribute(spanBuilder, NetworkAttributes.NETWORK_LOCAL_PORT.getKey(), span.localPort()); maybeAddStringAttribute(spanBuilder, NetworkAttributes.NETWORK_PEER_ADDRESS.getKey(), span.remoteIp()); @@ -123,7 +131,6 @@ private Span.Builder builderForSingleSpan(MutableSpan span, Builder resourceSpan span.forEachTag(tagMapper, spanBuilder); span.forEachAnnotation(tagMapper, spanBuilder); tagMapper.addErrorTag(spanBuilder, span); - return spanBuilder; } @@ -145,16 +152,16 @@ private static SpanKind translateKind(Kind kind) { static KeyValue stringAttribute(String key, String value) { return KeyValue.newBuilder() - .setKey(key) - .setValue(AnyValue.newBuilder().setStringValue(value)) - .build(); + .setKey(key) + .setValue(AnyValue.newBuilder().setStringValue(value)) + .build(); } static KeyValue intAttribute(String key, int value) { return KeyValue.newBuilder() - .setKey(key) - .setValue(AnyValue.newBuilder().setIntValue(value)) - .build(); + .setKey(key) + .setValue(AnyValue.newBuilder().setIntValue(value)) + .build(); } private static void maybeAddStringAttribute(Span.Builder spanBuilder, String key, String value) { diff --git a/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/ITOtlpProtoV1EncoderTest.java b/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/ITOtlpProtoV1EncoderTest.java index be073fa..8ac13e7 100644 --- a/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/ITOtlpProtoV1EncoderTest.java +++ b/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/ITOtlpProtoV1EncoderTest.java @@ -112,9 +112,9 @@ void beforeEach() { static Stream encoderAndEndpoint() { return Stream.of( /* existing sender + new encoder -> otlp with otel collector */ - Arguments.of(Encoding.PROTO3, new OtlpProtoV1Encoder(Tags.ERROR), String.format("http://localhost:%d/v1/traces", collector.getMappedPort(COLLECTOR_OTLP_HTTP_PORT))), + Arguments.of(Encoding.PROTO3, OtlpProtoV1Encoder.create(), String.format("http://localhost:%d/v1/traces", collector.getMappedPort(COLLECTOR_OTLP_HTTP_PORT))), /* existing sender + new encoder -> otlp without otel collector */ - Arguments.of(Encoding.PROTO3, new OtlpProtoV1Encoder(Tags.ERROR), String.format("http://localhost:%d/v1/traces", otlpHttpServer.httpPort())), + Arguments.of(Encoding.PROTO3, OtlpProtoV1Encoder.create(), String.format("http://localhost:%d/v1/traces", otlpHttpServer.httpPort())), /* existing sender + zipkin encoder -> zipkin endpoint on collector */ Arguments.of(Encoding.JSON, MutableSpanBytesEncoder.create(Encoding.JSON, Tags.ERROR), String.format("http://localhost:%d/api/v2/spans", collector.getMappedPort(COLLECTOR_ZIPKIN_PORT)))); } diff --git a/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/OtelToZipkinSpanTransformerTest.java b/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/OtelToZipkinSpanTransformerTest.java index ae599ec..1309ffa 100644 --- a/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/OtelToZipkinSpanTransformerTest.java +++ b/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/OtelToZipkinSpanTransformerTest.java @@ -4,13 +4,14 @@ */ package zipkin2.reporter.otel.brave; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import brave.Span; import brave.Span.Kind; -import brave.Tags; import brave.handler.MutableSpan; import com.google.protobuf.ByteString; import io.opentelemetry.proto.common.v1.InstrumentationScope; @@ -26,7 +27,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; -import static io.opentelemetry.api.common.AttributeKey.stringKey; import static io.opentelemetry.proto.trace.v1.Span.newBuilder; import static org.assertj.core.api.Assertions.assertThat; import static zipkin2.reporter.otel.brave.SpanTranslator.intAttribute; @@ -41,18 +41,18 @@ class OtelToZipkinSpanTransformerTest { static final String PARENT_SPAN_ID = "8b03ab423da481c5"; - private OtlpProtoV1Encoder transformer; + private OtlpProtoV1Encoder encoder; @BeforeEach void setup() { - transformer = new OtlpProtoV1Encoder(Tags.ERROR); + encoder = OtlpProtoV1Encoder.create(); } @Test void generateSpan_remoteParent() { MutableSpan data = span(Kind.SERVER); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -79,13 +79,93 @@ void generateSpan_remoteParent() { .build()); } + @Test + void generateSpan_resourceAttributes() { + MutableSpan data = span(Kind.SERVER); + Map resourceAttributes = new LinkedHashMap<>(); + resourceAttributes.put("java.version", "21.0.4"); + resourceAttributes.put("os.name", "Linux"); + resourceAttributes.put("os.arch", "amd64"); + resourceAttributes.put("hostname", "localhost"); + OtlpProtoV1Encoder encoder = OtlpProtoV1Encoder.newBuilder().resourceAttributes(resourceAttributes).build(); + assertThat(encoder.translate(data)) + .isEqualTo( + TracesData.newBuilder().addResourceSpans(ResourceSpans + .newBuilder() + .setResource(io.opentelemetry.proto.resource.v1.Resource.newBuilder() + .addAttributes(stringAttribute("service.name", "tweetiebird")) + .addAttributes(stringAttribute("java.version", "21.0.4")) + .addAttributes(stringAttribute("os.name", "Linux")) + .addAttributes(stringAttribute("os.arch", "amd64")) + .addAttributes(stringAttribute("hostname", "localhost"))) + .addScopeSpans(ScopeSpans.newBuilder() + .setScope(InstrumentationScope.newBuilder() + .setName(BraveScope.NAME).setVersion(BraveScope.VERSION).build()) + .addSpans(newBuilder() + .setSpanId(ByteString.fromHex(data.id())) + .setTraceId(ByteString.fromHex(data.traceId())) + .setParentSpanId(ByteString.fromHex(data.parentId())) + .setName(data.name()) + .setStartTimeUnixNano(1505855794194009000L) + .setEndTimeUnixNano(1505855799465726000L) + .setKind(SpanKind.SPAN_KIND_SERVER) + .setStatus( + Status.newBuilder().setCode(Status.StatusCode.STATUS_CODE_OK).build()) + .addEvents(Event.newBuilder().setName("\"RECEIVED\":{}").setTimeUnixNano(1505855799433901000L).build()) + .addEvents(Event.newBuilder().setName("\"SENT\":{}").setTimeUnixNano(1505855799459486000L).build()) + .build()) + .build()) + .build()) + .build()); + } + + @Test + void generateSpan_resourceAttributes_with_serviceName() { + MutableSpan data = span(Kind.SERVER); + Map resourceAttributes = new LinkedHashMap<>(); + resourceAttributes.put("service.name", "tweetie-bird"); + resourceAttributes.put("java.version", "21.0.4"); + resourceAttributes.put("os.name", "Linux"); + resourceAttributes.put("os.arch", "amd64"); + resourceAttributes.put("hostname", "localhost"); + OtlpProtoV1Encoder encoder = OtlpProtoV1Encoder.newBuilder().resourceAttributes(resourceAttributes).build(); + assertThat(encoder.translate(data)) + .isEqualTo( + TracesData.newBuilder().addResourceSpans(ResourceSpans + .newBuilder() + .setResource(io.opentelemetry.proto.resource.v1.Resource.newBuilder() + .addAttributes(stringAttribute("service.name", "tweetie-bird")) + .addAttributes(stringAttribute("java.version", "21.0.4")) + .addAttributes(stringAttribute("os.name", "Linux")) + .addAttributes(stringAttribute("os.arch", "amd64")) + .addAttributes(stringAttribute("hostname", "localhost"))) + .addScopeSpans(ScopeSpans.newBuilder() + .setScope(InstrumentationScope.newBuilder() + .setName(BraveScope.NAME).setVersion(BraveScope.VERSION).build()) + .addSpans(newBuilder() + .setSpanId(ByteString.fromHex(data.id())) + .setTraceId(ByteString.fromHex(data.traceId())) + .setParentSpanId(ByteString.fromHex(data.parentId())) + .setName(data.name()) + .setStartTimeUnixNano(1505855794194009000L) + .setEndTimeUnixNano(1505855799465726000L) + .setKind(SpanKind.SPAN_KIND_SERVER) + .setStatus( + Status.newBuilder().setCode(Status.StatusCode.STATUS_CODE_OK).build()) + .addEvents(Event.newBuilder().setName("\"RECEIVED\":{}").setTimeUnixNano(1505855799433901000L).build()) + .addEvents(Event.newBuilder().setName("\"SENT\":{}").setTimeUnixNano(1505855799459486000L).build()) + .build()) + .build()) + .build()) + .build()); + } @Test void generateSpan_subMicroDurations() { MutableSpan data = span(Kind.SERVER); data.startTimestamp(TimeUnit.NANOSECONDS.toMicros(1505855794_194009601L)); data.finishTimestamp(TimeUnit.NANOSECONDS.toMicros(1505855794_194009999L)); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -117,7 +197,7 @@ void generateSpan_subMicroDurations() { void generateSpan_ServerKind() { MutableSpan data = span(Kind.SERVER); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -148,7 +228,7 @@ void generateSpan_ServerKind() { void generateSpan_ClientKind() { MutableSpan data = span(Kind.CLIENT); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -179,7 +259,7 @@ void generateSpan_ClientKind() { void generateSpan_DefaultKind() { MutableSpan data = span(null); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -210,7 +290,7 @@ void generateSpan_DefaultKind() { void generateSpan_ConsumeKind() { MutableSpan data = span(Kind.CONSUMER); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -241,7 +321,7 @@ void generateSpan_ConsumeKind() { void generateSpan_ProducerKind() { MutableSpan data = span(Kind.PRODUCER); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -273,7 +353,7 @@ void generateSpan_ResourceServiceNameMapping() { MutableSpan data = span(Kind.PRODUCER); data.localServiceName("super-zipkin-service"); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -305,7 +385,7 @@ void generateSpan_defaultResourceServiceName() { MutableSpan data = span(Kind.PRODUCER); data.localServiceName(null); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -343,7 +423,7 @@ void generateSpan_RemoteEndpointMapping(Kind spanKind) { data.remotePort(42); data.kind(spanKind); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -380,7 +460,7 @@ void generateSpan_RemoteEndpointMappingWhenKindIsNotClientOrProducer(Kind spanKi data.remotePort(42); data.kind(spanKind); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -416,7 +496,7 @@ void generateSpan_RemoteEndpointMappingWhenServiceNameIsMissing(Kind spanKind) { data.remotePort(42); data.kind(spanKind); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -451,7 +531,7 @@ void generateSpan_RemoteEndpointMappingWhenPortIsMissing(Kind spanKind) { data.remoteIp("8.8.8.8"); data.kind(spanKind); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -485,7 +565,7 @@ void generateSpan_RemoteEndpointMappingWhenIpAndPortAreMissing(Kind spanKind) { data.remoteServiceName("remote-test-service"); data.kind(spanKind); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -522,7 +602,7 @@ void generateSpan_WithAttributes() { data.tag("doubleArray", "32.33,-98.3"); data.tag("longArray", "33,999"); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -558,7 +638,7 @@ void generateSpan_WithInstrumentationLibraryInfo() { MutableSpan data = new MutableSpan(); data.kind(Kind.CLIENT); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() @@ -588,7 +668,7 @@ void generateSpan_AlreadyHasHttpStatusInfo() { data.error(new RuntimeException("A user provided error")); data.tag("http.response.status.code", "404"); - assertThat(transformer.translate(data)) + assertThat(encoder.translate(data)) .isEqualTo( TracesData.newBuilder().addResourceSpans(ResourceSpans .newBuilder() diff --git a/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/SpanTranslatorTest.java b/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/SpanTranslatorTest.java index 6f14c3d..8c4e6e6 100644 --- a/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/SpanTranslatorTest.java +++ b/encoder-brave/src/test/java/zipkin2/reporter/otel/brave/SpanTranslatorTest.java @@ -6,6 +6,7 @@ import java.time.Instant; import java.util.Arrays; +import java.util.Collections; import java.util.concurrent.TimeUnit; import brave.Tags; @@ -28,7 +29,7 @@ class SpanTranslatorTest { - SpanTranslator spanTranslator = new SpanTranslator(Tags.ERROR); + SpanTranslator spanTranslator = new SpanTranslator(Tags.ERROR, Collections.emptyMap()); /** * This test is intentionally sensitive, so changing other parts makes obvious impact here