From b3abb03b41bd58c471a03c41a7c40ecc88b24160 Mon Sep 17 00:00:00 2001 From: Tim Quinn Date: Thu, 25 Jan 2024 10:34:00 -0600 Subject: [PATCH 1/6] Fix unwrap methods to return cast delegates rather than cast 'this' Signed-off-by: Tim Quinn --- .../opentelemetry/OpenTelemetrySpan.java | 5 ++ .../OpenTelemetrySpanBuilder.java | 5 ++ .../OpenTelemetrySpanContext.java | 5 +- .../providers/opentelemetry/TestUnwrap.java | 51 +++++++++++++++++++ .../opentracing/OpenTracingContext.java | 5 +- .../opentracing/OpenTracingSpanBuilder.java | 5 ++ .../providers/opentracing/TestUnwrap.java | 49 ++++++++++++++++++ .../java/io/helidon/tracing/NoOpTracer.java | 17 ++++++- .../main/java/io/helidon/tracing/Span.java | 16 +----- .../main/java/io/helidon/tracing/Tracer.java | 10 +--- 10 files changed, 139 insertions(+), 29 deletions(-) create mode 100644 tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/TestUnwrap.java create mode 100644 tracing/providers/opentracing/src/test/java/io/helidon/tracing/providers/opentracing/TestUnwrap.java 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 519accb7f54..7989639a55d 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,11 @@ public Optional baggage(String key) { return Optional.ofNullable(baggage.getEntryValue(key)); } + @Override + public T unwrap(Class spanClass) { + return spanClass.cast(delegate); + } + // 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 d7da2ed3b87..b2e635509f6 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,11 @@ public Span start(Instant instant) { return result; } + @Override + public T unwrap(Class type) { + return type.cast(spanBuilder); + } + // 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 6dad237df31..77bd7d1b887 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. @@ -39,8 +39,7 @@ public String spanId() { @Override public void asParent(io.helidon.tracing.Span.Builder spanBuilder) { - spanBuilder.unwrap(OpenTelemetrySpanBuilder.class) - .parent(context); + ((OpenTelemetrySpanBuilder) spanBuilder).parent(context); } Context openTelemetry() { 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 00000000000..d654364844f --- /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/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 b522eb1e611..7e2565f0fc0 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. @@ -38,8 +38,7 @@ public String spanId() { @Override public void asParent(Span.Builder spanBuilder) { - spanBuilder.unwrap(OpenTracingSpanBuilder.class) - .parent(this); + ((OpenTracingSpanBuilder) spanBuilder).parent(this); } SpanContext openTracing() { 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 16be27e0b59..c664c88d4e6 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,9 @@ public Span start(Instant instant) { } return result; } + + @Override + public T unwrap(Class type) { + return type.cast(delegate); + } } 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 00000000000..c7272558658 --- /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)); + + } +} 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 8ec7a3c0795..b3324213962 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,11 @@ public void inject(io.helidon.tracing.SpanContext spanContext, } + @Override + public T unwrap(Class tracerClass) { + return tracerClass.cast(this); + } + private static class Builder implements Span.Builder { @Override public Span build() { @@ -91,6 +96,11 @@ public Builder tag(String key, Number value) { public Span start(Instant instant) { return SPAN; } + + @Override + public T unwrap(Class type) { + return type.cast(this); + } } private static class Span implements io.helidon.tracing.Span { @@ -146,6 +156,11 @@ public Span baggage(String key, String value) { public Optional baggage(String key) { return Optional.empty(); } + + @Override + public T unwrap(Class 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/Span.java b/tracing/tracing/src/main/java/io/helidon/tracing/Span.java index 0d0d4f2f16b..0cca8f7111a 100644 --- a/tracing/tracing/src/main/java/io/helidon/tracing/Span.java +++ b/tracing/tracing/src/main/java/io/helidon/tracing/Span.java @@ -157,13 +157,7 @@ default void addEvent(String logMessage) { * @return instance of the span * @throws java.lang.IllegalArgumentException in case the span cannot provide the expected type */ - default T unwrap(Class spanClass) { - try { - return spanClass.cast(this); - } catch (ClassCastException e) { - throw new IllegalArgumentException("This span is not compatible with " + spanClass.getName()); - } - } + T unwrap(Class spanClass); /** * Span kind. @@ -297,12 +291,6 @@ default Span start() { * @throws java.lang.IllegalArgumentException when the expected type is not the actual type, or the builder cannot be * coerced into that type */ - default T unwrap(Class type) { - if (type.isAssignableFrom(getClass())) { - return type.cast(this); - } - throw new IllegalArgumentException("This instance cannot be unwrapped to " + type.getName() - + ", this builder: " + getClass().getName()); - } + T unwrap(Class type); } } 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 a92d06ff540..48c1a1fda9e 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. @@ -94,11 +94,5 @@ static void global(Tracer tracer) { * @param type of the tracer * @throws java.lang.IllegalArgumentException in case the tracer cannot provide the expected type */ - default T unwrap(Class tracerClass) { - try { - return tracerClass.cast(this); - } catch (ClassCastException e) { - throw new IllegalArgumentException("This tracer is not compatible with " + tracerClass.getName()); - } - } + T unwrap(Class tracerClass); } From bfef07fb0dbde6895cfec434a1a13fd75dbba837 Mon Sep 17 00:00:00 2001 From: Tim Quinn Date: Thu, 25 Jan 2024 17:03:27 -0600 Subject: [PATCH 2/6] Use a JUnit extension to set-up and shutdown OTel for tests --- tracing/providers/opentelemetry/pom.xml | 16 +++++++ .../OtelTestsJunitExtension.java | 45 +++++++++++++++++++ .../opentelemetry/TestSpanAndBaggage.java | 23 ---------- .../org.junit.jupiter.api.extension.Extension | 16 +++++++ 4 files changed, 77 insertions(+), 23 deletions(-) create mode 100644 tracing/providers/opentelemetry/src/test/java/io/helidon/tracing/providers/opentelemetry/OtelTestsJunitExtension.java create mode 100644 tracing/providers/opentelemetry/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension diff --git a/tracing/providers/opentelemetry/pom.xml b/tracing/providers/opentelemetry/pom.xml index 8fd6acc4942..d4bfe8dd53c 100644 --- a/tracing/providers/opentelemetry/pom.xml +++ b/tracing/providers/opentelemetry/pom.xml @@ -89,4 +89,20 @@ test + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + + junit.jupiter.extensions.autodetection.enabled = true + + + + + + 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 00000000000..f7f60c3537e --- /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 bc782f0a95f..69586c48450 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.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/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 00000000000..8145a367963 --- /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 From c480839413e873da619d9f8e1d7228b677c227af Mon Sep 17 00:00:00 2001 From: Tim Quinn Date: Mon, 29 Jan 2024 11:50:37 -0600 Subject: [PATCH 3/6] Review comments: add explicit type checking and exception throwing in unwrap implementations instead of simply delegating to Class.cast --- .../tracing/providers/opentelemetry/OpenTelemetrySpan.java | 6 +++++- .../providers/opentelemetry/OpenTelemetrySpanBuilder.java | 6 +++++- .../providers/opentracing/OpenTracingSpanBuilder.java | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) 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 7989639a55d..ed6f5ca3413 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 @@ -113,7 +113,11 @@ public Optional baggage(String key) { @Override public T unwrap(Class spanClass) { - return spanClass.cast(delegate); + if (spanClass.isInstance(delegate)) { + return spanClass.cast(delegate); + } + 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. 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 b2e635509f6..ffab0cf418e 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 @@ -99,7 +99,11 @@ public Span start(Instant instant) { @Override public T unwrap(Class type) { - return type.cast(spanBuilder); + if (type.isInstance(spanBuilder)) { + return type.cast(spanBuilder); + } + 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 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 c664c88d4e6..c913acfce6f 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 @@ -102,6 +102,10 @@ public Span start(Instant instant) { @Override public T unwrap(Class type) { - return type.cast(delegate); + if (type.isInstance(delegate)) { + return type.cast(delegate); + } + throw new IllegalArgumentException("Cannot provide an instance of " + type.getName() + + ", span builder is: " + delegate.getClass().getName()); } } From faff286ed9c3786a4eccb769c48666518ab7f193 Mon Sep 17 00:00:00 2001 From: Tim Quinn Date: Tue, 30 Jan 2024 10:52:21 -0600 Subject: [PATCH 4/6] Enhance unwrap to handle package-local and public implementations --- .../providers/opentelemetry/OpenTelemetrySpanBuilder.java | 3 +++ .../providers/opentelemetry/OpenTelemetrySpanContext.java | 3 ++- .../tracing/providers/opentracing/OpenTracingContext.java | 3 ++- .../tracing/providers/opentracing/OpenTracingSpanBuilder.java | 3 +++ 4 files changed, 10 insertions(+), 2 deletions(-) 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 ffab0cf418e..13617f87e28 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 @@ -102,6 +102,9 @@ public T unwrap(Class 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()); } 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 77bd7d1b887..b59ea513c78 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 @@ -39,7 +39,8 @@ public String spanId() { @Override public void asParent(io.helidon.tracing.Span.Builder spanBuilder) { - ((OpenTelemetrySpanBuilder) spanBuilder).parent(context); + spanBuilder.unwrap(OpenTelemetrySpanBuilder.class) + .parent(context); } Context openTelemetry() { 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 7e2565f0fc0..a86923834dc 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 @@ -38,7 +38,8 @@ public String spanId() { @Override public void asParent(Span.Builder spanBuilder) { - ((OpenTracingSpanBuilder) spanBuilder).parent(this); + spanBuilder.unwrap(OpenTracingSpanBuilder.class) + .parent(this); } SpanContext openTracing() { 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 c913acfce6f..563798bba49 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 @@ -105,6 +105,9 @@ public T unwrap(Class 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()); } From 08125654cc5350b788072555bd490f9de26c3377 Mon Sep 17 00:00:00 2001 From: Tim Quinn Date: Tue, 30 Jan 2024 11:18:05 -0600 Subject: [PATCH 5/6] Enhance unwrap for spans and no-op tracer also --- .../tracing/providers/opentelemetry/OpenTelemetrySpan.java | 3 +++ .../tracing/providers/opentracing/OpenTracingSpan.java | 7 +++++-- .../src/main/java/io/helidon/tracing/NoOpTracer.java | 6 +++++- 3 files changed, 13 insertions(+), 3 deletions(-) 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 ed6f5ca3413..a4ce762353b 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 @@ -116,6 +116,9 @@ public T unwrap(Class 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()); } 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 c0bdafbdd05..89d4b62e099 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 @@ public Optional baggage(String key) { @Override public T unwrap(Class 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/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java b/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java index b3324213962..a8befddce87 100644 --- a/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java +++ b/tracing/tracing/src/main/java/io/helidon/tracing/NoOpTracer.java @@ -58,7 +58,11 @@ public void inject(io.helidon.tracing.SpanContext spanContext, @Override public T unwrap(Class tracerClass) { - return tracerClass.cast(this); + 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 { From ae045a44c4eb0463a3a26a068667c1f85de4a39d Mon Sep 17 00:00:00 2001 From: Tim Quinn Date: Tue, 30 Jan 2024 11:53:37 -0600 Subject: [PATCH 6/6] Restore default implementations of unwrap --- .../src/main/java/io/helidon/tracing/Span.java | 16 ++++++++++++++-- .../src/main/java/io/helidon/tracing/Tracer.java | 8 +++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/tracing/tracing/src/main/java/io/helidon/tracing/Span.java b/tracing/tracing/src/main/java/io/helidon/tracing/Span.java index 0cca8f7111a..0d0d4f2f16b 100644 --- a/tracing/tracing/src/main/java/io/helidon/tracing/Span.java +++ b/tracing/tracing/src/main/java/io/helidon/tracing/Span.java @@ -157,7 +157,13 @@ default void addEvent(String logMessage) { * @return instance of the span * @throws java.lang.IllegalArgumentException in case the span cannot provide the expected type */ - T unwrap(Class spanClass); + default T unwrap(Class spanClass) { + try { + return spanClass.cast(this); + } catch (ClassCastException e) { + throw new IllegalArgumentException("This span is not compatible with " + spanClass.getName()); + } + } /** * Span kind. @@ -291,6 +297,12 @@ default Span start() { * @throws java.lang.IllegalArgumentException when the expected type is not the actual type, or the builder cannot be * coerced into that type */ - T unwrap(Class type); + default T unwrap(Class type) { + if (type.isAssignableFrom(getClass())) { + return type.cast(this); + } + throw new IllegalArgumentException("This instance cannot be unwrapped to " + type.getName() + + ", this builder: " + getClass().getName()); + } } } 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 48c1a1fda9e..de3815d8360 100644 --- a/tracing/tracing/src/main/java/io/helidon/tracing/Tracer.java +++ b/tracing/tracing/src/main/java/io/helidon/tracing/Tracer.java @@ -94,5 +94,11 @@ static void global(Tracer tracer) { * @param type of the tracer * @throws java.lang.IllegalArgumentException in case the tracer cannot provide the expected type */ - T unwrap(Class tracerClass); + default T unwrap(Class tracerClass) { + try { + return tracerClass.cast(this); + } catch (ClassCastException e) { + throw new IllegalArgumentException("This tracer is not compatible with " + tracerClass.getName()); + } + } }