diff --git a/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/ProxyingSpan.java b/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/DelegatingSpan.java similarity index 67% rename from opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/ProxyingSpan.java rename to opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/DelegatingSpan.java index f0ad9c69b0a..f32e8b204d1 100644 --- a/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/ProxyingSpan.java +++ b/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/DelegatingSpan.java @@ -18,7 +18,7 @@ /** * Delegates all {@link Span} methods to some underlying Span via {@link - * ProxyingSpan#getProxied()}. + * DelegatingSpan#getDelegate()}. * *

If not all calls are proxied, some, such as and in particular {@link * Span#storeInContext(Context)} and {@link Span#makeCurrent()}, will use {@code this} instead of @@ -30,141 +30,135 @@ * from the shim. * *

This addresses the inconsistency where not all methods are appropriately delegated by exposing - * a single method, {@link ProxyingSpan#getProxied()}, to simplify and better ensure delegation and - * meeting expectations. + * a single method, {@link DelegatingSpan#getDelegate()}, to simplify and better ensure delegation + * and meeting expectations. */ -// todo make this unnecessary -@SuppressWarnings("UngroupedOverloads") -interface ProxyingSpan extends Span { - Span getProxied(); - - // implementations +interface DelegatingSpan extends Span { + Span getDelegate(); @Override - default Span setAttribute(AttributeKey key, T value) { - return getProxied().setAttribute(key, value); + default Span updateName(String name) { + return getDelegate().updateName(name); } @Override - default Span addEvent(String name, Attributes attributes) { - return getProxied().addEvent(name, attributes); + default SpanContext getSpanContext() { + return getDelegate().getSpanContext(); } @Override - default Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) { - return getProxied().addEvent(name, attributes, timestamp, unit); + default boolean isRecording() { + return getDelegate().isRecording(); } @Override - default Span setStatus(StatusCode statusCode, String description) { - return getProxied().setStatus(statusCode, description); + default Span setAttribute(AttributeKey key, T value) { + return getDelegate().setAttribute(key, value); } @Override - default Span recordException(Throwable exception, Attributes additionalAttributes) { - return getProxied().recordException(exception, additionalAttributes); + default Span setAttribute(String key, String value) { + return getDelegate().setAttribute(key, value); } @Override - default Span updateName(String name) { - return getProxied().updateName(name); + default Span setAttribute(String key, long value) { + return getDelegate().setAttribute(key, value); } @Override - default void end() { - getProxied().end(); + default Span setAttribute(String key, double value) { + return getDelegate().setAttribute(key, value); } @Override - default void end(long timestamp, TimeUnit unit) { - getProxied().end(timestamp, unit); + default Span setAttribute(String key, boolean value) { + return getDelegate().setAttribute(key, value); } @Override - default SpanContext getSpanContext() { - return getProxied().getSpanContext(); + default Span setAttribute(AttributeKey key, int value) { + return getDelegate().setAttribute(key, value); } @Override - default boolean isRecording() { - return getProxied().isRecording(); + default Span setAllAttributes(Attributes attributes) { + return getDelegate().setAllAttributes(attributes); } - // default overrides - @Override - default Span setAttribute(String key, String value) { - return getProxied().setAttribute(key, value); + default Span addEvent(String name, Attributes attributes) { + return getDelegate().addEvent(name, attributes); } @Override - default Span setAttribute(String key, long value) { - return getProxied().setAttribute(key, value); + default Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) { + return getDelegate().addEvent(name, attributes, timestamp, unit); } @Override - default Span setAttribute(String key, double value) { - return getProxied().setAttribute(key, value); + default Span addEvent(String name) { + return getDelegate().addEvent(name); } @Override - default Span setAttribute(String key, boolean value) { - return getProxied().setAttribute(key, value); + default Span addEvent(String name, long timestamp, TimeUnit unit) { + return getDelegate().addEvent(name, timestamp, unit); } @Override - default Span setAttribute(AttributeKey key, int value) { - return getProxied().setAttribute(key, value); + default Span addEvent(String name, Instant timestamp) { + return getDelegate().addEvent(name, timestamp); } @Override - default Span setAllAttributes(Attributes attributes) { - return getProxied().setAllAttributes(attributes); + default Span addEvent(String name, Attributes attributes, Instant timestamp) { + return getDelegate().addEvent(name, attributes, timestamp); } @Override - default Span addEvent(String name) { - return getProxied().addEvent(name); + default Span setStatus(StatusCode statusCode, String description) { + return getDelegate().setStatus(statusCode, description); } @Override - default Span addEvent(String name, long timestamp, TimeUnit unit) { - return getProxied().addEvent(name, timestamp, unit); + default Span setStatus(StatusCode statusCode) { + return getDelegate().setStatus(statusCode); } @Override - default Span addEvent(String name, Instant timestamp) { - return getProxied().addEvent(name, timestamp); + default Span recordException(Throwable exception, Attributes additionalAttributes) { + return getDelegate().recordException(exception, additionalAttributes); } @Override - default Span addEvent(String name, Attributes attributes, Instant timestamp) { - return getProxied().addEvent(name, attributes, timestamp); + default Span recordException(Throwable exception) { + return getDelegate().recordException(exception); } @Override - default Span setStatus(StatusCode statusCode) { - return getProxied().setStatus(statusCode); + default void end(Instant timestamp) { + getDelegate().end(timestamp); } @Override - default Span recordException(Throwable exception) { - return getProxied().recordException(exception); + default void end() { + getDelegate().end(); } @Override - default void end(Instant timestamp) { - getProxied().end(timestamp); + default void end(long timestamp, TimeUnit unit) { + getDelegate().end(timestamp, unit); } @Override default Context storeInContext(Context context) { - return getProxied().storeInContext(context); + return getDelegate().storeInContext(context); } @MustBeClosed @Override default Scope makeCurrent() { - return getProxied().makeCurrent(); + return getDelegate().makeCurrent(); } } diff --git a/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanImpl.java b/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanImpl.java index ae0f517afa8..8d25f722786 100644 --- a/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanImpl.java +++ b/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanImpl.java @@ -48,7 +48,8 @@ import java.util.Map; import java.util.logging.Logger; -class OpenTelemetrySpanImpl extends Span implements io.opentelemetry.api.trace.Span, ProxyingSpan { +class OpenTelemetrySpanImpl extends Span + implements io.opentelemetry.api.trace.Span, DelegatingSpan { private static final Logger LOGGER = Logger.getLogger(OpenTelemetrySpanImpl.class.getName()); private static final EnumSet RECORD_EVENTS_SPAN_OPTIONS = EnumSet.of(Span.Options.RECORD_EVENTS); @@ -63,7 +64,7 @@ class OpenTelemetrySpanImpl extends Span implements io.opentelemetry.api.trace.S // otel @Override - public io.opentelemetry.api.trace.Span getProxied() { + public io.opentelemetry.api.trace.Span getDelegate() { return otelSpan; } @@ -74,10 +75,10 @@ public void putAttribute(String key, AttributeValue value) { Preconditions.checkNotNull(key, "key"); Preconditions.checkNotNull(value, "value"); value.match( - arg -> ProxyingSpan.super.setAttribute(key, arg), - arg -> ProxyingSpan.super.setAttribute(key, arg), - arg -> ProxyingSpan.super.setAttribute(key, arg), - arg -> ProxyingSpan.super.setAttribute(key, arg), + arg -> DelegatingSpan.super.setAttribute(key, arg), + arg -> DelegatingSpan.super.setAttribute(key, arg), + arg -> DelegatingSpan.super.setAttribute(key, arg), + arg -> DelegatingSpan.super.setAttribute(key, arg), arg -> null); } @@ -91,14 +92,14 @@ public void putAttributes(Map attributes) { public void addAnnotation(String description, Map attributes) { AttributesBuilder attributesBuilder = Attributes.builder(); mapAttributes(attributes, attributesBuilder); - ProxyingSpan.super.addEvent(description, attributesBuilder.build()); + DelegatingSpan.super.addEvent(description, attributesBuilder.build()); } @Override public void addAnnotation(Annotation annotation) { AttributesBuilder attributesBuilder = Attributes.builder(); mapAttributes(annotation.getAttributes(), attributesBuilder); - ProxyingSpan.super.addEvent(annotation.getDescription(), attributesBuilder.build()); + DelegatingSpan.super.addEvent(annotation.getDescription(), attributesBuilder.build()); } @Override @@ -108,7 +109,7 @@ public void addLink(Link link) { @Override public void addMessageEvent(MessageEvent messageEvent) { - ProxyingSpan.super.addEvent( + DelegatingSpan.super.addEvent( String.valueOf(messageEvent.getMessageId()), Attributes.of( AttributeKey.stringKey(MESSAGE_EVENT_ATTRIBUTE_KEY_TYPE), @@ -122,22 +123,22 @@ public void addMessageEvent(MessageEvent messageEvent) { @Override public void setStatus(Status status) { Preconditions.checkNotNull(status, "status"); - ProxyingSpan.super.setStatus(status.isOk() ? StatusCode.OK : StatusCode.ERROR); + DelegatingSpan.super.setStatus(status.isOk() ? StatusCode.OK : StatusCode.ERROR); } @Override public io.opentelemetry.api.trace.Span setStatus(StatusCode canonicalCode, String description) { - return ProxyingSpan.super.setStatus(canonicalCode, description); + return DelegatingSpan.super.setStatus(canonicalCode, description); } @Override public void end(EndSpanOptions options) { - ProxyingSpan.super.end(); + DelegatingSpan.super.end(); } @Override public SpanContext getSpanContext() { - return ProxyingSpan.super.getSpanContext(); + return DelegatingSpan.super.getSpanContext(); } @Override diff --git a/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/ProxyingSpanTest.java b/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/DelegatingSpanTest.java similarity index 93% rename from opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/ProxyingSpanTest.java rename to opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/DelegatingSpanTest.java index f77319d3c92..88b63c019bb 100644 --- a/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/ProxyingSpanTest.java +++ b/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/DelegatingSpanTest.java @@ -31,7 +31,7 @@ * and as instrumented via the javaagent. Details here. */ -class ProxyingSpanTest { +class DelegatingSpanTest { /* Verifies all methods on the otel Span interface are under test. @@ -41,7 +41,7 @@ flexibility for special cases (e.g. getSpanContext() and isRecording()) @Test void verifyAllMethodsAreUnderTest() { List methods = - proxyMethodsProvider() + delegateMethodsProvider() .map( pm -> { try { @@ -62,13 +62,13 @@ void verifyAllMethodsAreUnderTest() { } @ParameterizedTest - @MethodSource("proxyMethodsProvider") + @MethodSource("delegateMethodsProvider") void testit(String name, Class[] params, VerificationMode timesCalled) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { Method method = getInterfaceMethod(Span.class, name, params); Span mockedSpan = Mockito.spy(Span.current()); OpenTelemetrySpanImpl span = new OpenTelemetrySpanImpl(mockedSpan); - assertProxied(span, mockedSpan, method, timesCalled); + assertDelegated(span, mockedSpan, method, timesCalled); } static List allInterfaceMethods(Class clazz) { @@ -77,8 +77,7 @@ static List allInterfaceMethods(Class clazz) { .collect(Collectors.toList()); } - static Stream proxyMethodsProvider() { - // todo fill in remaining methods + static Stream delegateMethodsProvider() { return Stream.of( Arguments.of("end", new Class[] {}, times(1)), Arguments.of( @@ -145,7 +144,7 @@ static Stream proxyMethodsProvider() { // called never -- it's shared between OC and Otel Span types and is always true, so returns // `true` Arguments.of("isRecording", new Class[] {}, times(0)), - // called twice: once in constructor, then once during proxy + // called twice: once in constructor, then once during delegation Arguments.of("getSpanContext", new Class[] {}, times(2))); } @@ -175,14 +174,14 @@ static Object valueLookup(Class clazz) { } } - static Method getInterfaceMethod(Class proxy, String name, Class[] params) + static Method getInterfaceMethod(Class clazz, String name, Class[] params) throws NoSuchMethodException { - Method method = proxy.getMethod(name, params); + Method method = clazz.getMethod(name, params); assertThat(method).isNotNull(); return method; } - static void assertProxied(T proxy, T delegate, Method method, VerificationMode mode) + static void assertDelegated(T proxy, T delegate, Method method, VerificationMode mode) throws InvocationTargetException, IllegalAccessException { // Get parameters for method Class[] parameterTypes = method.getParameterTypes();