Repository: helidon-io/helidon (java)

Issue:
4.x Tracing `Span` and `Span.Builder` `unwrap` methods are essentially unusable; the types should expose their OpenTracing or OpenTelemetry delegates not the Helidon package-private impls
## Environment Details
* Helidon Version: 4.x
* Helidon SE or Helidon MP
* JDK version:
* OS:
* Docker version (if applicable):

----------

## Problem Description
The Helidon tracing API `Span` and `Span.Builder` types expose an `<T> T unwrap(Class<T> type)` method which essentially casts `this` to the specified type.

But these methods are currently of little/no use because the actual types to which the caller could cast (e.g., `OpenTelemetrySpan` and `OpenTelemetrySpanBuilder`) are package-private and so are inaccessible to the application. Further, those implementation types do not provide a way to get the underlying OpenTelemetry or OpenTracing object which, arguable, the developer really wants.

More useful would be to either change `unwrap` to return the `delegate` (instead of `this`) or add a `<T> T delegate(Class<T> type)` method.

One example of usage: As described in issue #8114, the Helidon tracing API does not provide a way to reflect the follows-from relationship among spans (just parent/child). If the Helidon types exposed their delegates, then user code could use the OpenTracing  `SpanBuilder#addReference` method  or the OpenTelemetry `SpanBuilder#addLink` method to work around that limitation.

Beyond that one example, the spirit of the existing `unwrap` methods on `Span` and `Span.Builder` is to allow developers who know what they are doing to have access to the underlying implementation objects. The existing `unwrap` methods do not do that now.



Pull request:
Fix unwrap methods to return cast delegates rather than cast 'this'
### Description
Resolves #8200 

This PR basically does these things:
1. Remove the default interface implementation of `unwrap` from `Tracer`, `Span`, and `Span.Builder`. The code would cast `this` but the classes were package-private and inaccessible to developers.
2. Implement proper logic to cast and return the delegate rather than `this` in the various implementations.
3. Slightly revise the `asParent` implementations which used `unwrap` and now just simply cast.
4. Add tests.

### Documentation
Bug fix; no doc impact

Patch:
diff --git a/tracing/providers/opentelemetry/pom.xml b/tracing/providers/opentelemetry/pom.xml
index 8fd6acc494..d4bfe8dd53 100644
--- a/tracing/providers/opentelemetry/pom.xml
+++ b/tracing/providers/opentelemetry/pom.xml
@@ -89,4 +89,20 @@
             <scope>test</scope>
         </dependency>
     </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <properties>
+                        <configurationParameters>
+                            junit.jupiter.extensions.autodetection.enabled = true
+                        </configurationParameters>
+                    </properties>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
 </project>
diff --git a/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpan.java b/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpan.java
index 519accb7f5..a4ce762353 100644
--- a/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpan.java
+++ b/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpan.java
@@ -111,6 +111,18 @@ class OpenTelemetrySpan implements Span {
         return Optional.ofNullable(baggage.getEntryValue(key));
     }
 
+    @Override
+    public <T> T unwrap(Class<T> spanClass) {
+        if (spanClass.isInstance(delegate)) {
+            return spanClass.cast(delegate);
+        }
+        if (spanClass.isInstance(this)) {
+            return spanClass.cast(this);
+        }
+        throw new IllegalArgumentException("Cannot provide an instance of " + spanClass.getName()
+                                                   + ", telemetry span is: " + delegate.getClass().getName());
+    }
+
     // Check if OTEL Context is already available in Global Helidon Context.
     // If not – use Current context.
     private static Context getContext() {
diff --git a/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanBuilder.java b/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanBuilder.java
index d7da2ed3b8..13617f87e2 100644
--- a/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanBuilder.java
+++ b/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanBuilder.java
@@ -97,6 +97,18 @@ class OpenTelemetrySpanBuilder implements Span.Builder<OpenTelemetrySpanBuilder>
         return result;
     }
 
+    @Override
+    public <T> T unwrap(Class<T> type) {
+        if (type.isInstance(spanBuilder)) {
+            return type.cast(spanBuilder);
+        }
+        if (type.isInstance(this)) {
+            return type.cast(this);
+        }
+        throw new IllegalArgumentException("Cannot provide an instance of " + type.getName()
+                                                   + ", span builder is: " + spanBuilder.getClass().getName());
+    }
+
     // used to set open telemetry context as parent, to be equivalent in function to
     // #parent(SpanContext)
     void parent(Context context) {
diff --git a/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanContext.java b/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanContext.java
index 6dad237df3..b59ea513c7 100644
--- a/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanContext.java
+++ b/tracing/providers/opentelemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanContext.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
diff --git a/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingContext.java b/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingContext.java
index b522eb1e61..a86923834d 100644
--- a/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingContext.java
+++ b/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingContext.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
diff --git a/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpan.java b/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpan.java
index c0bdafbdd0..89d4b62e09 100644
--- a/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpan.java
+++ b/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpan.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -114,9 +114,12 @@ class OpenTracingSpan implements Span {
 
     @Override
     public <T> T unwrap(Class<T> spanClass) {
-        if (spanClass.isAssignableFrom(delegate.getClass())) {
+        if (spanClass.isInstance(delegate)) {
             return spanClass.cast(delegate);
         }
+        if (spanClass.isInstance(this)) {
+            return spanClass.cast(this);
+        }
         throw new IllegalArgumentException("Cannot provide an instance of " + spanClass.getName()
                                                    + ", open tracing span is: " + delegate.getClass().getName());
     }
diff --git a/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpanBuilder.java b/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpanBuilder.java
index 16be27e0b5..563798bba4 100644
--- a/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpanBuilder.java
+++ b/tracing/providers/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpanBuilder.java
@@ -99,4 +99,16 @@ class OpenTracingSpanBuilder implements Span.Builder<OpenTracingSpanBuilder> {
         }
         return result;
     }
+
+    @Override
+    public <T> T unwrap(Class<T> type) {
+        if (type.isInstance(delegate)) {
+            return type.cast(delegate);
+        }
+        if (type.isInstance(this)) {
+            return type.cast(this);
+        }
+        throw new IllegalArgumentException("Cannot provide an instance of " + type.getName()
+                                                   + ", span builder is: " + delegate.getClass().getName());
+    }
 }
diff --git a/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java b/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java
index 8ec7a3c079..a8befddce8 100644
--- a/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java
+++ b/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -56,6 +56,15 @@ class NoOpTracer implements Tracer {
 
     }
 
+    @Override
+    public <T> T unwrap(Class<T> tracerClass) {
+        if (tracerClass.isInstance(this)) {
+            return tracerClass.cast(this);
+        }
+        throw new IllegalArgumentException("Cannot provide an instance of " + tracerClass.getName()
+                                                   + ",  tracer is: " + getClass().getName());
+    }
+
     private static class Builder implements Span.Builder<Builder> {
         @Override
         public Span build() {
@@ -91,6 +100,11 @@ class NoOpTracer implements Tracer {
         public Span start(Instant instant) {
             return SPAN;
         }
+
+        @Override
+        public <T> T unwrap(Class<T> type) {
+            return type.cast(this);
+        }
     }
 
     private static class Span implements io.helidon.tracing.Span {
@@ -146,6 +160,11 @@ class NoOpTracer implements Tracer {
         public Optional<String> baggage(String key) {
             return Optional.empty();
         }
+
+        @Override
+        public <T> T unwrap(Class<T> spanClass) {
+            return spanClass.cast(this);
+        }
     }
 
     private static class SpanContext implements io.helidon.tracing.SpanContext {
diff --git a/tracing/tracing/src/main/java/io/helidon/tracing/Tracer.java b/tracing/tracing/src/main/java/io/helidon/tracing/Tracer.java
index a92d06ff54..de3815d836 100644
--- a/tracing/tracing/src/main/java/io/helidon/tracing/Tracer.java
+++ b/tracing/tracing/src/main/java/io/helidon/tracing/Tracer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


Test patch:
diff --git a/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/OtelTestsJunitExtension.java b/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/OtelTestsJunitExtension.java
new file mode 100644
index 0000000000..f7f60c3537
--- /dev/null
+++ b/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/OtelTestsJunitExtension.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.tracing.providers.opentelemetry;
+
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.Extension;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class OtelTestsJunitExtension implements Extension, BeforeAllCallback, AfterAllCallback {
+
+    private static final String OTEL_AUTO_CONFIGURE_PROP = "otel.java.global-autoconfigure.enabled";
+    private static final String OTEL_SDK_DISABLED_PROP = "otel.sdk.disabled";
+    private String originalOtelSdkAutoConfiguredSetting;
+    private String originalOtelSdkDisabledSetting;
+
+    @Override
+    public void afterAll(ExtensionContext extensionContext) throws Exception {
+        if (originalOtelSdkAutoConfiguredSetting != null) {
+            System.setProperty(OTEL_AUTO_CONFIGURE_PROP, originalOtelSdkAutoConfiguredSetting);
+        }
+        if (originalOtelSdkDisabledSetting != null) {
+            System.setProperty(OTEL_SDK_DISABLED_PROP, originalOtelSdkDisabledSetting);
+        }
+    }
+
+    @Override
+    public void beforeAll(ExtensionContext extensionContext) throws Exception {
+        originalOtelSdkAutoConfiguredSetting = System.setProperty(OTEL_AUTO_CONFIGURE_PROP, "true");
+        originalOtelSdkDisabledSetting = System.setProperty(OTEL_SDK_DISABLED_PROP, "false");
+    }
+}
diff --git a/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestSpanAndBaggage.java b/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestSpanAndBaggage.java
index bc782f0a95..69586c4845 100644
--- a/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestSpanAndBaggage.java
+++ b/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestSpanAndBaggage.java
@@ -26,8 +26,6 @@ import io.helidon.tracing.Span;
 import io.helidon.tracing.SpanContext;
 import io.helidon.tracing.Tracer;
 
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -37,27 +35,6 @@ import static org.hamcrest.Matchers.not;
 
 class TestSpanAndBaggage {
 
-    private static final String OTEL_AUTO_CONFIGURE_PROP = "otel.java.global-autoconfigure.enabled";
-    private static final String OTEL_SDK_DISABLED_PROP = "otel.sdk.disabled";
-    private static String originalOtelSdkAutoConfiguredSetting;
-    private static String originalOtelSdkDisabledSetting;
-
-    @BeforeAll
-    static void init() {
-        originalOtelSdkAutoConfiguredSetting = System.setProperty(OTEL_AUTO_CONFIGURE_PROP, "true");
-        originalOtelSdkDisabledSetting = System.setProperty(OTEL_SDK_DISABLED_PROP, "false");
-    }
-
-    @AfterAll
-    static void wrapup() {
-        if (originalOtelSdkAutoConfiguredSetting != null) {
-            System.setProperty(OTEL_AUTO_CONFIGURE_PROP, originalOtelSdkAutoConfiguredSetting);
-        }
-        if (originalOtelSdkDisabledSetting != null) {
-            System.setProperty(OTEL_SDK_DISABLED_PROP, originalOtelSdkDisabledSetting);
-        }
-    }
-
     @Test
     void testActiveSpanScopeWithoutBaggage() {
         Tracer tracer = Tracer.global();
diff --git a/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestUnwrap.java b/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestUnwrap.java
new file mode 100644
index 0000000000..d654364844
--- /dev/null
+++ b/tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestUnwrap.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.tracing.providers.opentelemetry;
+
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanBuilder;
+import io.opentelemetry.api.trace.Tracer;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+
+class TestUnwrap {
+
+    @Test
+    void testTracer() {
+        var tracer = io.helidon.tracing.Tracer.global();
+        assertThat("Tracer unwrapped",
+                   tracer.unwrap(Tracer.class),
+                   instanceOf(Tracer.class));
+    }
+
+    @Test
+    void testSpanAndSpanBuilder() {
+        var tracer = io.helidon.tracing.Tracer.global();
+        var spanBuilder = tracer.spanBuilder("test1");
+        assertThat("Span builder unwrapped",
+                   spanBuilder.unwrap(SpanBuilder.class),
+                   instanceOf(SpanBuilder.class));
+
+        var span = spanBuilder.start();
+        assertThat("Span unwrapped",
+                   span.unwrap(Span.class),
+                   instanceOf(Span.class));
+
+    }
+}
diff --git a/tracing/providers/opentelemetry/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension b/tracing/providers/opentelemetry/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
new file mode 100644
index 0000000000..8145a36796
--- /dev/null
+++ b/tracing/providers/opentelemetry/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension
@@ -0,0 +1,16 @@
+#
+# Copyright (c) 2024 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+io.helidon.tracing.providers.opentelemetry.OtelTestsJunitExtension
\ No newline at end of file
diff --git a/tracing/providers/opentracing/src/test/java/io/helidon/tracing/providers/opentracing/TestUnwrap.java b/tracing/providers/opentracing/src/test/java/io/helidon/tracing/providers/opentracing/TestUnwrap.java
new file mode 100644
index 0000000000..c727255865
--- /dev/null
+++ b/tracing/providers/opentracing/src/test/java/io/helidon/tracing/providers/opentracing/TestUnwrap.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.tracing.providers.opentracing;
+
+import io.opentracing.Span;
+import io.opentracing.Tracer;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+
+class TestUnwrap {
+
+    @Test
+    void testTracer() {
+        var tracer = io.helidon.tracing.Tracer.global();
+        assertThat("Tracer unwrapped",
+                   tracer.unwrap(Tracer.class),
+                   instanceOf(Tracer.class));
+    }
+
+    @Test
+    void testSpanAndSpanBuilder() {
+        var tracer = io.helidon.tracing.Tracer.global();
+        var spanBuilder = tracer.spanBuilder("test1");
+        assertThat("Span builder unwrapped",
+                   spanBuilder.unwrap(Tracer.SpanBuilder.class),
+                   instanceOf(Tracer.SpanBuilder.class));
+
+        var span = spanBuilder.start();
+        assertThat("Span unwrapped",
+                   span.unwrap(Span.class),
+                   instanceOf(Span.class));
+
+    }
+}