From eedaf22853d2ccbd7f59feb0c66798ed440d3b68 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 23 Aug 2023 23:57:53 +0530 Subject: [PATCH 01/16] Add SpanBuilder support Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/tracing/Context.java | 45 +++++ .../telemetry/tracing/DefaultTracer.java | 5 + .../opensearch/telemetry/tracing/Tracer.java | 8 + .../telemetry/tracing/noop/NoopTracer.java | 6 + .../telemetry/tracing/DefaultTracerTests.java | 32 +--- .../telemetry/tracing/SpanBuilder.java | 117 +++++++++++++ .../telemetry/tracing/WrappedTracer.java | 5 + .../telemetry/tracing/SpanBuilderTests.java | 158 ++++++++++++++++++ 8 files changed, 351 insertions(+), 25 deletions(-) create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Context.java create mode 100644 server/src/main/java/org/opensearch/telemetry/tracing/SpanBuilder.java create mode 100644 server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Context.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Context.java new file mode 100644 index 0000000000000..31518f32f2b6b --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Context.java @@ -0,0 +1,45 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import org.opensearch.telemetry.tracing.attributes.Attributes; + +/** + * Context for span details. + */ +public final class Context { + private final String spanName; + private final Attributes attributes; + + /** + * Constructor. + * @param spanName span name. + * @param attributes attributes. + */ + public Context(String spanName, Attributes attributes) { + this.spanName = spanName; + this.attributes = attributes; + } + + /** + * Returns the span name. + * @return span name + */ + public String getSpanName() { + return spanName; + } + + /** + * Returns the span attributes. + * @return attributes. + */ + public Attributes getAttributes() { + return attributes; + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index bc1a08e2d3c72..863aaf9b89f0d 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -40,6 +40,11 @@ public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage> requestHeaders = new HashMap<>(); - requestHeaders.put("traceparent", Arrays.asList(traceId + "~" + spanId)); - - SpanScope spanScope = defaultTracer.startSpan("test_span", requestHeaders, Attributes.EMPTY); - SpanContext currentSpan = defaultTracer.getCurrentSpan(); - assertNotNull(currentSpan); - assertEquals(traceId, currentSpan.getSpan().getTraceId()); - assertEquals(traceId, currentSpan.getSpan().getParentSpan().getTraceId()); - assertEquals(spanId, currentSpan.getSpan().getParentSpan().getSpanId()); - spanScope.close(); + public void testCreateSpanWithContext() { + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + Attributes attributes = Attributes.create().addAttribute("name", "value"); + when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); + defaultTracer.startSpan(new Context("span_name", attributes)); + verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); } public void testCreateSpanWithNullParent() { @@ -137,7 +119,7 @@ public void testCreateSpanWithNullParent() { new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) ); - defaultTracer.startSpan("span_name"); + defaultTracer.startSpan("span_name", null, Attributes.EMPTY); Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); Assert.assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/SpanBuilder.java b/server/src/main/java/org/opensearch/telemetry/tracing/SpanBuilder.java new file mode 100644 index 0000000000000..c1bdbf0780b0e --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/tracing/SpanBuilder.java @@ -0,0 +1,117 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import org.opensearch.core.common.Strings; +import org.opensearch.http.HttpRequest; +import org.opensearch.rest.RestRequest; +import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.transport.Transport; + +import java.util.Arrays; +import java.util.List; + +/** + * Utility class, helps in creating the {@link Context} for span. + */ +public class SpanBuilder { + + private static final List HEADERS_TO_BE_ADDED_AS_ATTRIBUTES = Arrays.asList(AttributeNames.TRACE); + /** + * Attribute name Separator + */ + public static final String SEPARATOR = " "; + + /** + * Creates {@link Context} from the {@link HttpRequest} + * @param request Http request. + * @return context. + */ + public static Context from(HttpRequest request) { + return new Context(createSpanName(request), buildSpanAttributes(request)); + } + + /** + * Creates {@link Context} from the {@link RestRequest} + * @param request Rest request + * @return context + */ + public static Context from(RestRequest request) { + return new Context(createSpanName(request), buildSpanAttributes(request)); + } + + /** + * Creates {@link Context} from Transport action and connection details. + * @param action action. + * @param connection transport connection. + * @return context + */ + public static Context from(String action, Transport.Connection connection) { + return new Context(createSpanName(action, connection), buildSpanAttributes(action, connection)); + } + + private static String createSpanName(HttpRequest httpRequest) { + return httpRequest.method().name() + SEPARATOR + httpRequest.uri(); + } + + private static Attributes buildSpanAttributes(HttpRequest httpRequest) { + Attributes attributes = Attributes.create() + .addAttribute(AttributeNames.HTTP_URI, httpRequest.uri()) + .addAttribute(AttributeNames.HTTP_METHOD, httpRequest.method().name()) + .addAttribute(AttributeNames.HTTP_PROTOCOL_VERSION, httpRequest.protocolVersion().name()); + populateHeader(httpRequest, attributes); + return attributes; + } + + private static void populateHeader(HttpRequest httpRequest, Attributes attributes) { + HEADERS_TO_BE_ADDED_AS_ATTRIBUTES.forEach(x -> { + if (httpRequest.getHeaders() != null && httpRequest.getHeaders().get(x) != null) { + attributes.addAttribute(x, Strings.collectionToCommaDelimitedString(httpRequest.getHeaders().get(x))); + } + }); + } + + private static String createSpanName(RestRequest restRequest) { + String spanName = "rest_request"; + if (restRequest != null) { + try { + String methodName = restRequest.method().name(); + // path() does the decoding, which may give error + String path = restRequest.path(); + spanName = methodName + SEPARATOR + path; + } catch (Exception e) { + // swallow the exception and keep the default name. + } + } + return spanName; + } + + private static Attributes buildSpanAttributes(RestRequest restRequest) { + if (restRequest != null) { + return Attributes.create() + .addAttribute(AttributeNames.REST_REQ_ID, restRequest.getRequestId()) + .addAttribute(AttributeNames.REST_REQ_RAW_PATH, restRequest.rawPath()); + } else { + return Attributes.EMPTY; + } + } + + private static String createSpanName(String action, Transport.Connection connection) { + return action + SEPARATOR + (connection.getNode() != null ? connection.getNode().getHostAddress() : null); + } + + private static Attributes buildSpanAttributes(String action, Transport.Connection connection) { + Attributes attributes = Attributes.create().addAttribute(AttributeNames.TRANSPORT_ACTION, action); + if (connection != null && connection.getNode() != null) { + attributes.addAttribute(AttributeNames.TRANSPORT_TARGET_HOST, connection.getNode().getHostAddress()); + } + return attributes; + } + +} diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java index b699471be7f4c..d566aa01f9b6e 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java @@ -37,6 +37,11 @@ public WrappedTracer(TelemetrySettings telemetrySettings, Tracer defaultTracer) this.telemetrySettings = telemetrySettings; } + @Override + public SpanScope startSpan(Context context) { + return startSpan(context.getSpanName(), context.getAttributes()); + } + @Override public SpanScope startSpan(String spanName) { return startSpan(spanName, Attributes.EMPTY); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java new file mode 100644 index 0000000000000..966793ee8d233 --- /dev/null +++ b/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java @@ -0,0 +1,158 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import org.opensearch.Version; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.network.NetworkAddress; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.common.transport.TransportAddress; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.http.HttpRequest; +import org.opensearch.http.HttpResponse; +import org.opensearch.rest.RestRequest; +import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.transport.Transport; +import org.opensearch.transport.TransportException; +import org.opensearch.transport.TransportRequest; +import org.opensearch.transport.TransportRequestOptions; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +public class SpanBuilderTests extends OpenSearchTestCase { + + public void testHttpRequestContext() { + HttpRequest httpRequest = createHttpRequest(); + Context context = SpanBuilder.from(httpRequest); + Attributes attributes = context.getAttributes(); + assertEquals("GET /_test", context.getSpanName()); + assertEquals("true", attributes.getAttributesMap().get(AttributeNames.TRACE)); + assertEquals("GET", attributes.getAttributesMap().get(AttributeNames.HTTP_METHOD)); + assertEquals("HTTP_1_0", attributes.getAttributesMap().get(AttributeNames.HTTP_PROTOCOL_VERSION)); + assertEquals("/_test", attributes.getAttributesMap().get(AttributeNames.HTTP_URI)); + } + + public void testRestRequestContext() { + RestRequest restRequest = RestRequest.request(null, createHttpRequest(), null); + Context context = SpanBuilder.from(restRequest); + Attributes attributes = context.getAttributes(); + assertEquals("GET /_test", context.getSpanName()); + assertEquals("/_test", attributes.getAttributesMap().get(AttributeNames.REST_REQ_RAW_PATH)); + assertNotNull(attributes.getAttributesMap().get(AttributeNames.REST_REQ_ID)); + } + + public void testRestRequestContextForNull() { + Context context = SpanBuilder.from((RestRequest) null); + assertEquals("rest_request", context.getSpanName()); + assertEquals(Attributes.EMPTY, context.getAttributes()); + } + + public void testTransportContext() { + String action = "test-action"; + Transport.Connection connection = createTransportConnection(); + Context context = SpanBuilder.from(action, connection); + Attributes attributes = context.getAttributes(); + assertEquals(action + SpanBuilder.SEPARATOR + NetworkAddress.format(TransportAddress.META_ADDRESS), context.getSpanName()); + assertEquals(connection.getNode().getHostAddress(), attributes.getAttributesMap().get(AttributeNames.TRANSPORT_TARGET_HOST)); + } + + private static Transport.Connection createTransportConnection() { + return new Transport.Connection() { + @Override + public DiscoveryNode getNode() { + return new DiscoveryNode("local", new TransportAddress(TransportAddress.META_ADDRESS, 9200), Version.V_2_0_0); + } + + @Override + public void sendRequest(long requestId, String action, TransportRequest request, TransportRequestOptions options) + throws IOException, TransportException { + + } + + @Override + public void addCloseListener(ActionListener listener) { + + } + + @Override + public boolean isClosed() { + return false; + } + + @Override + public void close() { + + } + }; + } + + private static HttpRequest createHttpRequest() { + return new HttpRequest() { + @Override + public RestRequest.Method method() { + return RestRequest.Method.GET; + } + + @Override + public String uri() { + return "/_test"; + } + + @Override + public BytesReference content() { + return null; + } + + @Override + public Map> getHeaders() { + return Map.of("trace", Arrays.asList("true")); + } + + @Override + public List strictCookies() { + return null; + } + + @Override + public HttpVersion protocolVersion() { + return HttpVersion.HTTP_1_0; + } + + @Override + public HttpRequest removeHeader(String header) { + return null; + } + + @Override + public HttpResponse createResponse(RestStatus status, BytesReference content) { + return null; + } + + @Override + public Exception getInboundException() { + return null; + } + + @Override + public void release() { + + } + + @Override + public HttpRequest releaseAndCopy() { + return null; + } + }; + } +} From ec85630759773b4fe1b44d29dfb7f82dee1d7e3b Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 24 Aug 2023 22:36:31 +0530 Subject: [PATCH 02/16] Refactor code Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultTracer.java | 2 +- ...{Context.java => SpanCreationContext.java} | 4 +- .../opensearch/telemetry/tracing/Tracer.java | 4 +- .../telemetry/tracing/noop/NoopTracer.java | 4 +- .../telemetry/tracing/DefaultTracerTests.java | 2 +- .../telemetry/tracing/AttributeNames.java | 62 +++++++++++++++++++ .../telemetry/tracing/SpanBuilder.java | 31 ++++++---- .../telemetry/tracing/WrappedTracer.java | 2 +- .../routing/ShardMovementStrategyTests.java | 4 ++ .../telemetry/tracing/SpanBuilderTests.java | 10 +-- 10 files changed, 99 insertions(+), 26 deletions(-) rename libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/{Context.java => SpanCreationContext.java} (88%) create mode 100644 server/src/main/java/org/opensearch/telemetry/tracing/AttributeNames.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 863aaf9b89f0d..7854c3c3dd3e1 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -41,7 +41,7 @@ public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage HEADERS_TO_BE_ADDED_AS_ATTRIBUTES = Arrays.asList(AttributeNames.TRACE); /** * Attribute name Separator */ - public static final String SEPARATOR = " "; + private static final String SEPARATOR = " "; /** - * Creates {@link Context} from the {@link HttpRequest} + * Constructor + */ + private SpanBuilder() { + + } + + /** + * Creates {@link SpanCreationContext} from the {@link HttpRequest} * @param request Http request. * @return context. */ - public static Context from(HttpRequest request) { - return new Context(createSpanName(request), buildSpanAttributes(request)); + public static SpanCreationContext from(HttpRequest request) { + return new SpanCreationContext(createSpanName(request), buildSpanAttributes(request)); } /** - * Creates {@link Context} from the {@link RestRequest} + * Creates {@link SpanCreationContext} from the {@link RestRequest} * @param request Rest request * @return context */ - public static Context from(RestRequest request) { - return new Context(createSpanName(request), buildSpanAttributes(request)); + public static SpanCreationContext from(RestRequest request) { + return new SpanCreationContext(createSpanName(request), buildSpanAttributes(request)); } /** - * Creates {@link Context} from Transport action and connection details. + * Creates {@link SpanCreationContext} from Transport action and connection details. * @param action action. * @param connection transport connection. * @return context */ - public static Context from(String action, Transport.Connection connection) { - return new Context(createSpanName(action, connection), buildSpanAttributes(action, connection)); + public static SpanCreationContext from(String action, Transport.Connection connection) { + return new SpanCreationContext(createSpanName(action, connection), buildSpanAttributes(action, connection)); } private static String createSpanName(HttpRequest httpRequest) { diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java index d566aa01f9b6e..53a6b363fce50 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java @@ -38,7 +38,7 @@ public WrappedTracer(TelemetrySettings telemetrySettings, Tracer defaultTracer) } @Override - public SpanScope startSpan(Context context) { + public SpanScope startSpan(SpanCreationContext context) { return startSpan(context.getSpanName(), context.getAttributes()); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java b/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java index 7483e69fb0b0e..ebd4c9da1f5c2 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java @@ -25,6 +25,10 @@ @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class ShardMovementStrategyTests extends OpenSearchIntegTestCase { + protected boolean addMockTelemetryPlugin() { + return false; + } + protected String startDataOnlyNode(final String zone) { final Settings settings = Settings.builder().put("node.attr.zone", zone).build(); return internalCluster().startDataOnlyNode(settings); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java index 966793ee8d233..b4183412cdf02 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java @@ -34,7 +34,7 @@ public class SpanBuilderTests extends OpenSearchTestCase { public void testHttpRequestContext() { HttpRequest httpRequest = createHttpRequest(); - Context context = SpanBuilder.from(httpRequest); + SpanCreationContext context = SpanBuilder.from(httpRequest); Attributes attributes = context.getAttributes(); assertEquals("GET /_test", context.getSpanName()); assertEquals("true", attributes.getAttributesMap().get(AttributeNames.TRACE)); @@ -45,7 +45,7 @@ public void testHttpRequestContext() { public void testRestRequestContext() { RestRequest restRequest = RestRequest.request(null, createHttpRequest(), null); - Context context = SpanBuilder.from(restRequest); + SpanCreationContext context = SpanBuilder.from(restRequest); Attributes attributes = context.getAttributes(); assertEquals("GET /_test", context.getSpanName()); assertEquals("/_test", attributes.getAttributesMap().get(AttributeNames.REST_REQ_RAW_PATH)); @@ -53,7 +53,7 @@ public void testRestRequestContext() { } public void testRestRequestContextForNull() { - Context context = SpanBuilder.from((RestRequest) null); + SpanCreationContext context = SpanBuilder.from((RestRequest) null); assertEquals("rest_request", context.getSpanName()); assertEquals(Attributes.EMPTY, context.getAttributes()); } @@ -61,9 +61,9 @@ public void testRestRequestContextForNull() { public void testTransportContext() { String action = "test-action"; Transport.Connection connection = createTransportConnection(); - Context context = SpanBuilder.from(action, connection); + SpanCreationContext context = SpanBuilder.from(action, connection); Attributes attributes = context.getAttributes(); - assertEquals(action + SpanBuilder.SEPARATOR + NetworkAddress.format(TransportAddress.META_ADDRESS), context.getSpanName()); + assertEquals(action + " " + NetworkAddress.format(TransportAddress.META_ADDRESS), context.getSpanName()); assertEquals(connection.getNode().getHostAddress(), attributes.getAttributesMap().get(AttributeNames.TRANSPORT_TARGET_HOST)); } From d952d167cc484b1570a77cf48ed868385532ad82 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 29 Aug 2023 19:27:13 +0530 Subject: [PATCH 03/16] Redefine telemetry context restoration Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultScopedSpan.java | 92 ++++++++++ .../telemetry/tracing/DefaultSpanScope.java | 62 ++----- .../telemetry/tracing/DefaultTracer.java | 83 +++++++-- .../telemetry/tracing/ScopedSpan.java | 74 ++++++++ .../telemetry/tracing/SpanScope.java | 55 +----- .../telemetry/tracing/SpanScopeReference.java | 43 +++++ .../opensearch/telemetry/tracing/Tracer.java | 49 +++-- .../tracing/TracerContextStorage.java | 5 + .../telemetry/tracing/http/HttpTracer.java | 5 +- .../tracing/noop/NoopScopedSpan.java | 59 ++++++ .../telemetry/tracing/noop/NoopSpan.java | 81 +++++++++ .../telemetry/tracing/noop/NoopSpanScope.java | 41 +---- .../telemetry/tracing/noop/NoopTracer.java | 39 ++-- .../tracing/runnable/TraceableRunnable.java | 5 +- ...Tests.java => DefaultScopedSpanTests.java} | 29 +-- .../telemetry/tracing/DefaultTracerTests.java | 172 +++++++++++++++--- .../tracing/TraceableRunnableTests.java | 34 ++-- .../ThreadContextSpanScopeStorage.java | 53 ++++++ ...age.java => ThreadContextSpanStorage.java} | 7 +- .../telemetry/tracing/TracerFactory.java | 10 +- .../telemetry/tracing/WrappedTracer.java | 25 ++- .../telemetry/tracing/TracerFactoryTests.java | 20 +- 22 files changed, 806 insertions(+), 237 deletions(-) create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpan.java rename libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/{DefaultSpanScopeTests.java => DefaultScopedSpanTests.java} (55%) create mode 100644 server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java rename server/src/main/java/org/opensearch/telemetry/tracing/{ThreadContextBasedTracerContextStorage.java => ThreadContextSpanStorage.java} (90%) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java new file mode 100644 index 0000000000000..b68b0e7f848b5 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java @@ -0,0 +1,92 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import java.util.Objects; +import java.util.function.BiConsumer; + +/** + * Default implementation of Scope + * + * @opensearch.internal + */ +final class DefaultScopedSpan implements ScopedSpan { + + private final Span span; + + private final SpanScope spanScope; + + private final BiConsumer onCloseConsumer; + + /** + * Creates Scope instance for the given span + * + * @param span underlying span + * @param onCloseConsumer consumer to execute on scope close + */ + public DefaultScopedSpan(Span span, SpanScope spanScope, BiConsumer onCloseConsumer) { + this.span = Objects.requireNonNull(span); + this.spanScope = Objects.requireNonNull(spanScope); + this.onCloseConsumer = Objects.requireNonNull(onCloseConsumer); + } + + @Override + public void addSpanAttribute(String key, String value) { + span.addAttribute(key, value); + } + + @Override + public void addSpanAttribute(String key, long value) { + span.addAttribute(key, value); + } + + @Override + public void addSpanAttribute(String key, double value) { + span.addAttribute(key, value); + } + + @Override + public void addSpanAttribute(String key, boolean value) { + span.addAttribute(key, value); + } + + @Override + public void addSpanEvent(String event) { + span.addEvent(event); + } + + @Override + public void setError(Exception exception) { + span.setError(exception); + } + + /** + * Executes the runnable to end the scope + */ + @Override + public void close() { + onCloseConsumer.accept(span, spanScope); + } + + /** + * Returns span. + * @return + */ + Span getSpan() { + return span; + } + + /** + * Returns {@link SpanScope} + * @return spanScope + */ + SpanScope getSpanScope() { + return spanScope; + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java index 356b72187de74..801819786d13f 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java @@ -8,65 +8,37 @@ package org.opensearch.telemetry.tracing; +import java.util.Objects; import java.util.function.Consumer; /** - * Default implementation of Scope - * - * @opensearch.internal + * Default implementation for {@link SpanScope} */ -final class DefaultSpanScope implements SpanScope { - +public class DefaultSpanScope implements SpanScope { private final Span span; - - private final Consumer onCloseConsumer; + private final SpanScope beforeAttachedSpanScope; + private final Consumer onCloseConsumer; /** - * Creates Scope instance for the given span - * - * @param span underlying span - * @param onCloseConsumer consumer to execute on scope close + * Constructor + * @param span span + * @param beforeAttachedSpanScope before attached span scope. + * @param onCloseConsumer close consumer */ - public DefaultSpanScope(Span span, Consumer onCloseConsumer) { - this.span = span; - this.onCloseConsumer = onCloseConsumer; - } - - @Override - public void addSpanAttribute(String key, String value) { - span.addAttribute(key, value); + public DefaultSpanScope(Span span, SpanScope beforeAttachedSpanScope, Consumer onCloseConsumer) { + this.span = Objects.requireNonNull(span); + this.beforeAttachedSpanScope = beforeAttachedSpanScope; + this.onCloseConsumer = Objects.requireNonNull(onCloseConsumer); } @Override - public void addSpanAttribute(String key, long value) { - span.addAttribute(key, value); - } - - @Override - public void addSpanAttribute(String key, double value) { - span.addAttribute(key, value); - } - - @Override - public void addSpanAttribute(String key, boolean value) { - span.addAttribute(key, value); - } - - @Override - public void addSpanEvent(String event) { - span.addEvent(event); + public void close() { + onCloseConsumer.accept(beforeAttachedSpanScope); } @Override - public void setError(Exception exception) { - span.setError(exception); + public Span getSpan() { + return span; } - /** - * Executes the runnable to end the scope - */ - @Override - public void close() { - onCloseConsumer.accept(span); - } } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 7854c3c3dd3e1..26967520028fe 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -27,36 +27,42 @@ class DefaultTracer implements Tracer { static final String THREAD_NAME = "th_name"; private final TracingTelemetry tracingTelemetry; - private final TracerContextStorage tracerContextStorage; + private final TracerContextStorage spanTracerContextStorage; + private final TracerContextStorage spanScopeTracerContextStorage; /** * Creates DefaultTracer instance * * @param tracingTelemetry tracing telemetry instance - * @param tracerContextStorage storage used for storing current span context + * @param spanTracerContextStorage storage used for storing current span context */ - public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage tracerContextStorage) { + public DefaultTracer( + TracingTelemetry tracingTelemetry, + TracerContextStorage spanTracerContextStorage, + TracerContextStorage spanScopeTracerContextStorage + ) { this.tracingTelemetry = tracingTelemetry; - this.tracerContextStorage = tracerContextStorage; + this.spanTracerContextStorage = spanTracerContextStorage; + this.spanScopeTracerContextStorage = spanScopeTracerContextStorage; } @Override - public SpanScope startSpan(SpanCreationContext context) { + public Span startSpan(SpanCreationContext context) { return startSpan(context.getSpanName(), context.getAttributes()); } @Override - public SpanScope startSpan(String spanName) { + public Span startSpan(String spanName) { return startSpan(spanName, Attributes.EMPTY); } @Override - public SpanScope startSpan(String spanName, Attributes attributes) { + public Span startSpan(String spanName, Attributes attributes) { return startSpan(spanName, (SpanContext) null, attributes); } @Override - public SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { + public Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { Span span = null; if (parentSpan != null) { span = createSpan(spanName, parentSpan.getSpan(), attributes); @@ -65,7 +71,7 @@ public SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes a } setCurrentSpanInContext(span); addDefaultAttributes(span); - return new DefaultSpanScope(span, (scopeSpan) -> endSpan(scopeSpan)); + return span; } @Override @@ -74,14 +80,65 @@ public void close() throws IOException { } private Span getCurrentSpanInternal() { - return tracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); + return spanTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); } public SpanContext getCurrentSpan() { - final Span currentSpan = tracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); + final Span currentSpan = spanTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); return (currentSpan == null) ? null : new SpanContext(currentSpan); } + private SpanScope getCurrentSpanScope() { + return spanScopeTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN_SCOPE); + } + + @Override + public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) { + Span span = startSpan(spanCreationContext); + SpanScope spanScope = createSpanScope(span); + return new DefaultScopedSpan( + span, + spanScope, + (spanToBeClosed, scopeSpanToBeClosed) -> endScopedSpan(spanToBeClosed, scopeSpanToBeClosed) + ); + } + + @Override + public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan) { + Span span = startSpan(spanCreationContext.getSpanName(), parentSpan, spanCreationContext.getAttributes()); + SpanScope spanScope = createSpanScope(span); + return new DefaultScopedSpan( + span, + spanScope, + (spanToBeClosed, scopeSpanToBeClosed) -> endScopedSpan(spanToBeClosed, scopeSpanToBeClosed) + ); + } + + @Override + public SpanScope createSpanScope(Span span) { + SpanScope defaultSpanScope = new DefaultSpanScope(span, getCurrentSpanScope(), (spanScope) -> closeSpanScope(spanScope)); + setCurrentSpanScopeInContext(defaultSpanScope); + return defaultSpanScope; + } + + private void closeSpanScope(SpanScope beforeAttachedSpanScope) { + setCurrentSpanScopeInContext(beforeAttachedSpanScope); + if (beforeAttachedSpanScope != null) { + setCurrentSpanInContext(beforeAttachedSpanScope.getSpan()); + } else { + setCurrentSpanInContext(null); + } + } + + private void setCurrentSpanScopeInContext(SpanScope spanScope) { + spanScopeTracerContextStorage.put(TracerContextStorage.CURRENT_SPAN_SCOPE, spanScope); + } + + private void endScopedSpan(Span span, SpanScope spanScope) { + endSpan(span); + spanScope.close(); + } + private void endSpan(Span span) { if (span != null) { span.endSpan(); @@ -94,7 +151,7 @@ private Span createSpan(String spanName, Span parentSpan, Attributes attributes) } private void setCurrentSpanInContext(Span span) { - tracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, span); + spanTracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, span); } /** @@ -106,7 +163,7 @@ protected void addDefaultAttributes(Span span) { } @Override - public SpanScope startSpan(String spanName, Map> headers, Attributes attributes) { + public Span startSpan(String spanName, Map> headers, Attributes attributes) { Optional propagatedSpan = tracingTelemetry.getContextPropagator().extractFromHeaders(headers); return startSpan(spanName, propagatedSpan.map(SpanContext::new).orElse(null), attributes); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java new file mode 100644 index 0000000000000..0100960e0e793 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java @@ -0,0 +1,74 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import org.opensearch.telemetry.tracing.noop.NoopScopedSpan; + +/** + * An auto-closeable that represents scoped span. + * It provides interface for all the span operations. + */ +public interface ScopedSpan extends AutoCloseable { + /** + * No-op Scope implementation + */ + ScopedSpan NO_OP = new NoopScopedSpan(); + + /** + * Adds string attribute to the {@link Span}. + * + * @param key attribute key + * @param value attribute value + */ + void addSpanAttribute(String key, String value); + + /** + * Adds long attribute to the {@link Span}. + * + * @param key attribute key + * @param value attribute value + */ + void addSpanAttribute(String key, long value); + + /** + * Adds double attribute to the {@link Span}. + * + * @param key attribute key + * @param value attribute value + */ + void addSpanAttribute(String key, double value); + + /** + * Adds boolean attribute to the {@link Span}. + * + * @param key attribute key + * @param value attribute value + */ + void addSpanAttribute(String key, boolean value); + + /** + * Adds an event to the {@link Span}. + * + * @param event event name + */ + void addSpanEvent(String event); + + /** + * Records error in the span + * + * @param exception exception to be recorded + */ + void setError(Exception exception); + + /** + * closes the scope + */ + @Override + void close(); +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java index cf67165d889bc..0ce7bf3f65ccd 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java @@ -12,63 +12,20 @@ /** * An auto-closeable that represents scope of the span. - * It provides interface for all the span operations. */ public interface SpanScope extends AutoCloseable { + /** * No-op Scope implementation */ SpanScope NO_OP = new NoopSpanScope(); - /** - * Adds string attribute to the {@link Span}. - * - * @param key attribute key - * @param value attribute value - */ - void addSpanAttribute(String key, String value); - - /** - * Adds long attribute to the {@link Span}. - * - * @param key attribute key - * @param value attribute value - */ - void addSpanAttribute(String key, long value); - - /** - * Adds double attribute to the {@link Span}. - * - * @param key attribute key - * @param value attribute value - */ - void addSpanAttribute(String key, double value); - - /** - * Adds boolean attribute to the {@link Span}. - * - * @param key attribute key - * @param value attribute value - */ - void addSpanAttribute(String key, boolean value); - - /** - * Adds an event to the {@link Span}. - * - * @param event event name - */ - void addSpanEvent(String event); - - /** - * Records error in the span - * - * @param exception exception to be recorded - */ - void setError(Exception exception); + @Override + void close(); /** - * closes the scope + * Returns span attached with the {@link SpanScope} + * @return span. */ - @Override - void close(); + Span getSpan(); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java new file mode 100644 index 0000000000000..b980c02d5dac6 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java @@ -0,0 +1,43 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +/** + * Wrapper class to hold reference of {@link SpanScope} + * + * @opensearch.internal + */ +final class SpanScopeReference { + + private SpanScope spanScope; + + /** + * Creates the wrapper with given spanScope + * @param spanScope the spanScope object to wrap + */ + public SpanScopeReference(SpanScope spanScope) { + this.spanScope = spanScope; + } + + /** + * Returns the spanScope object + * @return underlying spanScope + */ + public SpanScope getSpanScope() { + return spanScope; + } + + /** + * Updates the underlying span + * @param spanScope underlying span + */ + public void setSpanScope(SpanScope spanScope) { + this.spanScope = spanScope; + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java index ab017058ae87d..d01323146dd2c 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java @@ -20,44 +20,71 @@ * All methods on the Tracer object are multi-thread safe. */ public interface Tracer extends HttpTracer, Closeable { - /** * Starts the {@link Span} with given {@link SpanCreationContext} * * @param context span context - * @return scope of the span, must be closed with explicit close or with try-with-resource + * @return span, must be closed. */ - SpanScope startSpan(SpanCreationContext context); + Span startSpan(SpanCreationContext context); /** * Starts the {@link Span} with given name * * @param spanName span name - * @return scope of the span, must be closed with explicit close or with try-with-resource + * @return span, must be closed. */ - SpanScope startSpan(String spanName); + Span startSpan(String spanName); /** * Starts the {@link Span} with given name and attributes. This is required in cases when some attribute based * decision needs to be made before starting the span. Very useful in the case of Sampling. - * @param spanName span name. + * + * @param spanName span name. * @param attributes attributes to be added. - * @return scope of the span, must be closed with explicit close or with try-with-resource + * @return span, must be closed. */ - SpanScope startSpan(String spanName, Attributes attributes); + Span startSpan(String spanName, Attributes attributes); /** * Starts the {@link Span} with the given name, parent and attributes. - * @param spanName span name. + * + * @param spanName span name. * @param parentSpan parent span. * @param attributes attributes to be added. - * @return scope of the span, must be closed with explicit close or with try-with-resource + * @return span, must be closed. */ - SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes attributes); + Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes); /** * Returns the current span. * @return current wrapped span. */ SpanContext getCurrentSpan(); + + /** + * Start the span and scoped it. This must be used for scenarios where {@link SpanScope} and {@link Span} lifecycles + * are same and ends within the same thread where created. + * @param spanCreationContext span creation context + * @return scope of the span, must be closed with explicit close or with try-with-resource + */ + ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext); + + /** + * Start the span and scoped it. This must be used for scenarios where {@link SpanScope} and {@link Span} lifecycles + * are same and ends within the same thread where created. + * @param spanCreationContext span creation context + * @param parentSpan parent span. + * @return scope of the span, must be closed with explicit close or with try-with-resource + */ + ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan); + + /** + * Creates the Span Scope for a current thread. It's mandatory to scope the span just after creation so that it will + * automatically manage the attach /detach to the current thread. + * @param span span to be scoped + * @return ScopedSpan + */ + SpanScope createSpanScope(Span span); + } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java index d85b404b0ce41..0f9ec54b18482 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java @@ -21,6 +21,11 @@ public interface TracerContextStorage { */ String CURRENT_SPAN = "current_span"; + /** + * Key for storing the current span. + */ + String CURRENT_SPAN_SCOPE = "current_span_scope"; + /** * Fetches value corresponding to key * @param key of the tracing context diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/http/HttpTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/http/HttpTracer.java index 64ef84335a95b..8027291d5480b 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/http/HttpTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/http/HttpTracer.java @@ -9,7 +9,6 @@ package org.opensearch.telemetry.tracing.http; import org.opensearch.telemetry.tracing.Span; -import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.attributes.Attributes; import java.util.List; @@ -28,7 +27,7 @@ public interface HttpTracer { * @param spanName span name. * @param header http request header. * @param attributes span attributes. - * @return scope of the span, must be closed with explicit close or with try-with-resource + * @return span. */ - SpanScope startSpan(String spanName, Map> header, Attributes attributes); + Span startSpan(String spanName, Map> header, Attributes attributes); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java new file mode 100644 index 0000000000000..b22e75bf37011 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java @@ -0,0 +1,59 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing.noop; + +import org.opensearch.telemetry.tracing.ScopedSpan; + +/** + * No-op implementation of SpanScope + * + * @opensearch.internal + */ +public final class NoopScopedSpan implements ScopedSpan { + + /** + * No-args constructor + */ + public NoopScopedSpan() {} + + @Override + public void addSpanAttribute(String key, String value) { + + } + + @Override + public void addSpanAttribute(String key, long value) { + + } + + @Override + public void addSpanAttribute(String key, double value) { + + } + + @Override + public void addSpanAttribute(String key, boolean value) { + + } + + @Override + public void addSpanEvent(String event) { + + } + + @Override + public void setError(Exception exception) { + + } + + @Override + public void close() { + + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpan.java new file mode 100644 index 0000000000000..e053e6576fcea --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpan.java @@ -0,0 +1,81 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing.noop; + +import org.opensearch.telemetry.tracing.Span; + +/** + * No-op implementation of {@link org.opensearch.telemetry.tracing.Span} + */ +public class NoopSpan implements Span { + + /** + * No-op Span instance + */ + public final static NoopSpan INSTANCE = new NoopSpan(); + + private NoopSpan() { + + } + + @Override + public void endSpan() { + + } + + @Override + public Span getParentSpan() { + return null; + } + + @Override + public String getSpanName() { + return "noop-span"; + } + + @Override + public void addAttribute(String key, String value) { + + } + + @Override + public void addAttribute(String key, Long value) { + + } + + @Override + public void addAttribute(String key, Double value) { + + } + + @Override + public void addAttribute(String key, Boolean value) { + + } + + @Override + public void setError(Exception exception) { + + } + + @Override + public void addEvent(String event) { + + } + + @Override + public String getTraceId() { + return "noop-trace-id"; + } + + @Override + public String getSpanId() { + return "noop-span-id"; + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java index a1d16d1d80d00..b1094ebb71731 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java @@ -8,52 +8,27 @@ package org.opensearch.telemetry.tracing.noop; +import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.SpanScope; /** - * No-op implementation of SpanScope - * - * @opensearch.internal + * No-op implementation of {@link SpanScope} */ -public final class NoopSpanScope implements SpanScope { - +public class NoopSpanScope implements SpanScope { /** - * No-args constructor + * Constructor. */ - public NoopSpanScope() {} - - @Override - public void addSpanAttribute(String key, String value) { - - } - - @Override - public void addSpanAttribute(String key, long value) { - - } - - @Override - public void addSpanAttribute(String key, double value) { + public NoopSpanScope() { } @Override - public void addSpanAttribute(String key, boolean value) { - - } - - @Override - public void addSpanEvent(String event) { - - } - - @Override - public void setError(Exception exception) { + public void close() { } @Override - public void close() { - + public Span getSpan() { + return NoopSpan.INSTANCE; } } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java index a190ac16ad305..c704e42867d78 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java @@ -8,6 +8,8 @@ package org.opensearch.telemetry.tracing.noop; +import org.opensearch.telemetry.tracing.ScopedSpan; +import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.SpanContext; import org.opensearch.telemetry.tracing.SpanCreationContext; import org.opensearch.telemetry.tracing.SpanScope; @@ -32,37 +34,52 @@ public class NoopTracer implements Tracer { private NoopTracer() {} @Override - public SpanScope startSpan(SpanCreationContext context) { - return SpanScope.NO_OP; + public Span startSpan(SpanCreationContext context) { + return NoopSpan.INSTANCE; } @Override - public SpanScope startSpan(String spanName) { - return SpanScope.NO_OP; + public Span startSpan(String spanName) { + return NoopSpan.INSTANCE; } @Override - public SpanScope startSpan(String spanName, Attributes attributes) { - return SpanScope.NO_OP; + public Span startSpan(String spanName, Attributes attributes) { + return NoopSpan.INSTANCE; } @Override - public SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { - return SpanScope.NO_OP; + public Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { + return NoopSpan.INSTANCE; } @Override public SpanContext getCurrentSpan() { - return null; + return new SpanContext(NoopSpan.INSTANCE); } @Override - public void close() { + public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) { + return ScopedSpan.NO_OP; + } + @Override + public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan) { + return ScopedSpan.NO_OP; } @Override - public SpanScope startSpan(String spanName, Map> header, Attributes attributes) { + public SpanScope createSpanScope(Span span) { return SpanScope.NO_OP; } + + @Override + public void close() { + + } + + @Override + public Span startSpan(String spanName, Map> header, Attributes attributes) { + return NoopSpan.INSTANCE; + } } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java index 54a5a7f1678e6..4672574e9f4ca 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/runnable/TraceableRunnable.java @@ -8,8 +8,9 @@ package org.opensearch.telemetry.tracing.runnable; +import org.opensearch.telemetry.tracing.ScopedSpan; import org.opensearch.telemetry.tracing.SpanContext; -import org.opensearch.telemetry.tracing.SpanScope; +import org.opensearch.telemetry.tracing.SpanCreationContext; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.telemetry.tracing.attributes.Attributes; @@ -41,7 +42,7 @@ public TraceableRunnable(Tracer tracer, String spanName, SpanContext parent, Att @Override public void run() { - try (SpanScope spanScope = tracer.startSpan(spanName, parent, attributes)) { + try (ScopedSpan spanScope = tracer.startScopedSpan(new SpanCreationContext(spanName, attributes), parent)) { runnable.run(); } } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultSpanScopeTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java similarity index 55% rename from libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultSpanScopeTests.java rename to libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java index eea6b77ce6e1e..3119b4a4ef827 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultSpanScopeTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java @@ -10,26 +10,28 @@ import org.opensearch.test.OpenSearchTestCase; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -public class DefaultSpanScopeTests extends OpenSearchTestCase { +public class DefaultScopedSpanTests extends OpenSearchTestCase { @SuppressWarnings("unchecked") public void testClose() { Span mockSpan = mock(Span.class); - Consumer mockConsumer = mock(Consumer.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, mockConsumer); + SpanScope mockSpanScope = mock(SpanScope.class); + BiConsumer mockConsumer = mock(BiConsumer.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, mockConsumer); defaultSpanScope.close(); - verify(mockConsumer).accept(mockSpan); + verify(mockConsumer).accept(mockSpan, mockSpanScope); } public void testAddSpanAttributeString() { Span mockSpan = mock(Span.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, null); + SpanScope mockSpanScope = mock(SpanScope.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); defaultSpanScope.addSpanAttribute("key", "value"); verify(mockSpan).addAttribute("key", "value"); @@ -37,7 +39,8 @@ public void testAddSpanAttributeString() { public void testAddSpanAttributeLong() { Span mockSpan = mock(Span.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, null); + SpanScope mockSpanScope = mock(SpanScope.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); defaultSpanScope.addSpanAttribute("key", 1L); verify(mockSpan).addAttribute("key", 1L); @@ -45,7 +48,8 @@ public void testAddSpanAttributeLong() { public void testAddSpanAttributeDouble() { Span mockSpan = mock(Span.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, null); + SpanScope mockSpanScope = mock(SpanScope.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); defaultSpanScope.addSpanAttribute("key", 1.0); verify(mockSpan).addAttribute("key", 1.0); @@ -53,7 +57,8 @@ public void testAddSpanAttributeDouble() { public void testAddSpanAttributeBoolean() { Span mockSpan = mock(Span.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, null); + SpanScope mockSpanScope = mock(SpanScope.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); defaultSpanScope.addSpanAttribute("key", true); verify(mockSpan).addAttribute("key", true); @@ -61,7 +66,8 @@ public void testAddSpanAttributeBoolean() { public void testAddEvent() { Span mockSpan = mock(Span.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, null); + SpanScope mockSpanScope = mock(SpanScope.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); defaultSpanScope.addSpanEvent("eventName"); verify(mockSpan).addEvent("eventName"); @@ -69,7 +75,8 @@ public void testAddEvent() { public void testSetError() { Span mockSpan = mock(Span.class); - DefaultSpanScope defaultSpanScope = new DefaultSpanScope(mockSpan, null); + SpanScope mockSpanScope = mock(SpanScope.class); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); Exception ex = new Exception("error"); defaultSpanScope.setError(ex); diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 325b6829c04b1..04bc6f717e9d6 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -14,7 +14,6 @@ import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.telemetry.tracing.MockSpan; import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; -import org.junit.Assert; import java.io.IOException; @@ -28,10 +27,13 @@ public class DefaultTracerTests extends OpenSearchTestCase { private TracingTelemetry mockTracingTelemetry; - private TracerContextStorage mockTracerContextStorage; + private TracerContextStorage mockTracerContextSpanStorage; + private TracerContextStorage mockTracerContextSpanScopeStorage; private Span mockSpan; private Span mockParentSpan; + private SpanScope mockSpanScope; + @Override public void setUp() throws Exception { super.setUp(); @@ -44,15 +46,23 @@ public void tearDown() throws Exception { } public void testCreateSpan() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer( + mockTracingTelemetry, + mockTracerContextSpanStorage, + mockTracerContextSpanScopeStorage + ); defaultTracer.startSpan("span_name"); - Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); } public void testCreateSpanWithAttributesWithMock() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer( + mockTracingTelemetry, + mockTracerContextSpanStorage, + mockTracerContextSpanScopeStorage + ); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan("span_name", attributes); @@ -60,19 +70,25 @@ public void testCreateSpanWithAttributesWithMock() { } public void testCreateSpanWithAttributesWithParentMock() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer( + mockTracingTelemetry, + mockTracerContextSpanStorage, + mockTracerContextSpanScopeStorage + ); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); - verify(mockTracerContextStorage, never()).get(TracerContextStorage.CURRENT_SPAN); + verify(mockTracerContextSpanStorage, never()).get(TracerContextStorage.CURRENT_SPAN); } public void testCreateSpanWithAttributes() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); DefaultTracer defaultTracer = new DefaultTracer( tracingTelemetry, - new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) + new ThreadContextSpanStorage(threadContext, tracingTelemetry), + new ThreadContextSpanScopeStorage(threadContext) ); defaultTracer.startSpan( @@ -80,18 +96,19 @@ public void testCreateSpanWithAttributes() { Attributes.create().addAttribute("key1", 1.0).addAttribute("key2", 2l).addAttribute("key3", true).addAttribute("key4", "key4") ); - Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - Assert.assertEquals(1.0, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key1")); - Assert.assertEquals(2l, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key2")); - Assert.assertEquals(true, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key3")); - Assert.assertEquals("key4", ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key4")); + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals(1.0, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key1")); + assertEquals(2l, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key2")); + assertEquals(true, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key3")); + assertEquals("key4", ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key4")); } public void testCreateSpanWithParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); DefaultTracer defaultTracer = new DefaultTracer( tracingTelemetry, - new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) + new ThreadContextSpanStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry), + mockTracerContextSpanScopeStorage ); defaultTracer.startSpan("span_name", null); @@ -100,12 +117,16 @@ public void testCreateSpanWithParent() { defaultTracer.startSpan("span_name_1", parentSpan, Attributes.EMPTY); - Assert.assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - Assert.assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan().getParentSpan()); + assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan().getParentSpan()); } public void testCreateSpanWithContext() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer( + mockTracingTelemetry, + mockTracerContextSpanStorage, + mockTracerContextSpanScopeStorage + ); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan(new SpanCreationContext("span_name", attributes)); @@ -114,27 +135,115 @@ public void testCreateSpanWithContext() { public void testCreateSpanWithNullParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); DefaultTracer defaultTracer = new DefaultTracer( tracingTelemetry, - new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) + new ThreadContextSpanStorage(threadContext, tracingTelemetry), + new ThreadContextSpanScopeStorage(threadContext) ); - defaultTracer.startSpan("span_name", null, Attributes.EMPTY); + defaultTracer.startSpan("span_name", (SpanContext) null, Attributes.EMPTY); - Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - Assert.assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); } - public void testEndSpanByClosingScope() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); - try (SpanScope spanScope = defaultTracer.startSpan("span_name")) { - verify(mockTracerContextStorage).put(TracerContextStorage.CURRENT_SPAN, mockSpan); - } - verify(mockTracerContextStorage).put(TracerContextStorage.CURRENT_SPAN, mockParentSpan); + public void testEndSpanByClosingScopedSpan() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals( + ((DefaultScopedSpan) scopedSpan).getSpanScope(), + spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE) + ); + scopedSpan.close(); + assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan).getSpan()).hasEnded()); + assertEquals(null, defaultTracer.getCurrentSpan()); + assertEquals(null, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + + } + + public void testEndSpanByClosingScopedSpanMultiple() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); + ScopedSpan scopedSpan1 = defaultTracer.startScopedSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); + + assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals( + ((DefaultScopedSpan) scopedSpan1).getSpanScope(), + spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE) + ); + + scopedSpan1.close(); + assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan1).getSpan()).hasEnded()); + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals( + ((DefaultScopedSpan) scopedSpan).getSpanScope(), + spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE) + ); + + scopedSpan.close(); + assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan).getSpan()).hasEnded()); + assertEquals(null, defaultTracer.getCurrentSpan()); + assertEquals(null, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + + } + + public void testEndSpanByClosingSpanScope() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); + SpanScope spanScope = defaultTracer.createSpanScope(span); + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals(spanScope, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + + spanScope.close(); + assertEquals(null, defaultTracer.getCurrentSpan()); + assertFalse(((MockSpan) span).hasEnded()); + + } + + public void testEndSpanByClosingSpanScopeMultiple() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); + Span span1 = defaultTracer.startSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); + SpanScope spanScope = defaultTracer.createSpanScope(span); + SpanScope spanScope1 = defaultTracer.createSpanScope(span1); + assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals(spanScope1, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + + spanScope1.close(); + + assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); + assertEquals(spanScope, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertFalse(((MockSpan) span1).hasEnded()); + assertFalse(((MockSpan) span).hasEnded()); + spanScope.close(); + + assertEquals(null, defaultTracer.getCurrentSpan()); + assertEquals(null, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertFalse(((MockSpan) span).hasEnded()); + assertFalse(((MockSpan) span1).hasEnded()); + } public void testClose() throws IOException { - Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); + Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage, mockTracerContextSpanScopeStorage); defaultTracer.close(); @@ -146,14 +255,17 @@ private void setupMocks() { mockTracingTelemetry = mock(TracingTelemetry.class); mockSpan = mock(Span.class); mockParentSpan = mock(Span.class); - mockTracerContextStorage = mock(TracerContextStorage.class); + mockSpanScope = mock(SpanScope.class); + mockTracerContextSpanStorage = mock(TracerContextStorage.class); + mockTracerContextSpanScopeStorage = mock(TracerContextStorage.class); when(mockSpan.getSpanName()).thenReturn("span_name"); when(mockSpan.getSpanId()).thenReturn("span_id"); when(mockSpan.getTraceId()).thenReturn("trace_id"); when(mockSpan.getParentSpan()).thenReturn(mockParentSpan); when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); - when(mockTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); + when(mockTracerContextSpanStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); + when(mockTracerContextSpanScopeStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockSpanScope); when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class))).thenReturn(mockSpan); } } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java index bcd8ffe41a17b..8405febb92b81 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java @@ -21,49 +21,57 @@ public class TraceableRunnableTests extends OpenSearchTestCase { - private final ThreadContextBasedTracerContextStorage contextStorage = new ThreadContextBasedTracerContextStorage( + private final ThreadContextSpanStorage spanContextStorage = new ThreadContextSpanStorage( new ThreadContext(Settings.EMPTY), new MockTracingTelemetry() ); + private final ThreadContextSpanScopeStorage spanScopeContextStorage = new ThreadContextSpanScopeStorage( + new ThreadContext(Settings.EMPTY) + ); + public void testRunnableWithNullParent() throws Exception { String spanName = "testRunnable"; - DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), contextStorage); + final DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage, spanScopeContextStorage); final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false); - + final AtomicReference spanNameCaptured = new AtomicReference<>(); + final AtomicReference attributeValue = new AtomicReference<>(); TraceableRunnable traceableRunnable = new TraceableRunnable( defaultTracer, spanName, null, Attributes.create().addAttribute("name", "value"), () -> { + spanNameCaptured.set(defaultTracer.getCurrentSpan().getSpan().getSpanName()); + attributeValue.set((String) ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("name")); isRunnableCompleted.set(true); } ); traceableRunnable.run(); assertTrue(isRunnableCompleted.get()); - assertEquals(spanName, defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); - assertEquals("value", ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("name")); - + assertEquals(spanName, spanNameCaptured.get()); + assertEquals(null, defaultTracer.getCurrentSpan()); + assertEquals(null, defaultTracer.getCurrentSpan()); + assertEquals("value", attributeValue.get()); } public void testRunnableWithParent() throws Exception { String spanName = "testRunnable"; String parentSpanName = "parentSpan"; - DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), contextStorage); - defaultTracer.startSpan(parentSpanName); - SpanContext parentSpan = defaultTracer.getCurrentSpan(); + DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage, spanScopeContextStorage); + ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext(parentSpanName, Attributes.EMPTY)); + SpanContext parentSpanContext = defaultTracer.getCurrentSpan(); AtomicReference currentSpan = new AtomicReference<>(); final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false); - TraceableRunnable traceableRunnable = new TraceableRunnable(defaultTracer, spanName, parentSpan, Attributes.EMPTY, () -> { + TraceableRunnable traceableRunnable = new TraceableRunnable(defaultTracer, spanName, parentSpanContext, Attributes.EMPTY, () -> { isRunnableCompleted.set(true); currentSpan.set(defaultTracer.getCurrentSpan()); }); traceableRunnable.run(); assertTrue(isRunnableCompleted.get()); assertEquals(spanName, currentSpan.get().getSpan().getSpanName()); - assertEquals(parentSpan.getSpan(), currentSpan.get().getSpan().getParentSpan()); - assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan()); + assertEquals(((DefaultScopedSpan) scopedSpan).getSpan(), currentSpan.get().getSpan().getParentSpan()); + assertEquals(((DefaultScopedSpan) scopedSpan).getSpan(), defaultTracer.getCurrentSpan().getSpan()); + scopedSpan.close(); } } diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java new file mode 100644 index 0000000000000..c2a2070602916 --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java @@ -0,0 +1,53 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +import org.opensearch.common.util.concurrent.ThreadContext; + +import java.util.Objects; +import java.util.Optional; + +/** + * Core's ThreadContext based TracerContextStorage implementation + * + * @opensearch.internal + */ +public class ThreadContextSpanScopeStorage implements TracerContextStorage { + + private final ThreadContext threadContext; + + public ThreadContextSpanScopeStorage(ThreadContext threadContext) { + this.threadContext = Objects.requireNonNull(threadContext); + } + + @Override + public SpanScope get(String key) { + return getCurrentSpanScope(key); + } + + @Override + public void put(String key, SpanScope spanScope) { + SpanScopeReference currentSpanRef = threadContext.getTransient(key); + if (currentSpanRef == null) { + threadContext.putTransient(key, new SpanScopeReference(spanScope)); + } else { + currentSpanRef.setSpanScope(spanScope); + } + } + + SpanScope getCurrentSpanScope(String key) { + return spanFromThreadContext(key).orElse(null); + } + + private Optional spanFromThreadContext(String key) { + SpanScopeReference currentSpanScopeRef = threadContext.getTransient(key); + return (currentSpanScopeRef == null) ? Optional.empty() : Optional.ofNullable(currentSpanScopeRef.getSpanScope()); + } + +} diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorage.java b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanStorage.java similarity index 90% rename from server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorage.java rename to server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanStorage.java index a32facdc71146..b4c3ab77819ee 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorage.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanStorage.java @@ -21,13 +21,13 @@ * * @opensearch.internal */ -public class ThreadContextBasedTracerContextStorage implements TracerContextStorage, ThreadContextStatePropagator { +public class ThreadContextSpanStorage implements TracerContextStorage, ThreadContextStatePropagator { private final ThreadContext threadContext; private final TracingTelemetry tracingTelemetry; - public ThreadContextBasedTracerContextStorage(ThreadContext threadContext, TracingTelemetry tracingTelemetry) { + public ThreadContextSpanStorage(ThreadContext threadContext, TracingTelemetry tracingTelemetry) { this.threadContext = Objects.requireNonNull(threadContext); this.tracingTelemetry = Objects.requireNonNull(tracingTelemetry); this.threadContext.registerThreadContextStatePropagator(this); @@ -40,9 +40,6 @@ public Span get(String key) { @Override public void put(String key, Span span) { - if (span == null) { - return; - } SpanReference currentSpanRef = threadContext.getTransient(key); if (currentSpanRef == null) { threadContext.putTransient(key, new SpanReference(span)); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java b/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java index d8fe812c82f53..b37e05c527abf 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java @@ -66,11 +66,11 @@ private Tracer tracer(Optional telemetry, ThreadContext threadContext } private Tracer createDefaultTracer(TracingTelemetry tracingTelemetry, ThreadContext threadContext) { - TracerContextStorage tracerContextStorage = new ThreadContextBasedTracerContextStorage( - threadContext, - tracingTelemetry - ); - return new DefaultTracer(tracingTelemetry, tracerContextStorage); + TracerContextStorage tracerContextSpanStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + + TracerContextStorage tracerContextSpanScopeStorage = new ThreadContextSpanScopeStorage(threadContext); + + return new DefaultTracer(tracingTelemetry, tracerContextSpanStorage, tracerContextSpanScopeStorage); } private Tracer createWrappedTracer(Tracer defaultTracer) { diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java index 53a6b363fce50..39d76a0384084 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java @@ -38,17 +38,17 @@ public WrappedTracer(TelemetrySettings telemetrySettings, Tracer defaultTracer) } @Override - public SpanScope startSpan(SpanCreationContext context) { + public Span startSpan(SpanCreationContext context) { return startSpan(context.getSpanName(), context.getAttributes()); } @Override - public SpanScope startSpan(String spanName) { + public Span startSpan(String spanName) { return startSpan(spanName, Attributes.EMPTY); } @Override - public SpanScope startSpan(String spanName, Attributes attributes) { + public Span startSpan(String spanName, Attributes attributes) { return startSpan(spanName, (SpanContext) null, attributes); } @@ -59,7 +59,22 @@ public SpanContext getCurrentSpan() { } @Override - public SpanScope startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { + public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) { + return startScopedSpan(spanCreationContext, null); + } + + @Override + public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan) { + return getDelegateTracer().startScopedSpan(spanCreationContext, parentSpan); + } + + @Override + public SpanScope createSpanScope(Span span) { + return getDelegateTracer().createSpanScope(span); + } + + @Override + public Span startSpan(String spanName, SpanContext parentSpan, Attributes attributes) { Tracer delegateTracer = getDelegateTracer(); return delegateTracer.startSpan(spanName, parentSpan, attributes); } @@ -75,7 +90,7 @@ Tracer getDelegateTracer() { } @Override - public SpanScope startSpan(String spanName, Map> headers, Attributes attributes) { + public Span startSpan(String spanName, Map> headers, Attributes attributes) { return defaultTracer.startSpan(spanName, headers, attributes); } } diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java index 451a1b9e3eb9c..7b1623435fabf 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java @@ -15,6 +15,8 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.telemetry.Telemetry; import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.tracing.noop.NoopSpan; import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.OpenSearchTestCase; import org.junit.After; @@ -46,7 +48,23 @@ public void testGetTracerWithUnavailableTracingTelemetryReturnsNoopTracer() { Tracer tracer = tracerFactory.getTracer(); assertTrue(tracer instanceof NoopTracer); - assertTrue(tracer.startSpan("foo") == SpanScope.NO_OP); + assertTrue(tracer.startSpan("foo") == NoopSpan.INSTANCE); + assertTrue(tracer.startScopedSpan(new SpanCreationContext("foo", Attributes.EMPTY)) == ScopedSpan.NO_OP); + assertTrue(tracer.startScopedSpan(new SpanCreationContext("foo", Attributes.EMPTY)) == ScopedSpan.NO_OP); + assertTrue(tracer.createSpanScope(tracer.startSpan("foo")) == SpanScope.NO_OP); + } + + public void testGetTracerWithUnavailableTracingTelemetry() { + Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + Telemetry mockTelemetry = mock(Telemetry.class); + when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); + tracerFactory = new TracerFactory(telemetrySettings, Optional.empty(), new ThreadContext(Settings.EMPTY)); + + Tracer tracer = tracerFactory.getTracer(); + + assertTrue(tracer instanceof NoopTracer); + assertTrue(tracer.startScopedSpan(new SpanCreationContext("foo", Attributes.EMPTY)) == ScopedSpan.NO_OP); } public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { From 17ef6ed3f309622156d4bc8f937b9050ded17d03 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 29 Aug 2023 21:08:04 +0530 Subject: [PATCH 04/16] Update changelog Signed-off-by: Gagan Juneja --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ff048fdbf1a3..a28761204d1f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Separate request-based and settings-based concurrent segment search controls and introduce AggregatorFactory method to determine concurrent search support ([#9469](https://github.com/opensearch-project/OpenSearch/pull/9469)) - [Remote Store] Rate limiter integration for remote store uploads and downloads([#9448](https://github.com/opensearch-project/OpenSearch/pull/9448/)) - [Remote Store] Implicitly use replication type SEGMENT for remote store clusters ([#9264](https://github.com/opensearch-project/OpenSearch/pull/9264)) +- Redefine telemetry context restoration and propagation ([#9617](https://github.com/opensearch-project/OpenSearch/pull/9617)) ### Deprecated From ae93e81d54342eb8dda4b94b0c1002606a4d01cb Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 30 Aug 2023 20:57:29 +0530 Subject: [PATCH 05/16] Stores the SpanScope in ThreadLocal Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultTracer.java | 15 ++-- .../telemetry/tracing/SpanScopeReference.java | 43 ---------- .../telemetry/tracing/DefaultTracerTests.java | 83 +++++-------------- .../tracing/TraceableRunnableTests.java | 8 +- .../ThreadContextSpanScopeStorage.java | 53 ------------ .../telemetry/tracing/TracerFactory.java | 5 +- 6 files changed, 29 insertions(+), 178 deletions(-) delete mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java delete mode 100644 server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 26967520028fe..99184fb857854 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -28,7 +28,7 @@ class DefaultTracer implements Tracer { private final TracingTelemetry tracingTelemetry; private final TracerContextStorage spanTracerContextStorage; - private final TracerContextStorage spanScopeTracerContextStorage; + private final ThreadLocal spanScopeThreadLocal = new ThreadLocal<>(); /** * Creates DefaultTracer instance @@ -36,14 +36,9 @@ class DefaultTracer implements Tracer { * @param tracingTelemetry tracing telemetry instance * @param spanTracerContextStorage storage used for storing current span context */ - public DefaultTracer( - TracingTelemetry tracingTelemetry, - TracerContextStorage spanTracerContextStorage, - TracerContextStorage spanScopeTracerContextStorage - ) { + public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage spanTracerContextStorage) { this.tracingTelemetry = tracingTelemetry; this.spanTracerContextStorage = spanTracerContextStorage; - this.spanScopeTracerContextStorage = spanScopeTracerContextStorage; } @Override @@ -88,8 +83,8 @@ public SpanContext getCurrentSpan() { return (currentSpan == null) ? null : new SpanContext(currentSpan); } - private SpanScope getCurrentSpanScope() { - return spanScopeTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN_SCOPE); + SpanScope getCurrentSpanScope() { + return spanScopeThreadLocal.get(); } @Override @@ -131,7 +126,7 @@ private void closeSpanScope(SpanScope beforeAttachedSpanScope) { } private void setCurrentSpanScopeInContext(SpanScope spanScope) { - spanScopeTracerContextStorage.put(TracerContextStorage.CURRENT_SPAN_SCOPE, spanScope); + spanScopeThreadLocal.set(spanScope); } private void endScopedSpan(Span span, SpanScope spanScope) { diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java deleted file mode 100644 index b980c02d5dac6..0000000000000 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScopeReference.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.tracing; - -/** - * Wrapper class to hold reference of {@link SpanScope} - * - * @opensearch.internal - */ -final class SpanScopeReference { - - private SpanScope spanScope; - - /** - * Creates the wrapper with given spanScope - * @param spanScope the spanScope object to wrap - */ - public SpanScopeReference(SpanScope spanScope) { - this.spanScope = spanScope; - } - - /** - * Returns the spanScope object - * @return underlying spanScope - */ - public SpanScope getSpanScope() { - return spanScope; - } - - /** - * Updates the underlying span - * @param spanScope underlying span - */ - public void setSpanScope(SpanScope spanScope) { - this.spanScope = spanScope; - } -} diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 04bc6f717e9d6..03c28e4665ac7 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -28,7 +28,6 @@ public class DefaultTracerTests extends OpenSearchTestCase { private TracingTelemetry mockTracingTelemetry; private TracerContextStorage mockTracerContextSpanStorage; - private TracerContextStorage mockTracerContextSpanScopeStorage; private Span mockSpan; private Span mockParentSpan; @@ -46,11 +45,7 @@ public void tearDown() throws Exception { } public void testCreateSpan() { - DefaultTracer defaultTracer = new DefaultTracer( - mockTracingTelemetry, - mockTracerContextSpanStorage, - mockTracerContextSpanScopeStorage - ); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); defaultTracer.startSpan("span_name"); @@ -58,11 +53,7 @@ public void testCreateSpan() { } public void testCreateSpanWithAttributesWithMock() { - DefaultTracer defaultTracer = new DefaultTracer( - mockTracingTelemetry, - mockTracerContextSpanStorage, - mockTracerContextSpanScopeStorage - ); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan("span_name", attributes); @@ -70,11 +61,7 @@ public void testCreateSpanWithAttributesWithMock() { } public void testCreateSpanWithAttributesWithParentMock() { - DefaultTracer defaultTracer = new DefaultTracer( - mockTracingTelemetry, - mockTracerContextSpanStorage, - mockTracerContextSpanScopeStorage - ); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); @@ -85,11 +72,7 @@ public void testCreateSpanWithAttributesWithParentMock() { public void testCreateSpanWithAttributes() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - DefaultTracer defaultTracer = new DefaultTracer( - tracingTelemetry, - new ThreadContextSpanStorage(threadContext, tracingTelemetry), - new ThreadContextSpanScopeStorage(threadContext) - ); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, new ThreadContextSpanStorage(threadContext, tracingTelemetry)); defaultTracer.startSpan( "span_name", @@ -107,8 +90,7 @@ public void testCreateSpanWithParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); DefaultTracer defaultTracer = new DefaultTracer( tracingTelemetry, - new ThreadContextSpanStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry), - mockTracerContextSpanScopeStorage + new ThreadContextSpanStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) ); defaultTracer.startSpan("span_name", null); @@ -122,11 +104,7 @@ public void testCreateSpanWithParent() { } public void testCreateSpanWithContext() { - DefaultTracer defaultTracer = new DefaultTracer( - mockTracingTelemetry, - mockTracerContextSpanStorage, - mockTracerContextSpanScopeStorage - ); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan(new SpanCreationContext("span_name", attributes)); @@ -136,11 +114,7 @@ public void testCreateSpanWithContext() { public void testCreateSpanWithNullParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - DefaultTracer defaultTracer = new DefaultTracer( - tracingTelemetry, - new ThreadContextSpanStorage(threadContext, tracingTelemetry), - new ThreadContextSpanScopeStorage(threadContext) - ); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, new ThreadContextSpanStorage(threadContext, tracingTelemetry)); defaultTracer.startSpan("span_name", (SpanContext) null, Attributes.EMPTY); @@ -152,18 +126,14 @@ public void testEndSpanByClosingScopedSpan() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); - ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); - DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals( - ((DefaultScopedSpan) scopedSpan).getSpanScope(), - spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE) - ); + assertEquals(((DefaultScopedSpan) scopedSpan).getSpanScope(), defaultTracer.getCurrentSpanScope()); scopedSpan.close(); assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan).getSpan()).hasEnded()); assertEquals(null, defaultTracer.getCurrentSpan()); - assertEquals(null, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertEquals(null, defaultTracer.getCurrentSpanScope()); } @@ -171,29 +141,22 @@ public void testEndSpanByClosingScopedSpanMultiple() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); - ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); - DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); ScopedSpan scopedSpan1 = defaultTracer.startScopedSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals( - ((DefaultScopedSpan) scopedSpan1).getSpanScope(), - spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE) - ); + assertEquals(((DefaultScopedSpan) scopedSpan1).getSpanScope(), defaultTracer.getCurrentSpanScope()); scopedSpan1.close(); assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan1).getSpan()).hasEnded()); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals( - ((DefaultScopedSpan) scopedSpan).getSpanScope(), - spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE) - ); + assertEquals(((DefaultScopedSpan) scopedSpan).getSpanScope(), defaultTracer.getCurrentSpanScope()); scopedSpan.close(); assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan).getSpan()).hasEnded()); assertEquals(null, defaultTracer.getCurrentSpan()); - assertEquals(null, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertEquals(null, defaultTracer.getCurrentSpanScope()); } @@ -201,12 +164,11 @@ public void testEndSpanByClosingSpanScope() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); - ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); - DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); SpanScope spanScope = defaultTracer.createSpanScope(span); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(spanScope, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertEquals(spanScope, defaultTracer.getCurrentSpanScope()); spanScope.close(); assertEquals(null, defaultTracer.getCurrentSpan()); @@ -218,32 +180,31 @@ public void testEndSpanByClosingSpanScopeMultiple() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); - ThreadContextSpanScopeStorage spanScopeTracerContextStorage = new ThreadContextSpanScopeStorage(threadContext); - DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage, spanScopeTracerContextStorage); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); Span span1 = defaultTracer.startSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); SpanScope spanScope = defaultTracer.createSpanScope(span); SpanScope spanScope1 = defaultTracer.createSpanScope(span1); assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(spanScope1, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertEquals(spanScope1, defaultTracer.getCurrentSpanScope()); spanScope1.close(); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(spanScope, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertEquals(spanScope, defaultTracer.getCurrentSpanScope()); assertFalse(((MockSpan) span1).hasEnded()); assertFalse(((MockSpan) span).hasEnded()); spanScope.close(); assertEquals(null, defaultTracer.getCurrentSpan()); - assertEquals(null, spanScopeTracerContextStorage.getCurrentSpanScope(TracerContextStorage.CURRENT_SPAN_SCOPE)); + assertEquals(null, defaultTracer.getCurrentSpanScope()); assertFalse(((MockSpan) span).hasEnded()); assertFalse(((MockSpan) span1).hasEnded()); } public void testClose() throws IOException { - Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage, mockTracerContextSpanScopeStorage); + Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); defaultTracer.close(); @@ -257,7 +218,6 @@ private void setupMocks() { mockParentSpan = mock(Span.class); mockSpanScope = mock(SpanScope.class); mockTracerContextSpanStorage = mock(TracerContextStorage.class); - mockTracerContextSpanScopeStorage = mock(TracerContextStorage.class); when(mockSpan.getSpanName()).thenReturn("span_name"); when(mockSpan.getSpanId()).thenReturn("span_id"); when(mockSpan.getTraceId()).thenReturn("trace_id"); @@ -265,7 +225,6 @@ private void setupMocks() { when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); when(mockTracerContextSpanStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); - when(mockTracerContextSpanScopeStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockSpanScope); when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class))).thenReturn(mockSpan); } } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java index 8405febb92b81..d8634dc694190 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java @@ -26,13 +26,9 @@ public class TraceableRunnableTests extends OpenSearchTestCase { new MockTracingTelemetry() ); - private final ThreadContextSpanScopeStorage spanScopeContextStorage = new ThreadContextSpanScopeStorage( - new ThreadContext(Settings.EMPTY) - ); - public void testRunnableWithNullParent() throws Exception { String spanName = "testRunnable"; - final DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage, spanScopeContextStorage); + final DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage); final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false); final AtomicReference spanNameCaptured = new AtomicReference<>(); final AtomicReference attributeValue = new AtomicReference<>(); @@ -58,7 +54,7 @@ public void testRunnableWithNullParent() throws Exception { public void testRunnableWithParent() throws Exception { String spanName = "testRunnable"; String parentSpanName = "parentSpan"; - DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage, spanScopeContextStorage); + DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext(parentSpanName, Attributes.EMPTY)); SpanContext parentSpanContext = defaultTracer.getCurrentSpan(); AtomicReference currentSpan = new AtomicReference<>(); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java deleted file mode 100644 index c2a2070602916..0000000000000 --- a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanScopeStorage.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.tracing; - -import org.opensearch.common.util.concurrent.ThreadContext; - -import java.util.Objects; -import java.util.Optional; - -/** - * Core's ThreadContext based TracerContextStorage implementation - * - * @opensearch.internal - */ -public class ThreadContextSpanScopeStorage implements TracerContextStorage { - - private final ThreadContext threadContext; - - public ThreadContextSpanScopeStorage(ThreadContext threadContext) { - this.threadContext = Objects.requireNonNull(threadContext); - } - - @Override - public SpanScope get(String key) { - return getCurrentSpanScope(key); - } - - @Override - public void put(String key, SpanScope spanScope) { - SpanScopeReference currentSpanRef = threadContext.getTransient(key); - if (currentSpanRef == null) { - threadContext.putTransient(key, new SpanScopeReference(spanScope)); - } else { - currentSpanRef.setSpanScope(spanScope); - } - } - - SpanScope getCurrentSpanScope(String key) { - return spanFromThreadContext(key).orElse(null); - } - - private Optional spanFromThreadContext(String key) { - SpanScopeReference currentSpanScopeRef = threadContext.getTransient(key); - return (currentSpanScopeRef == null) ? Optional.empty() : Optional.ofNullable(currentSpanScopeRef.getSpanScope()); - } - -} diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java b/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java index b37e05c527abf..3a9303fba4363 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java @@ -67,10 +67,7 @@ private Tracer tracer(Optional telemetry, ThreadContext threadContext private Tracer createDefaultTracer(TracingTelemetry tracingTelemetry, ThreadContext threadContext) { TracerContextStorage tracerContextSpanStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); - - TracerContextStorage tracerContextSpanScopeStorage = new ThreadContextSpanScopeStorage(threadContext); - - return new DefaultTracer(tracingTelemetry, tracerContextSpanStorage, tracerContextSpanScopeStorage); + return new DefaultTracer(tracingTelemetry, tracerContextSpanStorage); } private Tracer createWrappedTracer(Tracer defaultTracer) { From ad7959df97a24eaa043c1b3e8f3042721634cde1 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 30 Aug 2023 23:56:31 +0530 Subject: [PATCH 06/16] Revert the context name changes Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultTracer.java | 14 +++--- .../tracing/TracerContextStorage.java | 5 -- .../telemetry/tracing/DefaultTracerTests.java | 50 +++++++++++++------ .../tracing/TraceableRunnableTests.java | 6 +-- ...readContextBasedTracerContextStorage.java} | 4 +- .../telemetry/tracing/TracerFactory.java | 7 ++- .../routing/ShardMovementStrategyTests.java | 4 -- 7 files changed, 51 insertions(+), 39 deletions(-) rename server/src/main/java/org/opensearch/telemetry/tracing/{ThreadContextSpanStorage.java => ThreadContextBasedTracerContextStorage.java} (92%) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 99184fb857854..498764d19f978 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -27,18 +27,18 @@ class DefaultTracer implements Tracer { static final String THREAD_NAME = "th_name"; private final TracingTelemetry tracingTelemetry; - private final TracerContextStorage spanTracerContextStorage; + private final TracerContextStorage tracerContextStorage; private final ThreadLocal spanScopeThreadLocal = new ThreadLocal<>(); /** * Creates DefaultTracer instance * * @param tracingTelemetry tracing telemetry instance - * @param spanTracerContextStorage storage used for storing current span context + * @param tracerContextStorage storage used for storing current span context */ - public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage spanTracerContextStorage) { + public DefaultTracer(TracingTelemetry tracingTelemetry, TracerContextStorage tracerContextStorage) { this.tracingTelemetry = tracingTelemetry; - this.spanTracerContextStorage = spanTracerContextStorage; + this.tracerContextStorage = tracerContextStorage; } @Override @@ -75,11 +75,11 @@ public void close() throws IOException { } private Span getCurrentSpanInternal() { - return spanTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); + return tracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); } public SpanContext getCurrentSpan() { - final Span currentSpan = spanTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); + final Span currentSpan = tracerContextStorage.get(TracerContextStorage.CURRENT_SPAN); return (currentSpan == null) ? null : new SpanContext(currentSpan); } @@ -146,7 +146,7 @@ private Span createSpan(String spanName, Span parentSpan, Attributes attributes) } private void setCurrentSpanInContext(Span span) { - spanTracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, span); + tracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, span); } /** diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java index 0f9ec54b18482..d85b404b0ce41 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracerContextStorage.java @@ -21,11 +21,6 @@ public interface TracerContextStorage { */ String CURRENT_SPAN = "current_span"; - /** - * Key for storing the current span. - */ - String CURRENT_SPAN_SCOPE = "current_span_scope"; - /** * Fetches value corresponding to key * @param key of the tracing context diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 03c28e4665ac7..77c43803960ae 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -27,7 +27,7 @@ public class DefaultTracerTests extends OpenSearchTestCase { private TracingTelemetry mockTracingTelemetry; - private TracerContextStorage mockTracerContextSpanStorage; + private TracerContextStorage mockTracerContextStorage; private Span mockSpan; private Span mockParentSpan; @@ -45,7 +45,7 @@ public void tearDown() throws Exception { } public void testCreateSpan() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); defaultTracer.startSpan("span_name"); @@ -53,7 +53,7 @@ public void testCreateSpan() { } public void testCreateSpanWithAttributesWithMock() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan("span_name", attributes); @@ -61,18 +61,21 @@ public void testCreateSpanWithAttributesWithMock() { } public void testCreateSpanWithAttributesWithParentMock() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); - verify(mockTracerContextSpanStorage, never()).get(TracerContextStorage.CURRENT_SPAN); + verify(mockTracerContextStorage, never()).get(TracerContextStorage.CURRENT_SPAN); } public void testCreateSpanWithAttributes() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, new ThreadContextSpanStorage(threadContext, tracingTelemetry)); + DefaultTracer defaultTracer = new DefaultTracer( + tracingTelemetry, + new ThreadContextBasedTracerContextStorage(threadContext, tracingTelemetry) + ); defaultTracer.startSpan( "span_name", @@ -90,7 +93,7 @@ public void testCreateSpanWithParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); DefaultTracer defaultTracer = new DefaultTracer( tracingTelemetry, - new ThreadContextSpanStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) + new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) ); defaultTracer.startSpan("span_name", null); @@ -104,7 +107,7 @@ public void testCreateSpanWithParent() { } public void testCreateSpanWithContext() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); + DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); defaultTracer.startSpan(new SpanCreationContext("span_name", attributes)); @@ -114,7 +117,10 @@ public void testCreateSpanWithContext() { public void testCreateSpanWithNullParent() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, new ThreadContextSpanStorage(threadContext, tracingTelemetry)); + DefaultTracer defaultTracer = new DefaultTracer( + tracingTelemetry, + new ThreadContextBasedTracerContextStorage(threadContext, tracingTelemetry) + ); defaultTracer.startSpan("span_name", (SpanContext) null, Attributes.EMPTY); @@ -125,7 +131,10 @@ public void testCreateSpanWithNullParent() { public void testEndSpanByClosingScopedSpan() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); @@ -140,7 +149,10 @@ public void testEndSpanByClosingScopedSpan() { public void testEndSpanByClosingScopedSpanMultiple() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); ScopedSpan scopedSpan1 = defaultTracer.startScopedSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); @@ -163,7 +175,10 @@ public void testEndSpanByClosingScopedSpanMultiple() { public void testEndSpanByClosingSpanScope() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); SpanScope spanScope = defaultTracer.createSpanScope(span); @@ -179,7 +194,10 @@ public void testEndSpanByClosingSpanScope() { public void testEndSpanByClosingSpanScopeMultiple() { TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - ThreadContextSpanStorage spanTracerStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); Span span1 = defaultTracer.startSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); @@ -204,7 +222,7 @@ public void testEndSpanByClosingSpanScopeMultiple() { } public void testClose() throws IOException { - Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextSpanStorage); + Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); defaultTracer.close(); @@ -217,14 +235,14 @@ private void setupMocks() { mockSpan = mock(Span.class); mockParentSpan = mock(Span.class); mockSpanScope = mock(SpanScope.class); - mockTracerContextSpanStorage = mock(TracerContextStorage.class); + mockTracerContextStorage = mock(TracerContextStorage.class); when(mockSpan.getSpanName()).thenReturn("span_name"); when(mockSpan.getSpanId()).thenReturn("span_id"); when(mockSpan.getTraceId()).thenReturn("trace_id"); when(mockSpan.getParentSpan()).thenReturn(mockParentSpan); when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); - when(mockTracerContextSpanStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); + when(mockTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class))).thenReturn(mockSpan); } } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java index d8634dc694190..a67d9b22ca738 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/TraceableRunnableTests.java @@ -21,14 +21,14 @@ public class TraceableRunnableTests extends OpenSearchTestCase { - private final ThreadContextSpanStorage spanContextStorage = new ThreadContextSpanStorage( + private final ThreadContextBasedTracerContextStorage contextStorage = new ThreadContextBasedTracerContextStorage( new ThreadContext(Settings.EMPTY), new MockTracingTelemetry() ); public void testRunnableWithNullParent() throws Exception { String spanName = "testRunnable"; - final DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage); + final DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), contextStorage); final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false); final AtomicReference spanNameCaptured = new AtomicReference<>(); final AtomicReference attributeValue = new AtomicReference<>(); @@ -54,7 +54,7 @@ public void testRunnableWithNullParent() throws Exception { public void testRunnableWithParent() throws Exception { String spanName = "testRunnable"; String parentSpanName = "parentSpan"; - DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), spanContextStorage); + DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), contextStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext(parentSpanName, Attributes.EMPTY)); SpanContext parentSpanContext = defaultTracer.getCurrentSpan(); AtomicReference currentSpan = new AtomicReference<>(); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanStorage.java b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorage.java similarity index 92% rename from server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanStorage.java rename to server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorage.java index b4c3ab77819ee..8e50dac169efd 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextSpanStorage.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorage.java @@ -21,13 +21,13 @@ * * @opensearch.internal */ -public class ThreadContextSpanStorage implements TracerContextStorage, ThreadContextStatePropagator { +public class ThreadContextBasedTracerContextStorage implements TracerContextStorage, ThreadContextStatePropagator { private final ThreadContext threadContext; private final TracingTelemetry tracingTelemetry; - public ThreadContextSpanStorage(ThreadContext threadContext, TracingTelemetry tracingTelemetry) { + public ThreadContextBasedTracerContextStorage(ThreadContext threadContext, TracingTelemetry tracingTelemetry) { this.threadContext = Objects.requireNonNull(threadContext); this.tracingTelemetry = Objects.requireNonNull(tracingTelemetry); this.threadContext.registerThreadContextStatePropagator(this); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java b/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java index 3a9303fba4363..d8fe812c82f53 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/TracerFactory.java @@ -66,8 +66,11 @@ private Tracer tracer(Optional telemetry, ThreadContext threadContext } private Tracer createDefaultTracer(TracingTelemetry tracingTelemetry, ThreadContext threadContext) { - TracerContextStorage tracerContextSpanStorage = new ThreadContextSpanStorage(threadContext, tracingTelemetry); - return new DefaultTracer(tracingTelemetry, tracerContextSpanStorage); + TracerContextStorage tracerContextStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); + return new DefaultTracer(tracingTelemetry, tracerContextStorage); } private Tracer createWrappedTracer(Tracer defaultTracer) { diff --git a/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java b/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java index ebd4c9da1f5c2..7483e69fb0b0e 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/ShardMovementStrategyTests.java @@ -25,10 +25,6 @@ @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class ShardMovementStrategyTests extends OpenSearchIntegTestCase { - protected boolean addMockTelemetryPlugin() { - return false; - } - protected String startDataOnlyNode(final String zone) { final Settings settings = Settings.builder().put("node.attr.zone", zone).build(); return internalCluster().startDataOnlyNode(settings); From 1245d99b1c28b93a56d9e61bc6384a799fe68d7b Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 31 Aug 2023 02:23:11 +0530 Subject: [PATCH 07/16] Change the span::endSpan and SpanScope::close behaviour Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultSpanScope.java | 31 +++++++---- .../telemetry/tracing/DefaultTracer.java | 42 +++++---------- .../telemetry/tracing/TracingTelemetry.java | 4 +- .../telemetry/tracing/DefaultTracerTests.java | 52 ++++++++++++------- .../telemetry/tracing/OTelPropagatedSpan.java | 2 +- .../telemetry/tracing/OTelSpan.java | 22 +++++++- .../tracing/OTelTracingTelemetry.java | 9 ++-- .../telemetry/tracing/OTelSpanTests.java | 20 ++++--- .../OTelTracingContextPropagatorTests.java | 2 +- .../tracing/OTelTracingTelemetryTests.java | 10 ++-- .../test/telemetry/tracing/MockSpan.java | 41 ++++++++++++--- .../tracing/MockTracingContextPropagator.java | 2 +- .../tracing/MockTracingTelemetry.java | 5 +- 13 files changed, 151 insertions(+), 91 deletions(-) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java index 801819786d13f..a8791815006a1 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java @@ -9,31 +9,40 @@ package org.opensearch.telemetry.tracing; import java.util.Objects; -import java.util.function.Consumer; /** * Default implementation for {@link SpanScope} */ public class DefaultSpanScope implements SpanScope { private final Span span; - private final SpanScope beforeAttachedSpanScope; - private final Consumer onCloseConsumer; + private final SpanScope previousSpanScope; + private static final ThreadLocal spanScopeThreadLocal = new ThreadLocal<>(); /** * Constructor * @param span span - * @param beforeAttachedSpanScope before attached span scope. - * @param onCloseConsumer close consumer + * @param previousSpanScope before attached span scope. */ - public DefaultSpanScope(Span span, SpanScope beforeAttachedSpanScope, Consumer onCloseConsumer) { + private DefaultSpanScope(Span span, SpanScope previousSpanScope) { this.span = Objects.requireNonNull(span); - this.beforeAttachedSpanScope = beforeAttachedSpanScope; - this.onCloseConsumer = Objects.requireNonNull(onCloseConsumer); + this.previousSpanScope = previousSpanScope; + } + + /** + * Creates the SpanScope object. + * @param span span. + * @return SpanScope spanScope + */ + public static SpanScope create(Span span) { + final SpanScope beforeSpanScope = spanScopeThreadLocal.get(); + SpanScope newSpanScope = new DefaultSpanScope(span, beforeSpanScope); + spanScopeThreadLocal.set(newSpanScope); + return newSpanScope; } @Override public void close() { - onCloseConsumer.accept(beforeAttachedSpanScope); + spanScopeThreadLocal.set(previousSpanScope); } @Override @@ -41,4 +50,8 @@ public Span getSpan() { return span; } + static SpanScope getCurrentSpanScope() { + return spanScopeThreadLocal.get(); + } + } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 498764d19f978..20f38bb7350b6 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -28,7 +28,6 @@ class DefaultTracer implements Tracer { private final TracingTelemetry tracingTelemetry; private final TracerContextStorage tracerContextStorage; - private final ThreadLocal spanScopeThreadLocal = new ThreadLocal<>(); /** * Creates DefaultTracer instance @@ -83,10 +82,6 @@ public SpanContext getCurrentSpan() { return (currentSpan == null) ? null : new SpanContext(currentSpan); } - SpanScope getCurrentSpanScope() { - return spanScopeThreadLocal.get(); - } - @Override public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) { Span span = startSpan(spanCreationContext); @@ -111,38 +106,27 @@ public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanC @Override public SpanScope createSpanScope(Span span) { - SpanScope defaultSpanScope = new DefaultSpanScope(span, getCurrentSpanScope(), (spanScope) -> closeSpanScope(spanScope)); - setCurrentSpanScopeInContext(defaultSpanScope); - return defaultSpanScope; - } - - private void closeSpanScope(SpanScope beforeAttachedSpanScope) { - setCurrentSpanScopeInContext(beforeAttachedSpanScope); - if (beforeAttachedSpanScope != null) { - setCurrentSpanInContext(beforeAttachedSpanScope.getSpan()); - } else { - setCurrentSpanInContext(null); - } - } - - private void setCurrentSpanScopeInContext(SpanScope spanScope) { - spanScopeThreadLocal.set(spanScope); + return DefaultSpanScope.create(span); } private void endScopedSpan(Span span, SpanScope spanScope) { - endSpan(span); + span.endSpan(); spanScope.close(); } - private void endSpan(Span span) { - if (span != null) { - span.endSpan(); - setCurrentSpanInContext(span.getParentSpan()); - } + private Span createSpan(String spanName, Span parentSpan, Attributes attributes) { + return tracingTelemetry.createSpan(spanName, parentSpan, attributes, span -> onSpanEnd(span, parentSpan)); } - private Span createSpan(String spanName, Span parentSpan, Attributes attributes) { - return tracingTelemetry.createSpan(spanName, parentSpan, attributes); + private void onSpanEnd(Span span, Span parentSpan) { + if (parentSpan != null) { + setCurrentSpanInContext(parentSpan); + } else if (span.getParentSpan() == null) { + setCurrentSpanInContext(null); + } else { + // Most likely this scenario is where parent is coming from a calling thread + // and need not be updated. stashContext upstream would take care of this. + } } private void setCurrentSpanInContext(Span span) { diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java index 2e91cadbf395f..ccefccbe7031f 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java @@ -11,6 +11,7 @@ import org.opensearch.telemetry.tracing.attributes.Attributes; import java.io.Closeable; +import java.util.function.Consumer; /** * Interface for tracing telemetry providers @@ -24,9 +25,10 @@ public interface TracingTelemetry extends Closeable { * @param spanName name of the span * @param parentSpan span's parent span * @param attributes attributes to be added. + * @param onSpanEndConsumer consumer to be invoked on span end. * @return span instance */ - Span createSpan(String spanName, Span parentSpan, Attributes attributes); + Span createSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer); /** * provides tracing context propagator diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 77c43803960ae..e657260713848 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -16,6 +16,7 @@ import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; import java.io.IOException; +import java.util.function.Consumer; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -55,17 +56,21 @@ public void testCreateSpan() { public void testCreateSpanWithAttributesWithMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class))).thenReturn( + mockSpan + ); defaultTracer.startSpan("span_name", attributes); - verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); } public void testCreateSpanWithAttributesWithParentMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class))).thenReturn( + mockSpan + ); defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); - verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); verify(mockTracerContextStorage, never()).get(TracerContextStorage.CURRENT_SPAN); } @@ -109,9 +114,11 @@ public void testCreateSpanWithParent() { public void testCreateSpanWithContext() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan("span_name", mockParentSpan, attributes)).thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class))).thenReturn( + mockSpan + ); defaultTracer.startSpan(new SpanCreationContext("span_name", attributes)); - verify(mockTracingTelemetry).createSpan("span_name", mockParentSpan, attributes); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); } public void testCreateSpanWithNullParent() { @@ -138,11 +145,11 @@ public void testEndSpanByClosingScopedSpan() { DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); ScopedSpan scopedSpan = defaultTracer.startScopedSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(((DefaultScopedSpan) scopedSpan).getSpanScope(), defaultTracer.getCurrentSpanScope()); + assertEquals(((DefaultScopedSpan) scopedSpan).getSpanScope(), DefaultSpanScope.getCurrentSpanScope()); scopedSpan.close(); assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan).getSpan()).hasEnded()); assertEquals(null, defaultTracer.getCurrentSpan()); - assertEquals(null, defaultTracer.getCurrentSpanScope()); + assertEquals(null, DefaultSpanScope.getCurrentSpanScope()); } @@ -158,17 +165,17 @@ public void testEndSpanByClosingScopedSpanMultiple() { ScopedSpan scopedSpan1 = defaultTracer.startScopedSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(((DefaultScopedSpan) scopedSpan1).getSpanScope(), defaultTracer.getCurrentSpanScope()); + assertEquals(((DefaultScopedSpan) scopedSpan1).getSpanScope(), DefaultSpanScope.getCurrentSpanScope()); scopedSpan1.close(); assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan1).getSpan()).hasEnded()); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(((DefaultScopedSpan) scopedSpan).getSpanScope(), defaultTracer.getCurrentSpanScope()); + assertEquals(((DefaultScopedSpan) scopedSpan).getSpanScope(), DefaultSpanScope.getCurrentSpanScope()); scopedSpan.close(); assertTrue(((MockSpan) ((DefaultScopedSpan) scopedSpan).getSpan()).hasEnded()); assertEquals(null, defaultTracer.getCurrentSpan()); - assertEquals(null, defaultTracer.getCurrentSpanScope()); + assertEquals(null, DefaultSpanScope.getCurrentSpanScope()); } @@ -183,11 +190,12 @@ public void testEndSpanByClosingSpanScope() { Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); SpanScope spanScope = defaultTracer.createSpanScope(span); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(spanScope, defaultTracer.getCurrentSpanScope()); + assertEquals(spanScope, DefaultSpanScope.getCurrentSpanScope()); + span.endSpan(); spanScope.close(); assertEquals(null, defaultTracer.getCurrentSpan()); - assertFalse(((MockSpan) span).hasEnded()); + assertTrue(((MockSpan) span).hasEnded()); } @@ -204,20 +212,22 @@ public void testEndSpanByClosingSpanScopeMultiple() { SpanScope spanScope = defaultTracer.createSpanScope(span); SpanScope spanScope1 = defaultTracer.createSpanScope(span1); assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(spanScope1, defaultTracer.getCurrentSpanScope()); + assertEquals(spanScope1, DefaultSpanScope.getCurrentSpanScope()); + span1.endSpan(); spanScope1.close(); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); - assertEquals(spanScope, defaultTracer.getCurrentSpanScope()); - assertFalse(((MockSpan) span1).hasEnded()); + assertEquals(spanScope, DefaultSpanScope.getCurrentSpanScope()); + assertTrue(((MockSpan) span1).hasEnded()); assertFalse(((MockSpan) span).hasEnded()); + span.endSpan(); spanScope.close(); assertEquals(null, defaultTracer.getCurrentSpan()); - assertEquals(null, defaultTracer.getCurrentSpanScope()); - assertFalse(((MockSpan) span).hasEnded()); - assertFalse(((MockSpan) span1).hasEnded()); + assertEquals(null, DefaultSpanScope.getCurrentSpanScope()); + assertTrue(((MockSpan) span).hasEnded()); + assertTrue(((MockSpan) span1).hasEnded()); } @@ -243,6 +253,8 @@ private void setupMocks() { when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); when(mockTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class))).thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class), any(Consumer.class))).thenReturn( + mockSpan + ); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java index 5aa1069e60367..40ff99588b097 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java @@ -18,6 +18,6 @@ public class OTelPropagatedSpan extends OTelSpan { * @param span otel propagated span */ public OTelPropagatedSpan(io.opentelemetry.api.trace.Span span) { - super(null, span, null); + super(null, span, null, a -> {}); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java index ba63df4ae47a1..1ffd5c10cd8bd 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java @@ -8,6 +8,9 @@ package org.opensearch.telemetry.tracing; +import java.util.Objects; +import java.util.function.Consumer; + import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.StatusCode; @@ -18,15 +21,30 @@ class OTelSpan extends AbstractSpan { private final Span delegateSpan; - - public OTelSpan(String spanName, Span span, org.opensearch.telemetry.tracing.Span parentSpan) { + private final Consumer onSpanEndConsumer; + + /** + * Constructor + * @param spanName + * @param span + * @param parentSpan + * @param onSpanEndConsumer + */ + public OTelSpan( + String spanName, + Span span, + org.opensearch.telemetry.tracing.Span parentSpan, + Consumer onSpanEndConsumer + ) { super(spanName, parentSpan); this.delegateSpan = span; + this.onSpanEndConsumer = Objects.requireNonNull(onSpanEndConsumer); } @Override public void endSpan() { delegateSpan.end(); + onSpanEndConsumer.accept(this); } @Override diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 9a3a10e63503e..9878705671af2 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -14,6 +14,7 @@ import java.io.Closeable; import java.io.IOException; +import java.util.function.Consumer; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; @@ -47,8 +48,8 @@ public void close() { } @Override - public Span createSpan(String spanName, Span parentSpan, Attributes attributes) { - return createOtelSpan(spanName, parentSpan, attributes); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer) { + return createOtelSpan(spanName, parentSpan, attributes, onSpanEndConsumer); } @Override @@ -56,9 +57,9 @@ public TracingContextPropagator getContextPropagator() { return new OTelTracingContextPropagator(openTelemetry); } - private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes) { + private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer) { io.opentelemetry.api.trace.Span otelSpan = otelSpan(spanName, parentSpan, OTelAttributesConverter.convert(attributes)); - return new OTelSpan(spanName, otelSpan, parentSpan); + return new OTelSpan(spanName, otelSpan, parentSpan, onSpanEndConsumer); } io.opentelemetry.api.trace.Span otelSpan(String spanName, Span parentOTelSpan, io.opentelemetry.api.common.Attributes attributes) { diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java index fc92ab36908e1..acef528a2d6f5 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java @@ -10,6 +10,8 @@ import org.opensearch.test.OpenSearchTestCase; +import java.util.concurrent.atomic.AtomicBoolean; + import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.TraceFlags; @@ -26,14 +28,16 @@ public class OTelSpanTests extends OpenSearchTestCase { public void testEndSpanTest() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + final AtomicBoolean onSpanEndTestFlag = new AtomicBoolean(false); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> { onSpanEndTestFlag.set(true); }); oTelSpan.endSpan(); + assertTrue(onSpanEndTestFlag.get()); verify(mockSpan).end(); } public void testAddAttributeString() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); oTelSpan.addAttribute("key", "value"); verify(mockSpan).setAttribute("key", "value"); @@ -41,7 +45,7 @@ public void testAddAttributeString() { public void testAddAttributeLong() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); oTelSpan.addAttribute("key", 1L); verify(mockSpan).setAttribute("key", 1L); @@ -49,7 +53,7 @@ public void testAddAttributeLong() { public void testAddAttributeDouble() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); oTelSpan.addAttribute("key", 1.0); verify(mockSpan).setAttribute("key", 1.0); @@ -57,7 +61,7 @@ public void testAddAttributeDouble() { public void testAddAttributeBoolean() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); oTelSpan.addAttribute("key", true); verify(mockSpan).setAttribute("key", true); @@ -65,7 +69,7 @@ public void testAddAttributeBoolean() { public void testAddEvent() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); oTelSpan.addEvent("eventName"); verify(mockSpan).addEvent("eventName"); @@ -73,14 +77,14 @@ public void testAddEvent() { public void testGetTraceId() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); assertEquals(TRACE_ID, oTelSpan.getTraceId()); } public void testGetSpanId() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); assertEquals(SPAN_ID, oTelSpan.getSpanId()); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java index 16a3ec9493d5d..25948284a97fb 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java @@ -35,7 +35,7 @@ public class OTelTracingContextPropagatorTests extends OpenSearchTestCase { public void testAddTracerContextToHeader() { Span mockSpan = mock(Span.class); when(mockSpan.getSpanContext()).thenReturn(SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())); - OTelSpan span = new OTelSpan("spanName", mockSpan, null); + OTelSpan span = new OTelSpan("spanName", mockSpan, null, a -> {}); Map requestHeaders = new HashMap<>(); OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); when(mockOpenTelemetry.getPropagators()).thenReturn(ContextPropagators.create(W3CTraceContextPropagator.getInstance())); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 3f46cb621a8ec..88575dadcac41 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -38,7 +38,7 @@ public void testCreateSpanWithoutParent() { Map attributeMap = Collections.singletonMap("name", "value"); Attributes attributes = Attributes.create().addAttribute("name", "value"); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); - Span span = tracingTelemetry.createSpan("span_name", null, attributes); + Span span = tracingTelemetry.createSpan("span_name", null, attributes, a -> {}); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); @@ -55,11 +55,11 @@ public void testCreateSpanWithParent() { when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, a -> {}); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); Attributes attributes = Attributes.create().addAttribute("name", 1l); - Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, a -> {}); verify(mockSpanBuilder).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttributeLong(attributes)); @@ -78,7 +78,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, a -> {}); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); Attributes attributes = Attributes.create() @@ -86,7 +86,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { .addAttribute("key2", 2.0) .addAttribute("key3", true) .addAttribute("key4", "key4"); - Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, a -> {}); io.opentelemetry.api.common.Attributes otelAttributes = io.opentelemetry.api.common.Attributes.builder() .put("key1", 1l) diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java index c22a395a6e17c..bbc943a05b504 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java @@ -14,8 +14,10 @@ import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.concurrent.ThreadLocalRandom; +import java.util.function.Consumer; import java.util.function.Supplier; /** @@ -34,21 +36,31 @@ public class MockSpan extends AbstractSpan { private static final Supplier randomSupplier = ThreadLocalRandom::current; + private final Consumer onSpanEndConsumer; + /** * Base Constructor. - * @param spanName span name - * @param parentSpan parent span - * @param spanProcessor span processor + * @param spanName Span Name + * @param parentSpan Parent Span + * @param spanProcessor Span Processor * @param attributes attributes + * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, Attributes attributes) { + public MockSpan( + String spanName, + Span parentSpan, + SpanProcessor spanProcessor, + Attributes attributes, + Consumer onSpanEndConsumer + ) { this( spanName, parentSpan, parentSpan != null ? parentSpan.getTraceId() : IdGenerator.generateTraceId(), IdGenerator.generateSpanId(), spanProcessor, - attributes + attributes, + onSpanEndConsumer ); } @@ -57,15 +69,17 @@ public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, A * @param spanName span name. * @param parentSpan parent span name * @param spanProcessor span processor. + * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor) { + public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, Consumer onSpanEndConsumer) { this( spanName, parentSpan, parentSpan != null ? parentSpan.getTraceId() : IdGenerator.generateTraceId(), IdGenerator.generateSpanId(), spanProcessor, - Attributes.EMPTY + Attributes.EMPTY, + onSpanEndConsumer ); } @@ -77,8 +91,17 @@ public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor) { * @param spanId Span ID * @param spanProcessor Span Processor * @param attributes attributes + * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan(String spanName, Span parentSpan, String traceId, String spanId, SpanProcessor spanProcessor, Attributes attributes) { + public MockSpan( + String spanName, + Span parentSpan, + String traceId, + String spanId, + SpanProcessor spanProcessor, + Attributes attributes, + Consumer onSpanEndConsumer + ) { super(spanName, parentSpan); this.spanProcessor = spanProcessor; this.metadata = new HashMap<>(); @@ -88,6 +111,7 @@ public MockSpan(String spanName, Span parentSpan, String traceId, String spanId, if (attributes != null) { this.metadata.putAll(attributes.getAttributesMap()); } + this.onSpanEndConsumer = Objects.requireNonNull(onSpanEndConsumer); } @Override @@ -96,6 +120,7 @@ public void endSpan() { if (hasEnded) { return; } + onSpanEndConsumer.accept(this); endTime = System.nanoTime(); hasEnded = true; } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java index 7525b4424c243..dff46880a3e69 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java @@ -44,7 +44,7 @@ public Optional extract(Map props) { String[] values = value.split(SEPARATOR); String traceId = values[0]; String spanId = values[1]; - return Optional.of(new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY)); + return Optional.of(new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY, (a) -> {})); } else { return Optional.empty(); } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java index 9b958bbb40f84..250c4960ccf9e 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java @@ -17,6 +17,7 @@ import java.util.Arrays; import java.util.List; +import java.util.function.Consumer; /** * Mock {@link TracingTelemetry} implementation for testing. @@ -33,8 +34,8 @@ public MockTracingTelemetry() { } @Override - public Span createSpan(String spanName, Span parentSpan, Attributes attributes) { - Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer) { + Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes, onSpanEndConsumer); spanProcessor.onStart(span); return span; } From 137747fe13fa823d14bfe08324bf0828dd8657af Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 31 Aug 2023 02:45:39 +0530 Subject: [PATCH 08/16] Supressed warnings Signed-off-by: Gagan Juneja --- .../org/opensearch/telemetry/tracing/DefaultTracerTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index e657260713848..584e344d6f91f 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -53,6 +53,7 @@ public void testCreateSpan() { assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); } + @SuppressWarnings("unchecked") public void testCreateSpanWithAttributesWithMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); @@ -63,6 +64,7 @@ public void testCreateSpanWithAttributesWithMock() { verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); } + @SuppressWarnings("unchecked") public void testCreateSpanWithAttributesWithParentMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); @@ -111,6 +113,7 @@ public void testCreateSpanWithParent() { assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan().getParentSpan()); } + @SuppressWarnings("unchecked") public void testCreateSpanWithContext() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); From b479ffcbecd1ef64a60b1342494a367c35d24f2e Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 31 Aug 2023 16:27:14 +0530 Subject: [PATCH 09/16] Add more test cases Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultTracerTests.java | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 584e344d6f91f..2c278c835d38c 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -10,12 +10,16 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.node.Node; import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.telemetry.tracing.MockSpan; import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; +import org.opensearch.threadpool.ThreadPool; import java.io.IOException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import static org.mockito.ArgumentMatchers.any; @@ -33,16 +37,22 @@ public class DefaultTracerTests extends OpenSearchTestCase { private Span mockParentSpan; private SpanScope mockSpanScope; + private ThreadPool threadPool; + private ExecutorService executorService; @Override public void setUp() throws Exception { super.setUp(); + threadPool = new ThreadPool(Settings.builder().put(Node.NODE_NAME_SETTING.getKey(), "default tracer tests").build()); + executorService = threadPool.executor(ThreadPool.Names.GENERIC); setupMocks(); } @Override public void tearDown() throws Exception { super.tearDown(); + executorService.shutdown(); + threadPool.shutdownNow(); } public void testCreateSpan() { @@ -234,6 +244,116 @@ public void testEndSpanByClosingSpanScopeMultiple() { } + /** + * 1. CreateSpan in ThreadA (NotScopedSpan) + * 2. create Async task and pass the span + * 3. Scope.close + * 4. verify the current_span is still the same on async thread as the 2 + * 5. verify the main thread has current span as null. + */ + public void testSpanAcrossThreads() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + AtomicReference currentSpanRefThread1 = new AtomicReference<>(); + AtomicReference currentSpanRefThread2 = new AtomicReference<>(); + AtomicReference currentSpanRefAfterEndThread2 = new AtomicReference<>(); + + AtomicReference spanRef = new AtomicReference<>(); + AtomicReference spanT2Ref = new AtomicReference<>(); + + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); + + executorService.execute(() -> { + // create a span + Span span = defaultTracer.startSpan(new SpanCreationContext("span_name_t_1", Attributes.EMPTY)); + SpanScope spanScope = defaultTracer.createSpanScope(span); + spanRef.set(span); + + executorService.execute(() -> { + Span spanT2 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); + SpanScope spanScopeT2 = defaultTracer.createSpanScope(spanT2); + spanT2Ref.set(spanT2); + + currentSpanRefThread2.set(defaultTracer.getCurrentSpan().getSpan()); + + spanT2.endSpan(); + spanScopeT2.close(); + currentSpanRefAfterEndThread2.set(defaultTracer.getCurrentSpan() != null ? defaultTracer.getCurrentSpan().getSpan() : null); + }); + spanScope.close(); + currentSpanRefThread1.set(defaultTracer.getCurrentSpan().getSpan()); + }); + assertEquals(spanT2Ref.get(), currentSpanRefThread2.get()); + assertEquals(spanRef.get(), currentSpanRefAfterEndThread2.get()); + assertEquals(null, currentSpanRefThread1.get()); + } + + /** + * 1. CreateSpan in ThreadA (NotScopedSpan) + * 2. create Async task and pass the span + * 3. Inside Async task start a new span. + * 4. Scope.close + * 5. Parent Scope.close + * 6. verify the current_span is still the same on async thread as the 2 + * 7. verify the main thread has current span as null. + */ + public void testSpanAcrossThreadsMultipleSpans() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + AtomicReference currentSpanRefThread1 = new AtomicReference<>(); + AtomicReference currentSpanRefThread2 = new AtomicReference<>(); + AtomicReference currentSpanRefAfterEndThread2 = new AtomicReference<>(); + + AtomicReference parentSpanRef = new AtomicReference<>(); + AtomicReference spanRef = new AtomicReference<>(); + AtomicReference spanT2Ref = new AtomicReference<>(); + + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); + + executorService.execute(() -> { + // create a parent span + Span parentSpan = defaultTracer.startSpan(new SpanCreationContext("p_span_name", Attributes.EMPTY)); + SpanScope parentSpanScope = defaultTracer.createSpanScope(parentSpan); + parentSpanRef.set(parentSpan); + // create a span + Span span = defaultTracer.startSpan(new SpanCreationContext("span_name_t_1", Attributes.EMPTY)); + SpanScope spanScope = defaultTracer.createSpanScope(span); + spanRef.set(span); + + executorService.execute(() -> { + Span spanT2 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); + SpanScope spanScopeT2 = defaultTracer.createSpanScope(spanT2); + spanT2Ref.set(spanT2); + + Span spanT21 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); + SpanScope spanScopeT21 = defaultTracer.createSpanScope(spanT2); + + currentSpanRefThread2.set(defaultTracer.getCurrentSpan().getSpan()); + + spanT21.endSpan(); + spanScopeT21.close(); + + spanT2.endSpan(); + spanScopeT2.close(); + currentSpanRefAfterEndThread2.set(defaultTracer.getCurrentSpan() != null ? defaultTracer.getCurrentSpan().getSpan() : null); + }); + spanScope.close(); + parentSpanScope.close(); + currentSpanRefThread1.set(defaultTracer.getCurrentSpan().getSpan()); + }); + assertEquals(spanT2Ref.get(), currentSpanRefThread2.get()); + assertEquals(spanRef.get(), currentSpanRefAfterEndThread2.get()); + assertEquals(null, currentSpanRefThread1.get()); + } + public void testClose() throws IOException { Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); From c6ccae3371b90aca6027ff71f6194dcc9ea3cf6e Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 31 Aug 2023 23:26:03 +0530 Subject: [PATCH 10/16] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultScopedSpan.java | 21 ++++++------- .../telemetry/tracing/DefaultTracer.java | 25 ++++------------ .../telemetry/tracing/ScopedSpan.java | 10 +++---- .../opensearch/telemetry/tracing/Tracer.java | 2 +- .../tracing/noop/NoopScopedSpan.java | 10 +++---- .../telemetry/tracing/noop/NoopTracer.java | 2 +- .../tracing/DefaultScopedSpanTests.java | 30 +++++++++---------- .../telemetry/tracing/DefaultTracerTests.java | 18 +++++------ .../telemetry/tracing/WrappedTracer.java | 4 +-- .../telemetry/tracing/TracerFactoryTests.java | 2 +- 10 files changed, 52 insertions(+), 72 deletions(-) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java index b68b0e7f848b5..87802b1f1931d 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultScopedSpan.java @@ -9,7 +9,6 @@ package org.opensearch.telemetry.tracing; import java.util.Objects; -import java.util.function.BiConsumer; /** * Default implementation of Scope @@ -22,42 +21,39 @@ final class DefaultScopedSpan implements ScopedSpan { private final SpanScope spanScope; - private final BiConsumer onCloseConsumer; - /** * Creates Scope instance for the given span * * @param span underlying span - * @param onCloseConsumer consumer to execute on scope close + * @param spanScope span scope. */ - public DefaultScopedSpan(Span span, SpanScope spanScope, BiConsumer onCloseConsumer) { + public DefaultScopedSpan(Span span, SpanScope spanScope) { this.span = Objects.requireNonNull(span); this.spanScope = Objects.requireNonNull(spanScope); - this.onCloseConsumer = Objects.requireNonNull(onCloseConsumer); } @Override - public void addSpanAttribute(String key, String value) { + public void addAttribute(String key, String value) { span.addAttribute(key, value); } @Override - public void addSpanAttribute(String key, long value) { + public void addAttribute(String key, long value) { span.addAttribute(key, value); } @Override - public void addSpanAttribute(String key, double value) { + public void addAttribute(String key, double value) { span.addAttribute(key, value); } @Override - public void addSpanAttribute(String key, boolean value) { + public void addAttribute(String key, boolean value) { span.addAttribute(key, value); } @Override - public void addSpanEvent(String event) { + public void addEvent(String event) { span.addEvent(event); } @@ -71,7 +67,8 @@ public void setError(Exception exception) { */ @Override public void close() { - onCloseConsumer.accept(span, spanScope); + span.endSpan(); + spanScope.close(); } /** diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 20f38bb7350b6..c4f77c1318e3f 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -84,42 +84,27 @@ public SpanContext getCurrentSpan() { @Override public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext) { - Span span = startSpan(spanCreationContext); - SpanScope spanScope = createSpanScope(span); - return new DefaultScopedSpan( - span, - spanScope, - (spanToBeClosed, scopeSpanToBeClosed) -> endScopedSpan(spanToBeClosed, scopeSpanToBeClosed) - ); + return startScopedSpan(spanCreationContext, null); } @Override public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanContext parentSpan) { Span span = startSpan(spanCreationContext.getSpanName(), parentSpan, spanCreationContext.getAttributes()); - SpanScope spanScope = createSpanScope(span); - return new DefaultScopedSpan( - span, - spanScope, - (spanToBeClosed, scopeSpanToBeClosed) -> endScopedSpan(spanToBeClosed, scopeSpanToBeClosed) - ); + SpanScope spanScope = withSpanInScope(span); + return new DefaultScopedSpan(span, spanScope); } @Override - public SpanScope createSpanScope(Span span) { + public SpanScope withSpanInScope(Span span) { return DefaultSpanScope.create(span); } - private void endScopedSpan(Span span, SpanScope spanScope) { - span.endSpan(); - spanScope.close(); - } - private Span createSpan(String spanName, Span parentSpan, Attributes attributes) { return tracingTelemetry.createSpan(spanName, parentSpan, attributes, span -> onSpanEnd(span, parentSpan)); } private void onSpanEnd(Span span, Span parentSpan) { - if (parentSpan != null) { + if (getCurrentSpanInternal() == span && parentSpan != null) { setCurrentSpanInContext(parentSpan); } else if (span.getParentSpan() == null) { setCurrentSpanInContext(null); diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java index 0100960e0e793..833a85ef27baf 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/ScopedSpan.java @@ -26,7 +26,7 @@ public interface ScopedSpan extends AutoCloseable { * @param key attribute key * @param value attribute value */ - void addSpanAttribute(String key, String value); + void addAttribute(String key, String value); /** * Adds long attribute to the {@link Span}. @@ -34,7 +34,7 @@ public interface ScopedSpan extends AutoCloseable { * @param key attribute key * @param value attribute value */ - void addSpanAttribute(String key, long value); + void addAttribute(String key, long value); /** * Adds double attribute to the {@link Span}. @@ -42,7 +42,7 @@ public interface ScopedSpan extends AutoCloseable { * @param key attribute key * @param value attribute value */ - void addSpanAttribute(String key, double value); + void addAttribute(String key, double value); /** * Adds boolean attribute to the {@link Span}. @@ -50,14 +50,14 @@ public interface ScopedSpan extends AutoCloseable { * @param key attribute key * @param value attribute value */ - void addSpanAttribute(String key, boolean value); + void addAttribute(String key, boolean value); /** * Adds an event to the {@link Span}. * * @param event event name */ - void addSpanEvent(String event); + void addEvent(String event); /** * Records error in the span diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java index d01323146dd2c..dc247e45d77f6 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java @@ -85,6 +85,6 @@ public interface Tracer extends HttpTracer, Closeable { * @param span span to be scoped * @return ScopedSpan */ - SpanScope createSpanScope(Span span); + SpanScope withSpanInScope(Span span); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java index b22e75bf37011..22ee7e7ccb6fb 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopScopedSpan.java @@ -23,27 +23,27 @@ public final class NoopScopedSpan implements ScopedSpan { public NoopScopedSpan() {} @Override - public void addSpanAttribute(String key, String value) { + public void addAttribute(String key, String value) { } @Override - public void addSpanAttribute(String key, long value) { + public void addAttribute(String key, long value) { } @Override - public void addSpanAttribute(String key, double value) { + public void addAttribute(String key, double value) { } @Override - public void addSpanAttribute(String key, boolean value) { + public void addAttribute(String key, boolean value) { } @Override - public void addSpanEvent(String event) { + public void addEvent(String event) { } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java index c704e42867d78..a9eee725b4f1c 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java @@ -69,7 +69,7 @@ public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanC } @Override - public SpanScope createSpanScope(Span span) { + public SpanScope withSpanInScope(Span span) { return SpanScope.NO_OP; } diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java index 3119b4a4ef827..1d4871fe1419e 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultScopedSpanTests.java @@ -10,8 +10,6 @@ import org.opensearch.test.OpenSearchTestCase; -import java.util.function.BiConsumer; - import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -21,18 +19,18 @@ public class DefaultScopedSpanTests extends OpenSearchTestCase { public void testClose() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - BiConsumer mockConsumer = mock(BiConsumer.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, mockConsumer); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); defaultSpanScope.close(); - verify(mockConsumer).accept(mockSpan, mockSpanScope); + verify(mockSpan).endSpan(); + verify(mockSpanScope).close(); } public void testAddSpanAttributeString() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); - defaultSpanScope.addSpanAttribute("key", "value"); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); + defaultSpanScope.addAttribute("key", "value"); verify(mockSpan).addAttribute("key", "value"); } @@ -40,8 +38,8 @@ public void testAddSpanAttributeString() { public void testAddSpanAttributeLong() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); - defaultSpanScope.addSpanAttribute("key", 1L); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); + defaultSpanScope.addAttribute("key", 1L); verify(mockSpan).addAttribute("key", 1L); } @@ -49,8 +47,8 @@ public void testAddSpanAttributeLong() { public void testAddSpanAttributeDouble() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); - defaultSpanScope.addSpanAttribute("key", 1.0); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); + defaultSpanScope.addAttribute("key", 1.0); verify(mockSpan).addAttribute("key", 1.0); } @@ -58,8 +56,8 @@ public void testAddSpanAttributeDouble() { public void testAddSpanAttributeBoolean() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); - defaultSpanScope.addSpanAttribute("key", true); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); + defaultSpanScope.addAttribute("key", true); verify(mockSpan).addAttribute("key", true); } @@ -67,8 +65,8 @@ public void testAddSpanAttributeBoolean() { public void testAddEvent() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); - defaultSpanScope.addSpanEvent("eventName"); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); + defaultSpanScope.addEvent("eventName"); verify(mockSpan).addEvent("eventName"); } @@ -76,7 +74,7 @@ public void testAddEvent() { public void testSetError() { Span mockSpan = mock(Span.class); SpanScope mockSpanScope = mock(SpanScope.class); - DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope, (a, b) -> {}); + DefaultScopedSpan defaultSpanScope = new DefaultScopedSpan(mockSpan, mockSpanScope); Exception ex = new Exception("error"); defaultSpanScope.setError(ex); diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 2c278c835d38c..228a00d709695 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -201,7 +201,7 @@ public void testEndSpanByClosingSpanScope() { ); DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); - SpanScope spanScope = defaultTracer.createSpanScope(span); + SpanScope spanScope = defaultTracer.withSpanInScope(span); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); assertEquals(spanScope, DefaultSpanScope.getCurrentSpanScope()); @@ -222,8 +222,8 @@ public void testEndSpanByClosingSpanScopeMultiple() { DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); Span span = defaultTracer.startSpan(new SpanCreationContext("span_name", Attributes.EMPTY)); Span span1 = defaultTracer.startSpan(new SpanCreationContext("span_name_1", Attributes.EMPTY)); - SpanScope spanScope = defaultTracer.createSpanScope(span); - SpanScope spanScope1 = defaultTracer.createSpanScope(span1); + SpanScope spanScope = defaultTracer.withSpanInScope(span); + SpanScope spanScope1 = defaultTracer.withSpanInScope(span1); assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); assertEquals(spanScope1, DefaultSpanScope.getCurrentSpanScope()); @@ -270,12 +270,12 @@ public void testSpanAcrossThreads() { executorService.execute(() -> { // create a span Span span = defaultTracer.startSpan(new SpanCreationContext("span_name_t_1", Attributes.EMPTY)); - SpanScope spanScope = defaultTracer.createSpanScope(span); + SpanScope spanScope = defaultTracer.withSpanInScope(span); spanRef.set(span); executorService.execute(() -> { Span spanT2 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); - SpanScope spanScopeT2 = defaultTracer.createSpanScope(spanT2); + SpanScope spanScopeT2 = defaultTracer.withSpanInScope(spanT2); spanT2Ref.set(spanT2); currentSpanRefThread2.set(defaultTracer.getCurrentSpan().getSpan()); @@ -321,20 +321,20 @@ public void testSpanAcrossThreadsMultipleSpans() { executorService.execute(() -> { // create a parent span Span parentSpan = defaultTracer.startSpan(new SpanCreationContext("p_span_name", Attributes.EMPTY)); - SpanScope parentSpanScope = defaultTracer.createSpanScope(parentSpan); + SpanScope parentSpanScope = defaultTracer.withSpanInScope(parentSpan); parentSpanRef.set(parentSpan); // create a span Span span = defaultTracer.startSpan(new SpanCreationContext("span_name_t_1", Attributes.EMPTY)); - SpanScope spanScope = defaultTracer.createSpanScope(span); + SpanScope spanScope = defaultTracer.withSpanInScope(span); spanRef.set(span); executorService.execute(() -> { Span spanT2 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); - SpanScope spanScopeT2 = defaultTracer.createSpanScope(spanT2); + SpanScope spanScopeT2 = defaultTracer.withSpanInScope(spanT2); spanT2Ref.set(spanT2); Span spanT21 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); - SpanScope spanScopeT21 = defaultTracer.createSpanScope(spanT2); + SpanScope spanScopeT21 = defaultTracer.withSpanInScope(spanT2); currentSpanRefThread2.set(defaultTracer.getCurrentSpan().getSpan()); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java index 39d76a0384084..1fb1eed98f5bb 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java @@ -69,8 +69,8 @@ public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanC } @Override - public SpanScope createSpanScope(Span span) { - return getDelegateTracer().createSpanScope(span); + public SpanScope withSpanInScope(Span span) { + return getDelegateTracer().withSpanInScope(span); } @Override diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java index 7b1623435fabf..f202be70c9425 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java @@ -51,7 +51,7 @@ public void testGetTracerWithUnavailableTracingTelemetryReturnsNoopTracer() { assertTrue(tracer.startSpan("foo") == NoopSpan.INSTANCE); assertTrue(tracer.startScopedSpan(new SpanCreationContext("foo", Attributes.EMPTY)) == ScopedSpan.NO_OP); assertTrue(tracer.startScopedSpan(new SpanCreationContext("foo", Attributes.EMPTY)) == ScopedSpan.NO_OP); - assertTrue(tracer.createSpanScope(tracer.startSpan("foo")) == SpanScope.NO_OP); + assertTrue(tracer.withSpanInScope(tracer.startSpan("foo")) == SpanScope.NO_OP); } public void testGetTracerWithUnavailableTracingTelemetry() { From 8805a1da809844723f49094f0e74201601c01aa9 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Fri, 1 Sep 2023 02:01:16 +0530 Subject: [PATCH 11/16] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultSpanScope.java | 23 ++++- .../telemetry/tracing/DefaultTracer.java | 14 ++- .../tracing/SpanLifecycleListener.java | 26 ++++++ .../telemetry/tracing/SpanScope.java | 6 ++ .../telemetry/tracing/TracingTelemetry.java | 12 +-- .../telemetry/tracing/noop/NoopSpanScope.java | 5 ++ .../telemetry/tracing/DefaultTracerTests.java | 90 +++++++++++++------ .../telemetry/tracing/OTelPropagatedSpan.java | 12 ++- .../telemetry/tracing/OTelSpan.java | 11 ++- .../tracing/OTelTracingTelemetry.java | 11 +-- .../telemetry/tracing/OTelSpanTests.java | 40 +++++++-- .../OTelTracingContextPropagatorTests.java | 16 +++- .../tracing/OTelTracingTelemetryTests.java | 24 +++-- .../test/telemetry/tracing/MockSpan.java | 14 +-- .../tracing/MockTracingContextPropagator.java | 17 +++- .../tracing/MockTracingTelemetry.java | 7 +- 16 files changed, 254 insertions(+), 74 deletions(-) create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java index a8791815006a1..b5e106af357c3 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java @@ -17,15 +17,17 @@ public class DefaultSpanScope implements SpanScope { private final Span span; private final SpanScope previousSpanScope; private static final ThreadLocal spanScopeThreadLocal = new ThreadLocal<>(); + private final TracerContextStorage tracerContextStorage; /** * Constructor * @param span span * @param previousSpanScope before attached span scope. */ - private DefaultSpanScope(Span span, SpanScope previousSpanScope) { + private DefaultSpanScope(Span span, SpanScope previousSpanScope, TracerContextStorage tracerContextStorage) { this.span = Objects.requireNonNull(span); this.previousSpanScope = previousSpanScope; + this.tracerContextStorage = tracerContextStorage; } /** @@ -33,18 +35,33 @@ private DefaultSpanScope(Span span, SpanScope previousSpanScope) { * @param span span. * @return SpanScope spanScope */ - public static SpanScope create(Span span) { + public static SpanScope create(Span span, TracerContextStorage tracerContextStorage) { final SpanScope beforeSpanScope = spanScopeThreadLocal.get(); - SpanScope newSpanScope = new DefaultSpanScope(span, beforeSpanScope); + SpanScope newSpanScope = new DefaultSpanScope(span, beforeSpanScope, tracerContextStorage); spanScopeThreadLocal.set(newSpanScope); return newSpanScope; } @Override public void close() { + detach(); spanScopeThreadLocal.set(previousSpanScope); } + @Override + public SpanScope attach() { + tracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, this.span); + return this; + } + + private void detach() { + if (previousSpanScope != null) { + tracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, previousSpanScope.getSpan()); + } else { + tracerContextStorage.put(TracerContextStorage.CURRENT_SPAN, null); + } + } + @Override public Span getSpan() { return span; diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index c4f77c1318e3f..61bc30601063f 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -96,11 +96,21 @@ public ScopedSpan startScopedSpan(SpanCreationContext spanCreationContext, SpanC @Override public SpanScope withSpanInScope(Span span) { - return DefaultSpanScope.create(span); + return DefaultSpanScope.create(span, tracerContextStorage).attach(); } private Span createSpan(String spanName, Span parentSpan, Attributes attributes) { - return tracingTelemetry.createSpan(spanName, parentSpan, attributes, span -> onSpanEnd(span, parentSpan)); + return tracingTelemetry.createSpan(spanName, parentSpan, attributes, new SpanLifecycleListener() { + @Override + public void onStart(Span span) { + setCurrentSpanInContext(span); + } + + @Override + public void onEnd(Span span) { + onSpanEnd(span, parentSpan); + } + }); } private void onSpanEnd(Span span, Span parentSpan) { diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java new file mode 100644 index 0000000000000..a5aae3660fc47 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java @@ -0,0 +1,26 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing; + +/** + * A listener for Span Lifecycle + */ +public interface SpanLifecycleListener { + /** + * On Span start + * @param span span + */ + void onStart(Span span); + + /** + * On span end + * @param span span + */ + void onEnd(Span span); +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java index 0ce7bf3f65ccd..99c27b72fe1c7 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanScope.java @@ -23,6 +23,12 @@ public interface SpanScope extends AutoCloseable { @Override void close(); + /** + * Attaches span to the {@link SpanScope} + * @return spanScope + */ + SpanScope attach(); + /** * Returns span attached with the {@link SpanScope} * @return span. diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java index ccefccbe7031f..34fa8376a0098 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java @@ -11,7 +11,6 @@ import org.opensearch.telemetry.tracing.attributes.Attributes; import java.io.Closeable; -import java.util.function.Consumer; /** * Interface for tracing telemetry providers @@ -22,13 +21,14 @@ public interface TracingTelemetry extends Closeable { /** * Creates span with provided arguments - * @param spanName name of the span - * @param parentSpan span's parent span - * @param attributes attributes to be added. - * @param onSpanEndConsumer consumer to be invoked on span end. + * + * @param spanName name of the span + * @param parentSpan span's parent span + * @param attributes attributes to be added. + * @param spanLifecycleListener consumer to be invoked on span end. * @return span instance */ - Span createSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer); + Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener); /** * provides tracing context propagator diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java index b1094ebb71731..a9b72adeeda0e 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopSpanScope.java @@ -27,6 +27,11 @@ public void close() { } + @Override + public SpanScope attach() { + return this; + } + @Override public Span getSpan() { return NoopSpan.INSTANCE; diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 228a00d709695..9ebf1751061f9 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -10,6 +10,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.core.action.ActionListener; import org.opensearch.node.Node; import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; @@ -20,7 +21,6 @@ import java.io.IOException; import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -67,22 +67,20 @@ public void testCreateSpan() { public void testCreateSpanWithAttributesWithMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class))).thenReturn( - mockSpan - ); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class))) + .thenReturn(mockSpan); defaultTracer.startSpan("span_name", attributes); - verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class)); } @SuppressWarnings("unchecked") public void testCreateSpanWithAttributesWithParentMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class))).thenReturn( - mockSpan - ); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class))) + .thenReturn(mockSpan); defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); - verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class)); verify(mockTracerContextStorage, never()).get(TracerContextStorage.CURRENT_SPAN); } @@ -94,7 +92,7 @@ public void testCreateSpanWithAttributes() { new ThreadContextBasedTracerContextStorage(threadContext, tracingTelemetry) ); - defaultTracer.startSpan( + Span span = defaultTracer.startSpan( "span_name", Attributes.create().addAttribute("key1", 1.0).addAttribute("key2", 2l).addAttribute("key3", true).addAttribute("key4", "key4") ); @@ -104,6 +102,7 @@ public void testCreateSpanWithAttributes() { assertEquals(2l, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key2")); assertEquals(true, ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key3")); assertEquals("key4", ((MockSpan) defaultTracer.getCurrentSpan().getSpan()).getAttribute("key4")); + span.endSpan(); } public void testCreateSpanWithParent() { @@ -113,25 +112,26 @@ public void testCreateSpanWithParent() { new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry) ); - defaultTracer.startSpan("span_name", null); + Span span = defaultTracer.startSpan("span_name", null); SpanContext parentSpan = defaultTracer.getCurrentSpan(); - defaultTracer.startSpan("span_name_1", parentSpan, Attributes.EMPTY); + Span span1 = defaultTracer.startSpan("span_name_1", parentSpan, Attributes.EMPTY); assertEquals("span_name_1", defaultTracer.getCurrentSpan().getSpan().getSpanName()); assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan().getParentSpan()); + span1.endSpan(); + span.endSpan(); } @SuppressWarnings("unchecked") public void testCreateSpanWithContext() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class))).thenReturn( - mockSpan - ); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class))) + .thenReturn(mockSpan); defaultTracer.startSpan(new SpanCreationContext("span_name", attributes)); - verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(Consumer.class)); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class)); } public void testCreateSpanWithNullParent() { @@ -142,10 +142,11 @@ public void testCreateSpanWithNullParent() { new ThreadContextBasedTracerContextStorage(threadContext, tracingTelemetry) ); - defaultTracer.startSpan("span_name", (SpanContext) null, Attributes.EMPTY); + Span span = defaultTracer.startSpan("span_name", (SpanContext) null, Attributes.EMPTY); assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName()); assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan()); + span.endSpan(); } public void testEndSpanByClosingScopedSpan() { @@ -282,16 +283,51 @@ public void testSpanAcrossThreads() { spanT2.endSpan(); spanScopeT2.close(); - currentSpanRefAfterEndThread2.set(defaultTracer.getCurrentSpan() != null ? defaultTracer.getCurrentSpan().getSpan() : null); + currentSpanRefAfterEndThread2.set(getCurrentSpanFromContext(defaultTracer)); }); spanScope.close(); - currentSpanRefThread1.set(defaultTracer.getCurrentSpan().getSpan()); + currentSpanRefThread1.set(getCurrentSpanFromContext(defaultTracer)); }); assertEquals(spanT2Ref.get(), currentSpanRefThread2.get()); assertEquals(spanRef.get(), currentSpanRefAfterEndThread2.get()); assertEquals(null, currentSpanRefThread1.get()); } + public void testSpanCloseOnThread2() { + TracingTelemetry tracingTelemetry = new MockTracingTelemetry(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + ThreadContextBasedTracerContextStorage spanTracerStorage = new ThreadContextBasedTracerContextStorage( + threadContext, + tracingTelemetry + ); + AtomicReference currentSpanRefThread1 = new AtomicReference<>(); + AtomicReference currentSpanRefThread2 = new AtomicReference<>(); + DefaultTracer defaultTracer = new DefaultTracer(tracingTelemetry, spanTracerStorage); + final Span span = defaultTracer.startSpan(new SpanCreationContext("span_name_t1", Attributes.EMPTY)); + try (SpanScope spanScope = defaultTracer.withSpanInScope(span)) { + executorService.execute(() -> async(new ActionListener() { + @Override + public void onResponse(Boolean response) { + span.endSpan(); + currentSpanRefThread2.set(defaultTracer.getCurrentSpan()); + } + + @Override + public void onFailure(Exception e) { + + } + })); + currentSpanRefThread1.set(defaultTracer.getCurrentSpan()); + } + assertEquals(null, currentSpanRefThread2.get()); + assertEquals(span, currentSpanRefThread1.get().getSpan()); + assertEquals(null, defaultTracer.getCurrentSpan()); + } + + private void async(ActionListener actionListener) { + actionListener.onResponse(true); + } + /** * 1. CreateSpan in ThreadA (NotScopedSpan) * 2. create Async task and pass the span @@ -331,11 +367,10 @@ public void testSpanAcrossThreadsMultipleSpans() { executorService.execute(() -> { Span spanT2 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); SpanScope spanScopeT2 = defaultTracer.withSpanInScope(spanT2); - spanT2Ref.set(spanT2); Span spanT21 = defaultTracer.startSpan(new SpanCreationContext("span_name_t_2", Attributes.EMPTY)); SpanScope spanScopeT21 = defaultTracer.withSpanInScope(spanT2); - + spanT2Ref.set(spanT21); currentSpanRefThread2.set(defaultTracer.getCurrentSpan().getSpan()); spanT21.endSpan(); @@ -343,17 +378,21 @@ public void testSpanAcrossThreadsMultipleSpans() { spanT2.endSpan(); spanScopeT2.close(); - currentSpanRefAfterEndThread2.set(defaultTracer.getCurrentSpan() != null ? defaultTracer.getCurrentSpan().getSpan() : null); + currentSpanRefAfterEndThread2.set(getCurrentSpanFromContext(defaultTracer)); }); spanScope.close(); parentSpanScope.close(); - currentSpanRefThread1.set(defaultTracer.getCurrentSpan().getSpan()); + currentSpanRefThread1.set(getCurrentSpanFromContext(defaultTracer)); }); assertEquals(spanT2Ref.get(), currentSpanRefThread2.get()); assertEquals(spanRef.get(), currentSpanRefAfterEndThread2.get()); assertEquals(null, currentSpanRefThread1.get()); } + private static Span getCurrentSpanFromContext(DefaultTracer defaultTracer) { + return defaultTracer.getCurrentSpan() != null ? defaultTracer.getCurrentSpan().getSpan() : null; + } + public void testClose() throws IOException { Tracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); @@ -376,8 +415,7 @@ private void setupMocks() { when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); when(mockTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class), any(Consumer.class))).thenReturn( - mockSpan - ); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class), any(SpanLifecycleListener.class))) + .thenReturn(mockSpan); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java index 40ff99588b097..a16891188e2fe 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java @@ -18,6 +18,16 @@ public class OTelPropagatedSpan extends OTelSpan { * @param span otel propagated span */ public OTelPropagatedSpan(io.opentelemetry.api.trace.Span span) { - super(null, span, null, a -> {}); + super(null, span, null, new SpanLifecycleListener() { + @Override + public void onStart(Span span) { + + } + + @Override + public void onEnd(Span span) { + + } + }); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java index 1ffd5c10cd8bd..3e070b62acb6d 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java @@ -9,7 +9,6 @@ package org.opensearch.telemetry.tracing; import java.util.Objects; -import java.util.function.Consumer; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.StatusCode; @@ -21,30 +20,30 @@ class OTelSpan extends AbstractSpan { private final Span delegateSpan; - private final Consumer onSpanEndConsumer; + private final SpanLifecycleListener spanLifecycleListener; /** * Constructor * @param spanName * @param span * @param parentSpan - * @param onSpanEndConsumer + * @param spanLifecycleListener */ public OTelSpan( String spanName, Span span, org.opensearch.telemetry.tracing.Span parentSpan, - Consumer onSpanEndConsumer + SpanLifecycleListener spanLifecycleListener ) { super(spanName, parentSpan); this.delegateSpan = span; - this.onSpanEndConsumer = Objects.requireNonNull(onSpanEndConsumer); + this.spanLifecycleListener = Objects.requireNonNull(spanLifecycleListener); } @Override public void endSpan() { delegateSpan.end(); - onSpanEndConsumer.accept(this); + spanLifecycleListener.onEnd(this); } @Override diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 9878705671af2..bc3fea2c0658b 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -14,7 +14,6 @@ import java.io.Closeable; import java.io.IOException; -import java.util.function.Consumer; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; @@ -48,8 +47,8 @@ public void close() { } @Override - public Span createSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer) { - return createOtelSpan(spanName, parentSpan, attributes, onSpanEndConsumer); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener) { + return createOtelSpan(spanName, parentSpan, attributes, spanLifecycleListener); } @Override @@ -57,9 +56,11 @@ public TracingContextPropagator getContextPropagator() { return new OTelTracingContextPropagator(openTelemetry); } - private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer) { + private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener) { io.opentelemetry.api.trace.Span otelSpan = otelSpan(spanName, parentSpan, OTelAttributesConverter.convert(attributes)); - return new OTelSpan(spanName, otelSpan, parentSpan, onSpanEndConsumer); + Span newSpan = new OTelSpan(spanName, otelSpan, parentSpan, spanLifecycleListener); + spanLifecycleListener.onStart(newSpan); + return newSpan; } io.opentelemetry.api.trace.Span otelSpan(String spanName, Span parentOTelSpan, io.opentelemetry.api.common.Attributes attributes) { diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java index acef528a2d6f5..3017611499b11 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java @@ -29,7 +29,17 @@ public class OTelSpanTests extends OpenSearchTestCase { public void testEndSpanTest() { Span mockSpan = getMockSpan(); final AtomicBoolean onSpanEndTestFlag = new AtomicBoolean(false); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> { onSpanEndTestFlag.set(true); }); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, new SpanLifecycleListener() { + @Override + public void onStart(org.opensearch.telemetry.tracing.Span span) { + + } + + @Override + public void onEnd(org.opensearch.telemetry.tracing.Span span) { + onSpanEndTestFlag.set(true); + } + }); oTelSpan.endSpan(); assertTrue(onSpanEndTestFlag.get()); verify(mockSpan).end(); @@ -37,7 +47,7 @@ public void testEndSpanTest() { public void testAddAttributeString() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); oTelSpan.addAttribute("key", "value"); verify(mockSpan).setAttribute("key", "value"); @@ -45,7 +55,7 @@ public void testAddAttributeString() { public void testAddAttributeLong() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); oTelSpan.addAttribute("key", 1L); verify(mockSpan).setAttribute("key", 1L); @@ -53,7 +63,7 @@ public void testAddAttributeLong() { public void testAddAttributeDouble() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); oTelSpan.addAttribute("key", 1.0); verify(mockSpan).setAttribute("key", 1.0); @@ -61,7 +71,7 @@ public void testAddAttributeDouble() { public void testAddAttributeBoolean() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); oTelSpan.addAttribute("key", true); verify(mockSpan).setAttribute("key", true); @@ -69,7 +79,7 @@ public void testAddAttributeBoolean() { public void testAddEvent() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); oTelSpan.addEvent("eventName"); verify(mockSpan).addEvent("eventName"); @@ -77,14 +87,14 @@ public void testAddEvent() { public void testGetTraceId() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); assertEquals(TRACE_ID, oTelSpan.getTraceId()); } public void testGetSpanId() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); assertEquals(SPAN_ID, oTelSpan.getSpanId()); } @@ -94,4 +104,18 @@ private Span getMockSpan() { when(mockSpan.getSpanContext()).thenReturn(SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())); return mockSpan; } + + private SpanLifecycleListener getSpanLifecycleListener() { + return new SpanLifecycleListener() { + @Override + public void onStart(org.opensearch.telemetry.tracing.Span span) { + + } + + @Override + public void onEnd(org.opensearch.telemetry.tracing.Span span) { + + } + }; + } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java index 25948284a97fb..1a69f61c36258 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java @@ -35,7 +35,7 @@ public class OTelTracingContextPropagatorTests extends OpenSearchTestCase { public void testAddTracerContextToHeader() { Span mockSpan = mock(Span.class); when(mockSpan.getSpanContext()).thenReturn(SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())); - OTelSpan span = new OTelSpan("spanName", mockSpan, null, a -> {}); + OTelSpan span = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); Map requestHeaders = new HashMap<>(); OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); when(mockOpenTelemetry.getPropagators()).thenReturn(ContextPropagators.create(W3CTraceContextPropagator.getInstance())); @@ -86,4 +86,18 @@ public void testExtractTracerContextFromHttpHeaderEmpty() { assertEquals(propagatedSpan.getTraceId(), span.getTraceId()); assertEquals(propagatedSpan.getSpanId(), span.getSpanId()); } + + private SpanLifecycleListener getSpanLifecycleListener() { + return new SpanLifecycleListener() { + @Override + public void onStart(org.opensearch.telemetry.tracing.Span span) { + + } + + @Override + public void onEnd(org.opensearch.telemetry.tracing.Span span) { + + } + }; + } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 88575dadcac41..49f71e3177a92 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -38,7 +38,7 @@ public void testCreateSpanWithoutParent() { Map attributeMap = Collections.singletonMap("name", "value"); Attributes attributes = Attributes.create().addAttribute("name", "value"); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); - Span span = tracingTelemetry.createSpan("span_name", null, attributes, a -> {}); + Span span = tracingTelemetry.createSpan("span_name", null, attributes, getSpanLifecycleListener()); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); @@ -55,11 +55,11 @@ public void testCreateSpanWithParent() { when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, a -> {}); + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, getSpanLifecycleListener()); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); Attributes attributes = Attributes.create().addAttribute("name", 1l); - Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, a -> {}); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, getSpanLifecycleListener()); verify(mockSpanBuilder).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttributeLong(attributes)); @@ -78,7 +78,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, a -> {}); + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, getSpanLifecycleListener()); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); Attributes attributes = Attributes.create() @@ -86,7 +86,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { .addAttribute("key2", 2.0) .addAttribute("key3", true) .addAttribute("key4", "key4"); - Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, a -> {}); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, getSpanLifecycleListener()); io.opentelemetry.api.common.Attributes otelAttributes = io.opentelemetry.api.common.Attributes.builder() .put("key1", 1l) @@ -123,4 +123,18 @@ public void testGetContextPropagator() { assertTrue(tracingTelemetry.getContextPropagator() instanceof OTelTracingContextPropagator); } + private SpanLifecycleListener getSpanLifecycleListener() { + return new SpanLifecycleListener() { + @Override + public void onStart(org.opensearch.telemetry.tracing.Span span) { + + } + + @Override + public void onEnd(org.opensearch.telemetry.tracing.Span span) { + + } + }; + } + } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java index bbc943a05b504..085c5526e373b 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java @@ -10,6 +10,7 @@ import org.opensearch.telemetry.tracing.AbstractSpan; import org.opensearch.telemetry.tracing.Span; +import org.opensearch.telemetry.tracing.SpanLifecycleListener; import org.opensearch.telemetry.tracing.attributes.Attributes; import java.util.HashMap; @@ -17,7 +18,6 @@ import java.util.Objects; import java.util.Random; import java.util.concurrent.ThreadLocalRandom; -import java.util.function.Consumer; import java.util.function.Supplier; /** @@ -36,7 +36,7 @@ public class MockSpan extends AbstractSpan { private static final Supplier randomSupplier = ThreadLocalRandom::current; - private final Consumer onSpanEndConsumer; + private final SpanLifecycleListener spanLifecycleListener; /** * Base Constructor. @@ -51,7 +51,7 @@ public MockSpan( Span parentSpan, SpanProcessor spanProcessor, Attributes attributes, - Consumer onSpanEndConsumer + SpanLifecycleListener onSpanEndConsumer ) { this( spanName, @@ -71,7 +71,7 @@ public MockSpan( * @param spanProcessor span processor. * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, Consumer onSpanEndConsumer) { + public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, SpanLifecycleListener onSpanEndConsumer) { this( spanName, parentSpan, @@ -100,7 +100,7 @@ public MockSpan( String spanId, SpanProcessor spanProcessor, Attributes attributes, - Consumer onSpanEndConsumer + SpanLifecycleListener onSpanEndConsumer ) { super(spanName, parentSpan); this.spanProcessor = spanProcessor; @@ -111,7 +111,7 @@ public MockSpan( if (attributes != null) { this.metadata.putAll(attributes.getAttributesMap()); } - this.onSpanEndConsumer = Objects.requireNonNull(onSpanEndConsumer); + this.spanLifecycleListener = Objects.requireNonNull(onSpanEndConsumer); } @Override @@ -120,9 +120,9 @@ public void endSpan() { if (hasEnded) { return; } - onSpanEndConsumer.accept(this); endTime = System.nanoTime(); hasEnded = true; + spanLifecycleListener.onEnd(this); } spanProcessor.onEnd(this); } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java index dff46880a3e69..7a319547ad574 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java @@ -10,6 +10,7 @@ import org.opensearch.core.common.Strings; import org.opensearch.telemetry.tracing.Span; +import org.opensearch.telemetry.tracing.SpanLifecycleListener; import org.opensearch.telemetry.tracing.TracingContextPropagator; import org.opensearch.telemetry.tracing.attributes.Attributes; @@ -44,7 +45,7 @@ public Optional extract(Map props) { String[] values = value.split(SEPARATOR); String traceId = values[0]; String spanId = values[1]; - return Optional.of(new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY, (a) -> {})); + return Optional.of(new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY, getSpanLifecycleListener())); } else { return Optional.empty(); } @@ -72,4 +73,18 @@ public void inject(Span currentSpan, BiConsumer setter) { setter.accept(TRACE_PARENT, traceParent); } } + + private SpanLifecycleListener getSpanLifecycleListener() { + return new SpanLifecycleListener() { + @Override + public void onStart(org.opensearch.telemetry.tracing.Span span) { + + } + + @Override + public void onEnd(org.opensearch.telemetry.tracing.Span span) { + + } + }; + } } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java index 250c4960ccf9e..b074ebf342f17 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java @@ -9,6 +9,7 @@ package org.opensearch.test.telemetry.tracing; import org.opensearch.telemetry.tracing.Span; +import org.opensearch.telemetry.tracing.SpanLifecycleListener; import org.opensearch.telemetry.tracing.TracingContextPropagator; import org.opensearch.telemetry.tracing.TracingTelemetry; import org.opensearch.telemetry.tracing.attributes.Attributes; @@ -17,7 +18,6 @@ import java.util.Arrays; import java.util.List; -import java.util.function.Consumer; /** * Mock {@link TracingTelemetry} implementation for testing. @@ -34,8 +34,9 @@ public MockTracingTelemetry() { } @Override - public Span createSpan(String spanName, Span parentSpan, Attributes attributes, Consumer onSpanEndConsumer) { - Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes, onSpanEndConsumer); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener) { + Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes, spanLifecycleListener); + spanLifecycleListener.onStart(span); spanProcessor.onStart(span); return span; } From 1350ff55a029bfa6669049e34ecb896ab8246db2 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Fri, 1 Sep 2023 02:10:13 +0530 Subject: [PATCH 12/16] Fix java doc Signed-off-by: Gagan Juneja --- .../java/org/opensearch/telemetry/tracing/DefaultSpanScope.java | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java index b5e106af357c3..037309da0bc30 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultSpanScope.java @@ -33,6 +33,7 @@ private DefaultSpanScope(Span span, SpanScope previousSpanScope, TracerContextSt /** * Creates the SpanScope object. * @param span span. + * @param tracerContextStorage tracer context storage. * @return SpanScope spanScope */ public static SpanScope create(Span span, TracerContextStorage tracerContextStorage) { From aa4988bbb543dba750dd9280bbf81205f85b16d1 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Fri, 1 Sep 2023 19:25:07 +0530 Subject: [PATCH 13/16] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultTracer.java | 23 +---------- .../tracing/SpanLifecycleListener.java | 26 ------------ .../telemetry/tracing/TracingTelemetry.java | 9 ++--- .../telemetry/tracing/DefaultTracerTests.java | 18 ++++----- .../telemetry/tracing/OTelPropagatedSpan.java | 12 +----- .../telemetry/tracing/OTelSpan.java | 13 +----- .../tracing/OTelTracingTelemetry.java | 9 ++--- .../telemetry/tracing/OTelSpanTests.java | 40 ++++--------------- .../OTelTracingContextPropagatorTests.java | 16 +------- .../tracing/OTelTracingTelemetryTests.java | 24 +++-------- .../test/telemetry/tracing/MockSpan.java | 35 +++------------- .../tracing/MockTracingContextPropagator.java | 17 +------- .../tracing/MockTracingTelemetry.java | 6 +-- 13 files changed, 40 insertions(+), 208 deletions(-) delete mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 61bc30601063f..b75d761fc240c 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -100,28 +100,7 @@ public SpanScope withSpanInScope(Span span) { } private Span createSpan(String spanName, Span parentSpan, Attributes attributes) { - return tracingTelemetry.createSpan(spanName, parentSpan, attributes, new SpanLifecycleListener() { - @Override - public void onStart(Span span) { - setCurrentSpanInContext(span); - } - - @Override - public void onEnd(Span span) { - onSpanEnd(span, parentSpan); - } - }); - } - - private void onSpanEnd(Span span, Span parentSpan) { - if (getCurrentSpanInternal() == span && parentSpan != null) { - setCurrentSpanInContext(parentSpan); - } else if (span.getParentSpan() == null) { - setCurrentSpanInContext(null); - } else { - // Most likely this scenario is where parent is coming from a calling thread - // and need not be updated. stashContext upstream would take care of this. - } + return tracingTelemetry.createSpan(spanName, parentSpan, attributes); } private void setCurrentSpanInContext(Span span) { diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java deleted file mode 100644 index a5aae3660fc47..0000000000000 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/SpanLifecycleListener.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.tracing; - -/** - * A listener for Span Lifecycle - */ -public interface SpanLifecycleListener { - /** - * On Span start - * @param span span - */ - void onStart(Span span); - - /** - * On span end - * @param span span - */ - void onEnd(Span span); -} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java index 34fa8376a0098..895e14da69f7f 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java @@ -22,13 +22,12 @@ public interface TracingTelemetry extends Closeable { /** * Creates span with provided arguments * - * @param spanName name of the span - * @param parentSpan span's parent span - * @param attributes attributes to be added. - * @param spanLifecycleListener consumer to be invoked on span end. + * @param spanName name of the span + * @param parentSpan span's parent span + * @param attributes attributes to be added. * @return span instance */ - Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener); + Span createSpan(String spanName, Span parentSpan, Attributes attributes); /** * provides tracing context propagator diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 9ebf1751061f9..5205bdfc8a031 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -67,20 +67,18 @@ public void testCreateSpan() { public void testCreateSpanWithAttributesWithMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class))) - .thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes))).thenReturn(mockSpan); defaultTracer.startSpan("span_name", attributes); - verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class)); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes)); } @SuppressWarnings("unchecked") public void testCreateSpanWithAttributesWithParentMock() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class))) - .thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes))).thenReturn(mockSpan); defaultTracer.startSpan("span_name", new SpanContext(mockParentSpan), attributes); - verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class)); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes)); verify(mockTracerContextStorage, never()).get(TracerContextStorage.CURRENT_SPAN); } @@ -128,10 +126,9 @@ public void testCreateSpanWithParent() { public void testCreateSpanWithContext() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); Attributes attributes = Attributes.create().addAttribute("name", "value"); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class))) - .thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes))).thenReturn(mockSpan); defaultTracer.startSpan(new SpanCreationContext("span_name", attributes)); - verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes), any(SpanLifecycleListener.class)); + verify(mockTracingTelemetry).createSpan(eq("span_name"), eq(mockParentSpan), eq(attributes)); } public void testCreateSpanWithNullParent() { @@ -415,7 +412,6 @@ private void setupMocks() { when(mockParentSpan.getSpanId()).thenReturn("parent_span_id"); when(mockParentSpan.getTraceId()).thenReturn("trace_id"); when(mockTracerContextStorage.get(TracerContextStorage.CURRENT_SPAN)).thenReturn(mockParentSpan, mockSpan); - when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class), any(SpanLifecycleListener.class))) - .thenReturn(mockSpan); + when(mockTracingTelemetry.createSpan(eq("span_name"), eq(mockParentSpan), any(Attributes.class))).thenReturn(mockSpan); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java index a16891188e2fe..5aa1069e60367 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelPropagatedSpan.java @@ -18,16 +18,6 @@ public class OTelPropagatedSpan extends OTelSpan { * @param span otel propagated span */ public OTelPropagatedSpan(io.opentelemetry.api.trace.Span span) { - super(null, span, null, new SpanLifecycleListener() { - @Override - public void onStart(Span span) { - - } - - @Override - public void onEnd(Span span) { - - } - }); + super(null, span, null); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java index 3e070b62acb6d..7653a56cbb740 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelSpan.java @@ -8,8 +8,6 @@ package org.opensearch.telemetry.tracing; -import java.util.Objects; - import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.StatusCode; @@ -20,30 +18,21 @@ class OTelSpan extends AbstractSpan { private final Span delegateSpan; - private final SpanLifecycleListener spanLifecycleListener; /** * Constructor * @param spanName * @param span * @param parentSpan - * @param spanLifecycleListener */ - public OTelSpan( - String spanName, - Span span, - org.opensearch.telemetry.tracing.Span parentSpan, - SpanLifecycleListener spanLifecycleListener - ) { + public OTelSpan(String spanName, Span span, org.opensearch.telemetry.tracing.Span parentSpan) { super(spanName, parentSpan); this.delegateSpan = span; - this.spanLifecycleListener = Objects.requireNonNull(spanLifecycleListener); } @Override public void endSpan() { delegateSpan.end(); - spanLifecycleListener.onEnd(this); } @Override diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index bc3fea2c0658b..ee2f17aabb7f9 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -47,8 +47,8 @@ public void close() { } @Override - public Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener) { - return createOtelSpan(spanName, parentSpan, attributes, spanLifecycleListener); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes) { + return createOtelSpan(spanName, parentSpan, attributes); } @Override @@ -56,10 +56,9 @@ public TracingContextPropagator getContextPropagator() { return new OTelTracingContextPropagator(openTelemetry); } - private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener) { + private Span createOtelSpan(String spanName, Span parentSpan, Attributes attributes) { io.opentelemetry.api.trace.Span otelSpan = otelSpan(spanName, parentSpan, OTelAttributesConverter.convert(attributes)); - Span newSpan = new OTelSpan(spanName, otelSpan, parentSpan, spanLifecycleListener); - spanLifecycleListener.onStart(newSpan); + Span newSpan = new OTelSpan(spanName, otelSpan, parentSpan); return newSpan; } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java index 3017611499b11..0ccc95a1f5c16 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java @@ -29,17 +29,7 @@ public class OTelSpanTests extends OpenSearchTestCase { public void testEndSpanTest() { Span mockSpan = getMockSpan(); final AtomicBoolean onSpanEndTestFlag = new AtomicBoolean(false); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, new SpanLifecycleListener() { - @Override - public void onStart(org.opensearch.telemetry.tracing.Span span) { - - } - - @Override - public void onEnd(org.opensearch.telemetry.tracing.Span span) { - onSpanEndTestFlag.set(true); - } - }); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.endSpan(); assertTrue(onSpanEndTestFlag.get()); verify(mockSpan).end(); @@ -47,7 +37,7 @@ public void onEnd(org.opensearch.telemetry.tracing.Span span) { public void testAddAttributeString() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.addAttribute("key", "value"); verify(mockSpan).setAttribute("key", "value"); @@ -55,7 +45,7 @@ public void testAddAttributeString() { public void testAddAttributeLong() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.addAttribute("key", 1L); verify(mockSpan).setAttribute("key", 1L); @@ -63,7 +53,7 @@ public void testAddAttributeLong() { public void testAddAttributeDouble() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.addAttribute("key", 1.0); verify(mockSpan).setAttribute("key", 1.0); @@ -71,7 +61,7 @@ public void testAddAttributeDouble() { public void testAddAttributeBoolean() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.addAttribute("key", true); verify(mockSpan).setAttribute("key", true); @@ -79,7 +69,7 @@ public void testAddAttributeBoolean() { public void testAddEvent() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.addEvent("eventName"); verify(mockSpan).addEvent("eventName"); @@ -87,14 +77,14 @@ public void testAddEvent() { public void testGetTraceId() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); assertEquals(TRACE_ID, oTelSpan.getTraceId()); } public void testGetSpanId() { Span mockSpan = getMockSpan(); - OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); assertEquals(SPAN_ID, oTelSpan.getSpanId()); } @@ -104,18 +94,4 @@ private Span getMockSpan() { when(mockSpan.getSpanContext()).thenReturn(SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())); return mockSpan; } - - private SpanLifecycleListener getSpanLifecycleListener() { - return new SpanLifecycleListener() { - @Override - public void onStart(org.opensearch.telemetry.tracing.Span span) { - - } - - @Override - public void onEnd(org.opensearch.telemetry.tracing.Span span) { - - } - }; - } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java index 1a69f61c36258..16a3ec9493d5d 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagatorTests.java @@ -35,7 +35,7 @@ public class OTelTracingContextPropagatorTests extends OpenSearchTestCase { public void testAddTracerContextToHeader() { Span mockSpan = mock(Span.class); when(mockSpan.getSpanContext()).thenReturn(SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())); - OTelSpan span = new OTelSpan("spanName", mockSpan, null, getSpanLifecycleListener()); + OTelSpan span = new OTelSpan("spanName", mockSpan, null); Map requestHeaders = new HashMap<>(); OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); when(mockOpenTelemetry.getPropagators()).thenReturn(ContextPropagators.create(W3CTraceContextPropagator.getInstance())); @@ -86,18 +86,4 @@ public void testExtractTracerContextFromHttpHeaderEmpty() { assertEquals(propagatedSpan.getTraceId(), span.getTraceId()); assertEquals(propagatedSpan.getSpanId(), span.getSpanId()); } - - private SpanLifecycleListener getSpanLifecycleListener() { - return new SpanLifecycleListener() { - @Override - public void onStart(org.opensearch.telemetry.tracing.Span span) { - - } - - @Override - public void onEnd(org.opensearch.telemetry.tracing.Span span) { - - } - }; - } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 49f71e3177a92..3f46cb621a8ec 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -38,7 +38,7 @@ public void testCreateSpanWithoutParent() { Map attributeMap = Collections.singletonMap("name", "value"); Attributes attributes = Attributes.create().addAttribute("name", "value"); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); - Span span = tracingTelemetry.createSpan("span_name", null, attributes, getSpanLifecycleListener()); + Span span = tracingTelemetry.createSpan("span_name", null, attributes); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); @@ -55,11 +55,11 @@ public void testCreateSpanWithParent() { when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, getSpanLifecycleListener()); + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); Attributes attributes = Attributes.create().addAttribute("name", 1l); - Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, getSpanLifecycleListener()); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes); verify(mockSpanBuilder).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttributeLong(attributes)); @@ -78,7 +78,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); - Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null, getSpanLifecycleListener()); + Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); Attributes attributes = Attributes.create() @@ -86,7 +86,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { .addAttribute("key2", 2.0) .addAttribute("key3", true) .addAttribute("key4", "key4"); - Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes, getSpanLifecycleListener()); + Span span = tracingTelemetry.createSpan("span_name", parentSpan, attributes); io.opentelemetry.api.common.Attributes otelAttributes = io.opentelemetry.api.common.Attributes.builder() .put("key1", 1l) @@ -123,18 +123,4 @@ public void testGetContextPropagator() { assertTrue(tracingTelemetry.getContextPropagator() instanceof OTelTracingContextPropagator); } - private SpanLifecycleListener getSpanLifecycleListener() { - return new SpanLifecycleListener() { - @Override - public void onStart(org.opensearch.telemetry.tracing.Span span) { - - } - - @Override - public void onEnd(org.opensearch.telemetry.tracing.Span span) { - - } - }; - } - } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java index 085c5526e373b..892c1a8b3eeae 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockSpan.java @@ -10,12 +10,10 @@ import org.opensearch.telemetry.tracing.AbstractSpan; import org.opensearch.telemetry.tracing.Span; -import org.opensearch.telemetry.tracing.SpanLifecycleListener; import org.opensearch.telemetry.tracing.attributes.Attributes; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.Random; import java.util.concurrent.ThreadLocalRandom; import java.util.function.Supplier; @@ -36,31 +34,21 @@ public class MockSpan extends AbstractSpan { private static final Supplier randomSupplier = ThreadLocalRandom::current; - private final SpanLifecycleListener spanLifecycleListener; - /** * Base Constructor. * @param spanName Span Name * @param parentSpan Parent Span * @param spanProcessor Span Processor * @param attributes attributes - * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan( - String spanName, - Span parentSpan, - SpanProcessor spanProcessor, - Attributes attributes, - SpanLifecycleListener onSpanEndConsumer - ) { + public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, Attributes attributes) { this( spanName, parentSpan, parentSpan != null ? parentSpan.getTraceId() : IdGenerator.generateTraceId(), IdGenerator.generateSpanId(), spanProcessor, - attributes, - onSpanEndConsumer + attributes ); } @@ -69,17 +57,15 @@ public MockSpan( * @param spanName span name. * @param parentSpan parent span name * @param spanProcessor span processor. - * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, SpanLifecycleListener onSpanEndConsumer) { + public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor) { this( spanName, parentSpan, parentSpan != null ? parentSpan.getTraceId() : IdGenerator.generateTraceId(), IdGenerator.generateSpanId(), spanProcessor, - Attributes.EMPTY, - onSpanEndConsumer + Attributes.EMPTY ); } @@ -91,17 +77,8 @@ public MockSpan(String spanName, Span parentSpan, SpanProcessor spanProcessor, S * @param spanId Span ID * @param spanProcessor Span Processor * @param attributes attributes - * @param onSpanEndConsumer consumer to be executed on span end. */ - public MockSpan( - String spanName, - Span parentSpan, - String traceId, - String spanId, - SpanProcessor spanProcessor, - Attributes attributes, - SpanLifecycleListener onSpanEndConsumer - ) { + public MockSpan(String spanName, Span parentSpan, String traceId, String spanId, SpanProcessor spanProcessor, Attributes attributes) { super(spanName, parentSpan); this.spanProcessor = spanProcessor; this.metadata = new HashMap<>(); @@ -111,7 +88,6 @@ public MockSpan( if (attributes != null) { this.metadata.putAll(attributes.getAttributesMap()); } - this.spanLifecycleListener = Objects.requireNonNull(onSpanEndConsumer); } @Override @@ -122,7 +98,6 @@ public void endSpan() { } endTime = System.nanoTime(); hasEnded = true; - spanLifecycleListener.onEnd(this); } spanProcessor.onEnd(this); } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java index 7a319547ad574..7525b4424c243 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingContextPropagator.java @@ -10,7 +10,6 @@ import org.opensearch.core.common.Strings; import org.opensearch.telemetry.tracing.Span; -import org.opensearch.telemetry.tracing.SpanLifecycleListener; import org.opensearch.telemetry.tracing.TracingContextPropagator; import org.opensearch.telemetry.tracing.attributes.Attributes; @@ -45,7 +44,7 @@ public Optional extract(Map props) { String[] values = value.split(SEPARATOR); String traceId = values[0]; String spanId = values[1]; - return Optional.of(new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY, getSpanLifecycleListener())); + return Optional.of(new MockSpan(null, null, traceId, spanId, spanProcessor, Attributes.EMPTY)); } else { return Optional.empty(); } @@ -73,18 +72,4 @@ public void inject(Span currentSpan, BiConsumer setter) { setter.accept(TRACE_PARENT, traceParent); } } - - private SpanLifecycleListener getSpanLifecycleListener() { - return new SpanLifecycleListener() { - @Override - public void onStart(org.opensearch.telemetry.tracing.Span span) { - - } - - @Override - public void onEnd(org.opensearch.telemetry.tracing.Span span) { - - } - }; - } } diff --git a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java index b074ebf342f17..9b958bbb40f84 100644 --- a/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java +++ b/test/telemetry/src/main/java/org/opensearch/test/telemetry/tracing/MockTracingTelemetry.java @@ -9,7 +9,6 @@ package org.opensearch.test.telemetry.tracing; import org.opensearch.telemetry.tracing.Span; -import org.opensearch.telemetry.tracing.SpanLifecycleListener; import org.opensearch.telemetry.tracing.TracingContextPropagator; import org.opensearch.telemetry.tracing.TracingTelemetry; import org.opensearch.telemetry.tracing.attributes.Attributes; @@ -34,9 +33,8 @@ public MockTracingTelemetry() { } @Override - public Span createSpan(String spanName, Span parentSpan, Attributes attributes, SpanLifecycleListener spanLifecycleListener) { - Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes, spanLifecycleListener); - spanLifecycleListener.onStart(span); + public Span createSpan(String spanName, Span parentSpan, Attributes attributes) { + Span span = new MockSpan(spanName, parentSpan, spanProcessor, attributes); spanProcessor.onStart(span); return span; } From 2b4096cccbd46772d2c5e059477a1d3c96799d51 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Fri, 1 Sep 2023 19:56:39 +0530 Subject: [PATCH 14/16] Fix failing test Signed-off-by: Gagan Juneja --- .../java/org/opensearch/telemetry/tracing/OTelSpanTests.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java index 0ccc95a1f5c16..fc92ab36908e1 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelSpanTests.java @@ -10,8 +10,6 @@ import org.opensearch.test.OpenSearchTestCase; -import java.util.concurrent.atomic.AtomicBoolean; - import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.TraceFlags; @@ -28,10 +26,8 @@ public class OTelSpanTests extends OpenSearchTestCase { public void testEndSpanTest() { Span mockSpan = getMockSpan(); - final AtomicBoolean onSpanEndTestFlag = new AtomicBoolean(false); OTelSpan oTelSpan = new OTelSpan("spanName", mockSpan, null); oTelSpan.endSpan(); - assertTrue(onSpanEndTestFlag.get()); verify(mockSpan).end(); } From b19ae022ad0f41fc50140644dff01b09c277f2e3 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Fri, 1 Sep 2023 22:03:27 +0530 Subject: [PATCH 15/16] Empty-Commit Signed-off-by: Gagan Juneja From 05e65ca1ac738d032d2d9b236a1559ec7b978317 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Fri, 1 Sep 2023 23:04:29 +0530 Subject: [PATCH 16/16] Empty-Commit Signed-off-by: Gagan Juneja