Fix unwrap methods to return cast delegates rather than cast 'this' (#…

…8298) * Fix unwrap methods to return cast delegates rather than cast 'this' Signed-off-by: Tim Quinn <tim.quinn@oracle.com> * Use a JUnit extension to set-up and shutdown OTel for tests * Review comments: add explicit type checking and exception throwing in unwrap implementations instead of simply delegating to Class.cast * Enhance unwrap to handle package-local and public implementations * Enhance unwrap for spans and no-op tracer also * Restore default implementations of unwrap --------- Signed-off-by: Tim Quinn <tim.quinn@oracle.com>
helidon-io · Jan 30, 2024 · 52a5a1b · 52a5a1b
1 parent 52d8a5d
commit 52a5a1b
Show file tree

Hide file tree

Showing 14 changed files with 241 additions and 29 deletions.
diff --git 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/...telemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpan.java b/...telemetry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpan.java
@@ -111,6 +111,18 @@ public Optional<String> baggage(String key) {
         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/...ry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanBuilder.java b/...ry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanBuilder.java
@@ -97,6 +97,18 @@ public Span start(Instant instant) {
         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/...ry/src/main/java/io/helidon/tracing/providers/opentelemetry/OpenTelemetrySpanContext.java b/...ry/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/...try/src/test/java/io/helidon/tracing/providers/opentelemetry/OtelTestsJunitExtension.java b/...try/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/...elemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestSpanAndBaggage.java b/...elemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestSpanAndBaggage.java
@@ -26,8 +26,6 @@
 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 @@
 
 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/...rs/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestUnwrap.java b/...rs/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/...ntelemetry/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension b/...ntelemetry/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
diff --git a/...pentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingContext.java b/...pentracing/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/...s/opentracing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpan.java b/...s/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 @@ public Optional<String> baggage(String key) {
 
     @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/...racing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpanBuilder.java b/...racing/src/main/java/io/helidon/tracing/providers/opentracing/OpenTracingSpanBuilder.java
@@ -99,4 +99,16 @@ public Span start(Instant instant) {
         }
         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/...viders/opentracing/src/test/java/io/helidon/tracing/providers/opentracing/TestUnwrap.java b/...viders/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));
+
+    }
+}
diff --git 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 @@ public void inject(io.helidon.tracing.SpanContext spanContext,
 
     }
 
+    @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 @@ public Builder tag(String key, Number value) {
         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 @@ public Span baggage(String key, String value) {
         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
@@ -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.