From bae04ac7cdb475f104d590377de73a2ee2976a8e Mon Sep 17 00:00:00 2001 From: suranjay Date: Tue, 27 Jun 2023 21:17:12 +0530 Subject: [PATCH] Remove endSpan method --- .../telemetry/tracing/DefaultTracer.java | 6 ----- .../opensearch/telemetry/tracing/Tracer.java | 8 +------ .../telemetry/tracing/noop/NoopTracer.java | 3 --- .../telemetry/tracing/DefaultTracerTests.java | 10 -------- .../telemetry/tracing/OTelSpan.java | 24 +++++++++---------- .../tracing/OTelTracingContextPropagator.java | 2 +- .../tracing/OTelTracingTelemetry.java | 2 +- 7 files changed, 15 insertions(+), 40 deletions(-) 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 b9f6fcec33603..ab9110af7c3ab 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 @@ -43,12 +43,6 @@ public Scope startSpan(String spanName) { return new ScopeImpl(() -> endSpan(span)); } - @Override - public void endSpan() { - Span currentSpan = getCurrentSpan(); - endSpan(currentSpan); - } - @Override public void addSpanAttribute(String key, String value) { Span currentSpan = getCurrentSpan(); 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 1a27770eb9758..fcc091eb39c48 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 @@ -22,16 +22,10 @@ public interface Tracer extends Closeable { * Starts the {@link Span} with given name * * @param spanName span name - * @return scope of the span, can be used with try-with-resources to close the span + * @return scope of the span, must be closed with explicit close or with try-with-resource */ Scope startSpan(String spanName); - /** - * Ends the current active {@link Span} - * - */ - void endSpan(); - /** * Adds string attribute to the current active {@link Span}. * 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 9a36c8c665e97..18fc60e41e54d 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 @@ -28,9 +28,6 @@ public Scope startSpan(String spanName) { return Scope.NO_OP; } - @Override - public void endSpan() {} - /** * @param key attribute key * @param value attribute value 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 f2203ec20d116..f0e8f3c2e2344 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 @@ -43,16 +43,6 @@ public void testCreateSpan() { Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpanName()); } - public void testEndSpan() { - DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); - defaultTracer.startSpan("span_name"); - verify(mockTracerContextStorage).put(TracerContextStorage.CURRENT_SPAN, mockSpan); - - defaultTracer.endSpan(); - verify(mockSpan).endSpan(); - verify(mockTracerContextStorage).put(TracerContextStorage.CURRENT_SPAN, mockParentSpan); - } - public void testEndSpanByClosingScope() { DefaultTracer defaultTracer = new DefaultTracer(mockTracingTelemetry, mockTracerContextStorage); try (Scope scope = defaultTracer.startSpan("span_name")) { 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 c1095e1aa9350..23a2d9baa3e6e 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 @@ -16,55 +16,55 @@ */ class OTelSpan extends AbstractSpan { - private final Span oTelSpan; + private final Span delegateSpan; public OTelSpan(String spanName, Span span, org.opensearch.telemetry.tracing.Span parentSpan) { super(spanName, parentSpan); - this.oTelSpan = span; + this.delegateSpan = span; } @Override public void endSpan() { - oTelSpan.end(); + delegateSpan.end(); } @Override public void addAttribute(String key, String value) { - oTelSpan.setAttribute(key, value); + delegateSpan.setAttribute(key, value); } @Override public void addAttribute(String key, Long value) { - oTelSpan.setAttribute(key, value); + delegateSpan.setAttribute(key, value); } @Override public void addAttribute(String key, Double value) { - oTelSpan.setAttribute(key, value); + delegateSpan.setAttribute(key, value); } @Override public void addAttribute(String key, Boolean value) { - oTelSpan.setAttribute(key, value); + delegateSpan.setAttribute(key, value); } @Override public void addEvent(String event) { - oTelSpan.addEvent(event); + delegateSpan.addEvent(event); } @Override public String getTraceId() { - return oTelSpan.getSpanContext().getTraceId(); + return delegateSpan.getSpanContext().getTraceId(); } @Override public String getSpanId() { - return oTelSpan.getSpanContext().getSpanId(); + return delegateSpan.getSpanContext().getSpanId(); } - io.opentelemetry.api.trace.Span getoTelSpan() { - return oTelSpan; + io.opentelemetry.api.trace.Span getDelegateSpan() { + return delegateSpan; } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagator.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagator.java index 3fa4de2a5d137..15609b39b6b94 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagator.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingContextPropagator.java @@ -48,7 +48,7 @@ public void inject(Span currentSpan, BiConsumer setter) { } private static Context context(OTelSpan oTelSpan) { - return Context.current().with(io.opentelemetry.api.trace.Span.wrap(oTelSpan.getoTelSpan().getSpanContext())); + return Context.current().with(io.opentelemetry.api.trace.Span.wrap(oTelSpan.getDelegateSpan().getSpanContext())); } private static final TextMapSetter> TEXT_MAP_SETTER = (carrier, key, value) -> { 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 39a4cf3bb7545..8a0034e098461 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 @@ -63,6 +63,6 @@ private Span createOtelSpan(String spanName, Span parentSpan) { io.opentelemetry.api.trace.Span otelSpan(String spanName, Span parentOTelSpan) { return parentOTelSpan == null || !(parentOTelSpan instanceof OTelSpan) ? otelTracer.spanBuilder(spanName).startSpan() - : otelTracer.spanBuilder(spanName).setParent(Context.current().with(((OTelSpan) parentOTelSpan).getoTelSpan())).startSpan(); + : otelTracer.spanBuilder(spanName).setParent(Context.current().with(((OTelSpan) parentOTelSpan).getDelegateSpan())).startSpan(); } }