From ac1099ee9b0a635d033942b9cd48802a4bd90dfb Mon Sep 17 00:00:00 2001 From: dengliming Date: Thu, 12 Nov 2020 04:36:05 +0800 Subject: [PATCH] Update the zipkin exporter Span Status handling (#2050) CHANGELOG: Zipkin exporter has had it's handling of Span Status to match the specification. * Update the zipkin exporter Span Status handling * Fix review * Remove unneeded check null --- .../exporter/zipkin/ZipkinSpanExporter.java | 7 +- .../ZipkinSpanExporterEndToEndHttpTest.java | 1 + .../zipkin/ZipkinSpanExporterTest.java | 73 ++++++++++++++++--- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporter.java b/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporter.java index 250874bea8..0813a5facb 100644 --- a/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporter.java +++ b/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporter.java @@ -14,7 +14,6 @@ import io.opentelemetry.api.common.AttributeType; import io.opentelemetry.api.common.ReadableAttributes; import io.opentelemetry.api.trace.Span.Kind; import io.opentelemetry.api.trace.SpanId; -import io.opentelemetry.api.trace.attributes.SemanticAttributes; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.common.export.ConfigBuilder; @@ -140,15 +139,15 @@ public final class ZipkinSpanExporter implements SpanExporter { } }); SpanData.Status status = spanData.getStatus(); - // for GRPC spans, include status code & description. - if (status != null && spanAttributes.get(SemanticAttributes.RPC_SERVICE) != null) { + // include status code & description. + if (!status.isUnset()) { spanBuilder.putTag(OTEL_STATUS_CODE, status.getCanonicalCode().toString()); if (status.getDescription() != null) { spanBuilder.putTag(OTEL_STATUS_DESCRIPTION, status.getDescription()); } } // add the error tag, if it isn't already in the source span. - if (status != null && !status.isOk() && spanAttributes.get(STATUS_ERROR) == null) { + if (!status.isOk() && spanAttributes.get(STATUS_ERROR) == null) { spanBuilder.putTag(STATUS_ERROR.getKey(), status.getCanonicalCode().toString()); } diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterEndToEndHttpTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterEndToEndHttpTest.java index a8d51ed3dd..a2b1c83a9c 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterEndToEndHttpTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterEndToEndHttpTest.java @@ -166,6 +166,7 @@ public class ZipkinSpanExporterEndToEndHttpTest { .localEndpoint(localEndpoint) .addAnnotation(RECEIVED_TIMESTAMP_NANOS / 1000, "RECEIVED") .addAnnotation(SENT_TIMESTAMP_NANOS / 1000, "SENT") + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") .build(); } } diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java index cf2c958e63..428bf914ff 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java @@ -73,7 +73,10 @@ class ZipkinSpanExporterTest { SpanData data = buildStandardSpan().build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) - .isEqualTo(buildZipkinSpan(Span.Kind.SERVER)); + .isEqualTo( + standardZipkinSpanBuilder(Span.Kind.SERVER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build()); } @Test @@ -84,7 +87,11 @@ class ZipkinSpanExporterTest { .setEndEpochNanos(1505855794_194009999L) .build(); - Span expected = standardZipkinSpanBuilder(Span.Kind.SERVER).duration(1).build(); + Span expected = + standardZipkinSpanBuilder(Span.Kind.SERVER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .duration(1) + .build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)).isEqualTo(expected); } @@ -93,7 +100,10 @@ class ZipkinSpanExporterTest { SpanData data = buildStandardSpan().setKind(Kind.SERVER).build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) - .isEqualTo(buildZipkinSpan(Span.Kind.SERVER)); + .isEqualTo( + standardZipkinSpanBuilder(Span.Kind.SERVER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build()); } @Test @@ -101,7 +111,10 @@ class ZipkinSpanExporterTest { SpanData data = buildStandardSpan().setKind(Kind.CLIENT).build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) - .isEqualTo(buildZipkinSpan(Span.Kind.CLIENT)); + .isEqualTo( + standardZipkinSpanBuilder(Span.Kind.CLIENT) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build()); } @Test @@ -109,7 +122,10 @@ class ZipkinSpanExporterTest { SpanData data = buildStandardSpan().setKind(Kind.INTERNAL).build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) - .isEqualTo(buildZipkinSpan(null)); + .isEqualTo( + standardZipkinSpanBuilder(null) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build()); } @Test @@ -117,7 +133,10 @@ class ZipkinSpanExporterTest { SpanData data = buildStandardSpan().setKind(Kind.CONSUMER).build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) - .isEqualTo(buildZipkinSpan(Span.Kind.CONSUMER)); + .isEqualTo( + standardZipkinSpanBuilder(Span.Kind.CONSUMER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build()); } @Test @@ -125,7 +144,10 @@ class ZipkinSpanExporterTest { SpanData data = buildStandardSpan().setKind(Kind.PRODUCER).build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) - .isEqualTo(buildZipkinSpan(Span.Kind.PRODUCER)); + .isEqualTo( + standardZipkinSpanBuilder(Span.Kind.PRODUCER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build()); } @Test @@ -136,7 +158,10 @@ class ZipkinSpanExporterTest { Endpoint expectedEndpoint = Endpoint.newBuilder().serviceName("super-zipkin-service").build(); Span expectedZipkinSpan = - buildZipkinSpan(Span.Kind.SERVER).toBuilder().localEndpoint(expectedEndpoint).build(); + buildZipkinSpan(Span.Kind.SERVER).toBuilder() + .localEndpoint(expectedEndpoint) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build(); assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)).isEqualTo(expectedZipkinSpan); } @@ -166,6 +191,7 @@ class ZipkinSpanExporterTest { .putTag("stringArray", "Hello") .putTag("doubleArray", "32.33,-98.3") .putTag("longArray", "33,999") + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") .build()); } @@ -183,6 +209,7 @@ class ZipkinSpanExporterTest { buildZipkinSpan(Span.Kind.CLIENT).toBuilder() .putTag("otel.library.name", "io.opentelemetry.auto") .putTag("otel.library.version", "1.0.0") + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") .build()); } @@ -203,6 +230,7 @@ class ZipkinSpanExporterTest { buildZipkinSpan(Span.Kind.CLIENT).toBuilder() .clearTags() .putTag(SemanticAttributes.HTTP_STATUS_CODE.getKey(), "404") + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "ERROR") .putTag("error", "A user provided error") .build()); } @@ -229,13 +257,34 @@ class ZipkinSpanExporterTest { .build()); } + @Test + void generateSpan_WithRpcUnsetStatus() { + Attributes attributeMap = Attributes.of(SemanticAttributes.RPC_SERVICE, "my service name"); + + SpanData data = + buildStandardSpan() + .setStatus(SpanData.Status.create(StatusCode.UNSET, null)) + .setAttributes(attributeMap) + .build(); + + assertThat(ZipkinSpanExporter.generateSpan(data, localEndpoint)) + .isEqualTo( + buildZipkinSpan(Span.Kind.SERVER).toBuilder() + .putTag(SemanticAttributes.RPC_SERVICE.getKey(), "my service name") + .build()); + } + @Test void testExport() { ZipkinSpanExporter zipkinSpanExporter = new ZipkinSpanExporter(mockEncoder, mockSender, "tweetiebird"); byte[] someBytes = new byte[0]; - when(mockEncoder.encode(buildZipkinSpan(Span.Kind.SERVER))).thenReturn(someBytes); + when(mockEncoder.encode( + standardZipkinSpanBuilder(Span.Kind.SERVER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build())) + .thenReturn(someBytes); when(mockSender.sendSpans(Collections.singletonList(someBytes))).thenReturn(mockZipkinCall); doAnswer( invocation -> { @@ -258,7 +307,11 @@ class ZipkinSpanExporterTest { new ZipkinSpanExporter(mockEncoder, mockSender, "tweetiebird"); byte[] someBytes = new byte[0]; - when(mockEncoder.encode(buildZipkinSpan(Span.Kind.SERVER))).thenReturn(someBytes); + when(mockEncoder.encode( + standardZipkinSpanBuilder(Span.Kind.SERVER) + .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") + .build())) + .thenReturn(someBytes); when(mockSender.sendSpans(Collections.singletonList(someBytes))).thenReturn(mockZipkinCall); doAnswer( invocation -> {