From c9038f0ef02639d7563b10cd170a6f10776d9bc3 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 21 Jan 2021 12:52:29 -0800 Subject: [PATCH] Deprecate the service name configuration options for exporters (#2566) * deprecate the service name configurations on the jaeger and zipkin exporters * merge from upstream --- .../JaegerThriftSpanExporterBuilder.java | 12 ++++++++++-- .../thrift/JaegerThriftIntegrationTest.java | 1 - .../thrift/JaegerThriftSpanExporterTest.java | 6 +----- .../jaeger/JaegerGrpcSpanExporterBuilder.java | 6 +++++- .../jaeger/JaegerGrpcSpanExporterTest.java | 2 +- .../exporter/jaeger/JaegerIntegrationTest.java | 1 - .../exporter/zipkin/ZipkinSpanExporter.java | 1 - .../zipkin/ZipkinSpanExporterBuilder.java | 10 ++++++---- .../ZipkinSpanExporterEndToEndHttpTest.java | 18 ++++++------------ .../zipkin/ZipkinSpanExporterTest.java | 6 ++---- .../io/opentelemetry/SendTraceToJaeger.java | 1 - 11 files changed, 31 insertions(+), 33 deletions(-) diff --git a/exporters/jaeger-thrift/src/main/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterBuilder.java b/exporters/jaeger-thrift/src/main/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterBuilder.java index c0c6b84e4a..fb0f6065f3 100644 --- a/exporters/jaeger-thrift/src/main/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterBuilder.java +++ b/exporters/jaeger-thrift/src/main/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterBuilder.java @@ -7,11 +7,15 @@ package io.opentelemetry.exporter.jaeger.thrift; import io.jaegertracing.thrift.internal.senders.HttpSender; import io.jaegertracing.thrift.internal.senders.ThriftSender; +import io.opentelemetry.sdk.resources.Resource; +import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; /** Builder utility for this exporter. */ public final class JaegerThriftSpanExporterBuilder { - private String serviceName = JaegerThriftSpanExporter.DEFAULT_SERVICE_NAME; + private String serviceName = + Resource.getDefault().getAttributes().get(ResourceAttributes.SERVICE_NAME); + private String endpoint = JaegerThriftSpanExporter.DEFAULT_ENDPOINT; private ThriftSender thriftSender; @@ -28,11 +32,15 @@ public final class JaegerThriftSpanExporterBuilder { } /** - * Sets the service name to be used by this exporter. Required. + * Sets the service name to be used by this exporter, if none is found in the Resource associated + * with a span. * * @param serviceName the service name. * @return this. + * @deprecated The default service name is now extracted from the default Resource. This method + * will be removed in the next release. */ + @Deprecated public JaegerThriftSpanExporterBuilder setServiceName(String serviceName) { this.serviceName = serviceName; return this; diff --git a/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftIntegrationTest.java b/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftIntegrationTest.java index cfc796f358..5b40088c04 100644 --- a/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftIntegrationTest.java +++ b/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftIntegrationTest.java @@ -60,7 +60,6 @@ class JaegerThriftIntegrationTest { SpanExporter jaegerExporter = JaegerThriftSpanExporter.builder() - .setServiceName(SERVICE_NAME) .setEndpoint(JAEGER_URL + ":" + mappedPort + "/api/traces") .build(); return OpenTelemetrySdk.builder() diff --git a/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterTest.java b/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterTest.java index 5c419b97bd..2a04e376ea 100644 --- a/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterTest.java +++ b/exporters/jaeger-thrift/src/test/java/io/opentelemetry/exporter/jaeger/thrift/JaegerThriftSpanExporterTest.java @@ -50,11 +50,7 @@ class JaegerThriftSpanExporterTest { @BeforeEach void beforeEach() { - exporter = - JaegerThriftSpanExporter.builder() - .setThriftSender(thriftSender) - .setServiceName("myservice.name") - .build(); + exporter = JaegerThriftSpanExporter.builder().setThriftSender(thriftSender).build(); } @Test diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java index 576ebea96c..d17f6dd615 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java @@ -25,11 +25,15 @@ public final class JaegerGrpcSpanExporterBuilder { private long timeoutNanos = TimeUnit.SECONDS.toNanos(DEFAULT_TIMEOUT_SECS); /** - * Sets the service name to be used by this exporter. Required. + * Sets the service name to be used by this exporter, if none is found in the Resource associated + * with a span. * * @param serviceName the service name. * @return this. + * @deprecated The default service name is now extracted from the default Resource. This method + * will be removed in the next release. */ + @Deprecated public JaegerGrpcSpanExporterBuilder setServiceName(String serviceName) { this.serviceName = serviceName; return this; diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java index 9c6c0142fb..2bf7cf5c20 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java @@ -69,7 +69,7 @@ class JaegerGrpcSpanExporterTest { closer.register(server::shutdownNow); ManagedChannel channel = InProcessChannelBuilder.forName(serverName).directExecutor().build(); - exporter = JaegerGrpcSpanExporter.builder().setServiceName("test").setChannel(channel).build(); + exporter = JaegerGrpcSpanExporter.builder().setChannel(channel).build(); } @AfterEach diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerIntegrationTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerIntegrationTest.java index 7093202b64..f104e8d8aa 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerIntegrationTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerIntegrationTest.java @@ -63,7 +63,6 @@ class JaegerIntegrationTest { .build(); SpanExporter jaegerExporter = JaegerGrpcSpanExporter.builder() - .setServiceName(SERVICE_NAME) .setChannel(jaegerChannel) .setTimeout(Duration.ofSeconds(30)) .build(); 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 689ec658b8..96068db999 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 @@ -42,7 +42,6 @@ import zipkin2.reporter.Sender; */ public final class ZipkinSpanExporter implements SpanExporter { public static final String DEFAULT_ENDPOINT = "http://localhost:9411/api/v2/spans"; - public static final String DEFAULT_SERVICE_NAME = "unknown"; private static final Logger logger = Logger.getLogger(ZipkinSpanExporter.class.getName()); diff --git a/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java b/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java index b55215cf83..c7f431707d 100644 --- a/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java +++ b/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java @@ -17,12 +17,13 @@ import zipkin2.reporter.okhttp3.OkHttpSender; public final class ZipkinSpanExporterBuilder { private BytesEncoder encoder = SpanBytesEncoder.JSON_V2; private Sender sender; - private String serviceName = ZipkinSpanExporter.DEFAULT_SERVICE_NAME; + private String serviceName = + Resource.getDefault().getAttributes().get(ResourceAttributes.SERVICE_NAME); private String endpoint = ZipkinSpanExporter.DEFAULT_ENDPOINT; /** * Label of the remote node in the service graph, such as "favstar". Avoid names with variables or - * unique identifiers embedded. Defaults to "unknown". + * unique identifiers embedded. Defaults to the service.name specified in the default Resource. * *

This is a primary label for trace lookup and aggregation, so it should be intuitive and * consistent. Many use a name from service discovery. @@ -30,13 +31,14 @@ public final class ZipkinSpanExporterBuilder { *

Note: this value, will be superseded by the value of {@link ResourceAttributes#SERVICE_NAME} * if it has been set in the {@link Resource} associated with the Tracer that created the spans. * - *

This property is required to be set. - * * @param serviceName The service name. It defaults to "unknown". * @return this. * @see Resource * @see ResourceAttributes + * @deprecated The default service name is now extracted from the default Resource. This method + * will be removed in the next release. */ + @Deprecated public ZipkinSpanExporterBuilder setServiceName(String serviceName) { this.serviceName = serviceName; return this; 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 42d7c2f9e1..ad0d0be894 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 @@ -14,10 +14,12 @@ import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.TraceFlags; import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.sdk.common.CompletableResultCode; +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.SpanData; import io.opentelemetry.sdk.trace.data.StatusData; +import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -53,26 +55,19 @@ public class ZipkinSpanExporterEndToEndHttpTest { private static final String ENDPOINT_V1_SPANS = "/api/v1/spans"; private static final String ENDPOINT_V2_SPANS = "/api/v2/spans"; private static final String SERVICE_NAME = "myService"; - private static final Endpoint localEndpoint = - ZipkinSpanExporter.produceLocalEndpoint(SERVICE_NAME); @Rule public ZipkinRule zipkin = new ZipkinRule(); @Test public void testExportWithDefaultEncoding() { - ZipkinSpanExporter exporter = - ZipkinSpanExporter.builder() - .setEndpoint(zipkin.httpUrl() + ENDPOINT_V2_SPANS) - .setServiceName(SERVICE_NAME) - .build(); + ZipkinSpanExporter.builder().setEndpoint(zipkin.httpUrl() + ENDPOINT_V2_SPANS).build(); exportAndVerify(exporter); } @Test public void testExportAsProtobuf() { - ZipkinSpanExporter exporter = buildZipkinExporter( zipkin.httpUrl() + ENDPOINT_V2_SPANS, Encoding.PROTO3, SpanBytesEncoder.PROTO3); @@ -81,7 +76,6 @@ public class ZipkinSpanExporterEndToEndHttpTest { @Test public void testExportAsThrift() { - @SuppressWarnings("deprecation") // we have to use the deprecated thrift encoding to test it ZipkinSpanExporter exporter = buildZipkinExporter( @@ -116,7 +110,6 @@ public class ZipkinSpanExporterEndToEndHttpTest { String endpoint, Encoding encoding, SpanBytesEncoder encoder) { return ZipkinSpanExporter.builder() .setSender(OkHttpSender.newBuilder().endpoint(endpoint).encoding(encoding).build()) - .setServiceName(SERVICE_NAME) .setEncoder(encoder) .build(); } @@ -156,7 +149,8 @@ public class ZipkinSpanExporterEndToEndHttpTest { .setEvents(annotations) .setLinks(Collections.emptyList()) .setEndEpochNanos(END_EPOCH_NANOS) - .setHasEnded(true); + .setHasEnded(true) + .setResource(Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, SERVICE_NAME))); } private static Span buildZipkinSpan() { @@ -168,7 +162,7 @@ public class ZipkinSpanExporterEndToEndHttpTest { .name(SPAN_NAME) .timestamp(START_EPOCH_NANOS / 1000) .duration((END_EPOCH_NANOS / 1000) - (START_EPOCH_NANOS / 1000)) - .localEndpoint(localEndpoint) + .localEndpoint(Endpoint.newBuilder().serviceName(SERVICE_NAME).build()) .addAnnotation(RECEIVED_TIMESTAMP_NANOS / 1000, "RECEIVED") .addAnnotation(SENT_TIMESTAMP_NANOS / 1000, "SENT") .putTag(ZipkinSpanExporter.OTEL_STATUS_CODE, "OK") 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 85225c5c21..b288c595c5 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 @@ -348,16 +348,14 @@ class ZipkinSpanExporterTest { @Test void testCreate() { - ZipkinSpanExporter exporter = - ZipkinSpanExporter.builder().setSender(mockSender).setServiceName("myGreatService").build(); + ZipkinSpanExporter exporter = ZipkinSpanExporter.builder().setSender(mockSender).build(); assertThat(exporter).isNotNull(); } @Test void testShutdown() throws IOException { - ZipkinSpanExporter exporter = - ZipkinSpanExporter.builder().setServiceName("myGreatService").setSender(mockSender).build(); + ZipkinSpanExporter exporter = ZipkinSpanExporter.builder().setSender(mockSender).build(); exporter.shutdown(); verify(mockSender).close(); diff --git a/integration-tests/src/main/java/io/opentelemetry/SendTraceToJaeger.java b/integration-tests/src/main/java/io/opentelemetry/SendTraceToJaeger.java index d51f5b1a5f..19ff419f8e 100644 --- a/integration-tests/src/main/java/io/opentelemetry/SendTraceToJaeger.java +++ b/integration-tests/src/main/java/io/opentelemetry/SendTraceToJaeger.java @@ -35,7 +35,6 @@ public class SendTraceToJaeger { // Export traces to Jaeger JaegerGrpcSpanExporter jaegerExporter = JaegerGrpcSpanExporter.builder() - .setServiceName("integration test") .setChannel(jaegerChannel) .setTimeout(Duration.ofSeconds(30)) .build();