From 58bcf7fe88c125c36072e6bf103fdf1e80fd4e50 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 21 Jan 2021 14:39:32 +0900 Subject: [PATCH] Deprecate tracermanagement and SDK global access (#2547) * Deprecate SdkTracerManagement * Deprecate global SDK * Deprecate more * Fix * Cleanup * Fix * Handle test order random * Collector removed queued_retry --- .../opentracingshim/OpenTracingShimTest.java | 6 +++ .../perf/OtlpPipelineStressTest.java | 7 ++- .../resources/otel-collector-config-perf.yaml | 3 +- .../opentelemetry/sdk/OpenTelemetrySdk.java | 33 ++++++++++--- .../sdk/OpenTelemetrySdkTest.java | 22 ++------- .../sdk/testing/junit4/OpenTelemetryRule.java | 12 +++-- .../junit5/OpenTelemetryExtension.java | 12 +++-- .../sdk/trace/SdkTracerManagement.java | 3 ++ .../sdk/trace/SdkTracerProvider.java | 46 ++++++++++++++++++- .../sdk/trace/export/SpanExporter.java | 6 +-- 10 files changed, 108 insertions(+), 42 deletions(-) diff --git a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/OpenTracingShimTest.java b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/OpenTracingShimTest.java index 57f93fac14..7289f2d6b4 100644 --- a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/OpenTracingShimTest.java +++ b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/OpenTracingShimTest.java @@ -13,10 +13,16 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.sdk.trace.SdkTracerProvider; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; class OpenTracingShimTest { + @AfterEach + void tearDown() { + GlobalOpenTelemetry.resetForTest(); + } + @Test void createTracerShim_default() { TracerShim tracerShim = (TracerShim) OpenTracingShim.createTracerShim(); diff --git a/perf-harness/src/test/java/io/opentelemetry/perf/OtlpPipelineStressTest.java b/perf-harness/src/test/java/io/opentelemetry/perf/OtlpPipelineStressTest.java index 9e7046d916..31f2609d53 100644 --- a/perf-harness/src/test/java/io/opentelemetry/perf/OtlpPipelineStressTest.java +++ b/perf-harness/src/test/java/io/opentelemetry/perf/OtlpPipelineStressTest.java @@ -25,7 +25,6 @@ import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.export.IntervalMetricReader; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricExporter; -import io.opentelemetry.sdk.trace.SdkTracerManagement; import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; @@ -102,7 +101,7 @@ public class OtlpPipelineStressTest { private final InMemoryMetricExporter metricExporter = InMemoryMetricExporter.create(); - private SdkTracerManagement tracerManagement; + private SdkTracerProvider sdkTracerProvider; private OpenTelemetry openTelemetry; private IntervalMetricReader intervalMetricReader; private Proxy collectorProxy; @@ -131,7 +130,7 @@ public class OtlpPipelineStressTest { @AfterEach void tearDown() throws IOException { intervalMetricReader.shutdown(); - tracerManagement.shutdown(); + sdkTracerProvider.shutdown(); toxiproxyClient.reset(); collectorProxy.delete(); @@ -274,6 +273,6 @@ public class OtlpPipelineStressTest { SdkTracerProvider.builder().addSpanProcessor(spanProcessor).build(); openTelemetry = OpenTelemetrySdk.builder().setTracerProvider(tracerProvider).buildAndRegisterGlobal(); - tracerManagement = tracerProvider; + sdkTracerProvider = tracerProvider; } } diff --git a/perf-harness/src/test/resources/otel-collector-config-perf.yaml b/perf-harness/src/test/resources/otel-collector-config-perf.yaml index ac0263ffb0..0aa39a4d99 100644 --- a/perf-harness/src/test/resources/otel-collector-config-perf.yaml +++ b/perf-harness/src/test/resources/otel-collector-config-perf.yaml @@ -11,7 +11,6 @@ exporters: processors: batch: - queued_retry: extensions: health_check: @@ -25,7 +24,7 @@ service: pipelines: traces: receivers: [otlp] - processors: [batch, queued_retry] + processors: [batch] exporters: [logging] metrics: receivers: [otlp] diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/OpenTelemetrySdk.java b/sdk/all/src/main/java/io/opentelemetry/sdk/OpenTelemetrySdk.java index 75e1a534ac..0f4583444f 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/OpenTelemetrySdk.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/OpenTelemetrySdk.java @@ -10,12 +10,12 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.api.trace.TracerProvider; import io.opentelemetry.context.propagation.ContextPropagators; -import io.opentelemetry.sdk.trace.SdkTracerManagement; import io.opentelemetry.sdk.trace.SdkTracerProvider; import javax.annotation.concurrent.ThreadSafe; /** The SDK implementation of {@link OpenTelemetry}. */ @ThreadSafe +@SuppressWarnings("deprecation") // Remove when SdkTracerManagement is removed public final class OpenTelemetrySdk implements OpenTelemetry { private final ObfuscatedTracerProvider tracerProvider; private final ContextPropagators propagators; @@ -33,13 +33,23 @@ public final class OpenTelemetrySdk implements OpenTelemetry { return new OpenTelemetrySdkBuilder(); } - /** Returns the global {@link OpenTelemetrySdk}. */ + /** + * Returns the global {@link OpenTelemetrySdk}. + * + * @deprecated Will be removed without replacement + */ + @Deprecated public static OpenTelemetrySdk get() { return (OpenTelemetrySdk) GlobalOpenTelemetry.get(); } - /** Returns the global {@link SdkTracerManagement}. */ - public static SdkTracerManagement getGlobalTracerManagement() { + /** + * Returns the global {@link io.opentelemetry.sdk.trace.SdkTracerManagement}. + * + * @deprecated Will be removed without replacement + */ + @Deprecated + public static io.opentelemetry.sdk.trace.SdkTracerManagement getGlobalTracerManagement() { TracerProvider tracerProvider = GlobalOpenTelemetry.get().getTracerProvider(); if (!(tracerProvider instanceof ObfuscatedTracerProvider)) { throw new IllegalStateException( @@ -54,13 +64,24 @@ public final class OpenTelemetrySdk implements OpenTelemetry { return tracerProvider; } + /** Returns the {@link SdkTracerProvider} for this {@link OpenTelemetrySdk}. */ + public SdkTracerProvider getSdkTracerProvider() { + return tracerProvider.unobfuscate(); + } + @Override public ContextPropagators getPropagators() { return propagators; } - /** Returns the {@link SdkTracerManagement} for this {@link OpenTelemetrySdk}. */ - public SdkTracerManagement getTracerManagement() { + /** + * Returns the {@link io.opentelemetry.sdk.trace.SdkTracerManagement} for this {@link + * OpenTelemetrySdk}. + * + * @deprecated Use {@link #getSdkTracerProvider()} + */ + @Deprecated + public io.opentelemetry.sdk.trace.SdkTracerManagement getTracerManagement() { return tracerProvider.unobfuscate(); } diff --git a/sdk/all/src/test/java/io/opentelemetry/sdk/OpenTelemetrySdkTest.java b/sdk/all/src/test/java/io/opentelemetry/sdk/OpenTelemetrySdkTest.java index f2cb210b21..2bb40389ea 100644 --- a/sdk/all/src/test/java/io/opentelemetry/sdk/OpenTelemetrySdkTest.java +++ b/sdk/all/src/test/java/io/opentelemetry/sdk/OpenTelemetrySdkTest.java @@ -6,12 +6,9 @@ package io.opentelemetry.sdk; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.mockito.Mockito.mock; -import io.opentelemetry.api.DefaultOpenTelemetry; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.TracerProvider; @@ -48,21 +45,9 @@ class OpenTelemetrySdkTest { void testRegisterGlobal() { OpenTelemetrySdk sdk = OpenTelemetrySdk.builder().buildAndRegisterGlobal(); assertThat(sdk).isSameAs(GlobalOpenTelemetry.get()); - assertThat(OpenTelemetrySdk.get()).isSameAs(sdk); - assertThat(((SdkTracerProvider) OpenTelemetrySdk.getGlobalTracerManagement()).get("")) + assertThat(GlobalOpenTelemetry.get()).isSameAs(sdk); + assertThat(((OpenTelemetrySdk) GlobalOpenTelemetry.get()).getSdkTracerProvider().get("")) .isSameAs(GlobalOpenTelemetry.getTracerProvider().get("")); - assertThat(OpenTelemetrySdk.getGlobalTracerManagement()).isNotNull(); - } - - @Test - void testGetTracerManagementWhenNotTracerSdk() { - OpenTelemetrySdk.builder().buildAndRegisterGlobal(); - assertThatCode(OpenTelemetrySdk::getGlobalTracerManagement).doesNotThrowAnyException(); - GlobalOpenTelemetry.resetForTest(); - GlobalOpenTelemetry.set( - DefaultOpenTelemetry.builder().setTracerProvider(tracerProvider).build()); - assertThatThrownBy(OpenTelemetrySdk::getGlobalTracerManagement) - .isInstanceOf(IllegalStateException.class); } @Test @@ -93,6 +78,7 @@ class OpenTelemetrySdkTest { .build(); assertThat(((ObfuscatedTracerProvider) openTelemetry.getTracerProvider()).unobfuscate()) .isEqualTo(tracerProvider); + assertThat(openTelemetry.getSdkTracerProvider()).isEqualTo(tracerProvider); assertThat(openTelemetry.getPropagators()).isEqualTo(propagators); } @@ -136,7 +122,7 @@ class OpenTelemetrySdkTest { .asInstanceOf(type(ObfuscatedTracerProvider.class)) .isNotNull() .matches(obfuscated -> obfuscated.unobfuscate() == tracerProvider); - assertThat(openTelemetry.getTracerManagement()).isNotNull(); + assertThat(openTelemetry.getSdkTracerProvider()).isNotNull(); } // This is just a demonstration of maximum that one can do with OpenTelemetry configuration. diff --git a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit4/OpenTelemetryRule.java b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit4/OpenTelemetryRule.java index d76f759a9f..148ddb8bde 100644 --- a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit4/OpenTelemetryRule.java +++ b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit4/OpenTelemetryRule.java @@ -11,7 +11,6 @@ import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; -import io.opentelemetry.sdk.trace.SdkTracerManagement; import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; @@ -78,9 +77,14 @@ public final class OpenTelemetryRule extends ExternalResource { return openTelemetry; } - /** Returns the {@link SdkTracerManagement} created by this extension. */ - public SdkTracerManagement getTracerManagement() { - return openTelemetry.getTracerManagement(); + /** + * Returns the {@link io.opentelemetry.sdk.trace.SdkTracerManagement} created by this extension. + * + * @deprecated Will be removed without replacement + */ + @Deprecated + public io.opentelemetry.sdk.trace.SdkTracerManagement getTracerManagement() { + return openTelemetry.getSdkTracerProvider(); } /** Returns all the exported {@link SpanData} so far. */ diff --git a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtension.java b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtension.java index d4af1c101b..af7cf02585 100644 --- a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtension.java +++ b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtension.java @@ -14,7 +14,6 @@ import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.testing.assertj.TracesAssert; import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; -import io.opentelemetry.sdk.trace.SdkTracerManagement; import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; @@ -85,9 +84,14 @@ public final class OpenTelemetryExtension return openTelemetry; } - /** Returns the {@link SdkTracerManagement} created by this extension. */ - public SdkTracerManagement getTracerManagement() { - return openTelemetry.getTracerManagement(); + /** + * Returns the {@link io.opentelemetry.sdk.trace.SdkTracerManagement} created by this extension. + * + * @deprecated Will be removed without replacement + */ + @Deprecated + public io.opentelemetry.sdk.trace.SdkTracerManagement getTracerManagement() { + return openTelemetry.getSdkTracerProvider(); } /** Returns all the exported {@link SpanData} so far. */ diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerManagement.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerManagement.java index bb4f3f9e35..27e6086c39 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerManagement.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerManagement.java @@ -14,7 +14,10 @@ import java.util.concurrent.TimeUnit; /** * "Management" interface for the Tracing SDK. This interface exposes methods for configuring the * Tracing SDK, as well as several lifecycle methods. + * + * @deprecated Use {@link SdkTracerProvider} directly. */ +@Deprecated public interface SdkTracerManagement extends Closeable { /** diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java index 59c28f2787..b9ab0def64 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java @@ -13,7 +13,9 @@ import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.internal.ComponentRegistry; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.config.TraceConfig; +import java.io.Closeable; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -26,7 +28,8 @@ import javax.annotation.Nullable; * OpenTelemetry}. However, if you need a custom implementation of the factory, you can create one * as needed. */ -public final class SdkTracerProvider implements TracerProvider, SdkTracerManagement { +@SuppressWarnings("deprecation") // Remove when SdkTracerManagement is removed +public final class SdkTracerProvider implements TracerProvider, SdkTracerManagement, Closeable { private static final Logger logger = Logger.getLogger(SdkTracerProvider.class.getName()); static final String DEFAULT_TRACER_NAME = "unknown"; private final TracerSharedState sharedState; @@ -69,11 +72,28 @@ public final class SdkTracerProvider implements TracerProvider, SdkTracerManagem return tracerSdkComponentRegistry.get(instrumentationName, instrumentationVersion); } + /** Returns the active {@link TraceConfig}. */ @Override public TraceConfig getActiveTraceConfig() { return sharedState.getActiveTraceConfig(); } + /** + * Attempts to stop all the activity for this {@link Tracer}. Calls {@link + * SpanProcessor#shutdown()} for all registered {@link SpanProcessor}s. + * + *

The returned {@link CompletableResultCode} will be completed when all the Spans are + * processed. + * + *

After this is called, newly created {@code Span}s will be no-ops. + * + *

After this is called, further attempts at re-using or reconfiguring this instance will + * result in undefined behavior. It should be considered a terminal operation for the SDK + * implementation. + * + * @return a {@link CompletableResultCode} which is completed when all the span processors have + * been shut down. + */ @Override public CompletableResultCode shutdown() { if (sharedState.isStopped()) { @@ -83,8 +103,32 @@ public final class SdkTracerProvider implements TracerProvider, SdkTracerManagem return sharedState.shutdown(); } + /** + * Requests the active span processor to process all span events that have not yet been processed + * and returns a {@link CompletableResultCode} which is completed when the flush is finished. + * + * @see SpanProcessor#forceFlush() + */ @Override public CompletableResultCode forceFlush() { return sharedState.getActiveSpanProcessor().forceFlush(); } + + /** + * Attempts to stop all the activity for this {@link Tracer}. Calls {@link + * SpanProcessor#shutdown()} for all registered {@link SpanProcessor}s. + * + *

This operation may block until all the Spans are processed. Must be called before turning + * off the main application to ensure all data are processed and exported. + * + *

After this is called, newly created {@code Span}s will be no-ops. + * + *

After this is called, further attempts at re-using or reconfiguring this instance will + * result in undefined behavior. It should be considered a terminal operation for the SDK + * implementation. + */ + @Override + public void close() { + shutdown().join(10, TimeUnit.SECONDS); + } } diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/SpanExporter.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/SpanExporter.java index f283d45430..824700a210 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/SpanExporter.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/SpanExporter.java @@ -6,7 +6,7 @@ package io.opentelemetry.sdk.trace.export; import io.opentelemetry.sdk.common.CompletableResultCode; -import io.opentelemetry.sdk.trace.SdkTracerManagement; +import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.data.SpanData; import java.io.Closeable; import java.util.ArrayList; @@ -77,8 +77,8 @@ public interface SpanExporter extends Closeable { CompletableResultCode flush(); /** - * Called when {@link SdkTracerManagement#shutdown()} is called, if this {@code SpanExporter} is - * registered to a {@code TracerSdkManagement} object. + * Called when {@link SdkTracerProvider#shutdown()} is called, if this {@code SpanExporter} is + * registered to a {@link SdkTracerProvider} object. * * @return a {@link CompletableResultCode} which is completed when shutdown completes. */