diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesGetter.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesGetter.java index b05f9bd4331b..397cd3711fcf 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesGetter.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesGetter.java @@ -16,7 +16,23 @@ */ public interface RpcAttributesGetter { - @Nullable + /** + * Returns the stable semconv system name for the RPC framework (e.g. {@code "grpc"}, {@code + * "java_rmi"}, {@code "dotnet_wcf"}). + * + * @see rpc.system.name + * spec + */ + @SuppressWarnings("deprecation") + default String getRpcSystemName(REQUEST request) { + return getSystem(request); + } + + /** + * @deprecated Use {@link #getRpcSystemName(REQUEST)}. To be removed in 3.0. + */ + @Deprecated String getSystem(REQUEST request); @Nullable @@ -47,6 +63,7 @@ default Long getResponseSize(REQUEST request) { * method is unavailable */ @Nullable + // TODO remove default implementation default String getRpcMethod(REQUEST request) { return null; } @@ -56,15 +73,16 @@ default String getRpcMethod(REQUEST request) { * *

This method should return {@code null} if there was no error. * - *

If this method is not implemented, or if it returns {@code null}, the exception class name - * will be used as error type. + *

If this method returns {@code null}, the exception class name will be used as error type if + * one was thrown. * *

The cardinality of the error type should be low. The instrumentations implementing this * method are recommended to document the custom values they support. * - *

Examples: {@code OK}, {@code CANCELLED}, {@code UNKNOWN}, {@code -32602} + *

Examples: {@code CANCELLED}, {@code UNKNOWN}, {@code -32602} */ @Nullable + // TODO remove default implementation default String getErrorType( REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) { return null; diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java index e85e80bad44f..724738af2196 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java @@ -5,6 +5,10 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.rpc; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitOldRpcSemconv; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableRpcSemconv; +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; @@ -14,9 +18,15 @@ abstract class RpcCommonAttributesExtractor implements AttributesExtractor { - // copied from RpcIncubatingAttributes static final AttributeKey RPC_METHOD = AttributeKey.stringKey("rpc.method"); + + // Stable semconv keys + static final AttributeKey RPC_SYSTEM_NAME = AttributeKey.stringKey("rpc.system.name"); + + // removed in stable semconv (merged into rpc.method) static final AttributeKey RPC_SERVICE = AttributeKey.stringKey("rpc.service"); + + // use RPC_SYSTEM_NAME for stable semconv static final AttributeKey RPC_SYSTEM = AttributeKey.stringKey("rpc.system"); private final RpcAttributesGetter getter; @@ -25,12 +35,23 @@ abstract class RpcCommonAttributesExtractor this.getter = getter; } - @SuppressWarnings("deprecation") // for getMethod() + @SuppressWarnings("deprecation") // for getSystem(), getMethod() @Override public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) { - attributes.put(RPC_SYSTEM, getter.getSystem(request)); - attributes.put(RPC_SERVICE, getter.getService(request)); - attributes.put(RPC_METHOD, getter.getMethod(request)); + + if (emitStableRpcSemconv()) { + attributes.put(RPC_SYSTEM_NAME, getter.getRpcSystemName(request)); + attributes.put(RPC_METHOD, getter.getRpcMethod(request)); + } + + if (emitOldRpcSemconv()) { + attributes.put(RPC_SYSTEM, getter.getSystem(request)); + attributes.put(RPC_SERVICE, getter.getService(request)); + if (!emitStableRpcSemconv()) { + // only set old rpc.method on spans when there's no clash with stable rpc.method + attributes.put(RPC_METHOD, getter.getMethod(request)); + } + } } @Override @@ -40,6 +61,13 @@ public final void onEnd( REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) { - // No response attributes + if (emitStableRpcSemconv()) { + String errorType = getter.getErrorType(request, response, error); + // fall back to exception class name + if (errorType == null && error != null) { + errorType = error.getClass().getName(); + } + attributes.put(ERROR_TYPE, errorType); + } } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractor.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractor.java index 41fb57a7a023..45ef81b80bd9 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractor.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractor.java @@ -5,6 +5,8 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.rpc; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableRpcSemconv; + import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; /** A {@link SpanNameExtractor} for RPC requests. */ @@ -28,6 +30,15 @@ private RpcSpanNameExtractor(RpcAttributesGetter getter) { @SuppressWarnings("deprecation") // for getMethod() @Override public String extract(REQUEST request) { + if (emitStableRpcSemconv()) { + String method = getter.getRpcMethod(request); + if (method != null) { + return method; + } + // fall back to rpc.system.name + return getter.getRpcSystemName(request); + } + String service = getter.getService(request); String method = getter.getMethod(request); if (service == null || method == null) { diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesExtractorTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesExtractorTest.java index 20427a4a294e..dde44959a5e0 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesExtractorTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesExtractorTest.java @@ -5,25 +5,36 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.rpc; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitOldRpcSemconv; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableRpcSemconv; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; import static io.opentelemetry.semconv.incubating.RpcIncubatingAttributes.RPC_METHOD; import static io.opentelemetry.semconv.incubating.RpcIncubatingAttributes.RPC_SERVICE; import static io.opentelemetry.semconv.incubating.RpcIncubatingAttributes.RPC_SYSTEM; import static org.assertj.core.api.Assertions.entry; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import javax.annotation.Nullable; import org.junit.jupiter.api.Test; @SuppressWarnings("deprecation") // using deprecated semconv class RpcAttributesExtractorTest { - enum TestGetter implements RpcAttributesGetter, Void> { - INSTANCE; + private static class TestGetter implements RpcAttributesGetter, Void> { + + @Override + public String getRpcSystemName(Map request) { + return "test"; + } @Override public String getSystem(Map request) { @@ -40,18 +51,40 @@ public String getService(Map request) { public String getMethod(Map request) { return request.get("method"); } + + @Nullable + @Override + public String getRpcMethod(Map request) { + String service = getService(request); + String method = getMethod(request); + if (service == null || method == null) { + return null; + } + return service + "/" + method; + } + + @Nullable + @Override + public String getErrorType( + Map request, @Nullable Void response, @Nullable Throwable error) { + return request.get("errorType"); + } } @Test void server() { - testExtractor(RpcServerAttributesExtractor.create(TestGetter.INSTANCE)); + testExtractor(RpcServerAttributesExtractor.create(new TestGetter())); } @Test void client() { - testExtractor(RpcClientAttributesExtractor.create(TestGetter.INSTANCE)); + testExtractor(RpcClientAttributesExtractor.create(new TestGetter())); } + // Stable semconv keys + private static final AttributeKey RPC_SYSTEM_NAME = + AttributeKey.stringKey("rpc.system.name"); + private static void testExtractor(AttributesExtractor, Void> extractor) { Map request = new HashMap<>(); request.put("service", "my.Service"); @@ -61,16 +94,89 @@ private static void testExtractor(AttributesExtractor, Void> AttributesBuilder attributes = Attributes.builder(); extractor.onStart(attributes, context, request); - assertThat(attributes.build()) - .containsOnly( - entry(RPC_SYSTEM, "test"), - entry(RPC_SERVICE, "my.Service"), - entry(RPC_METHOD, "Method")); + + // Build expected entries list based on semconv mode + List, ?>> expectedEntries = new ArrayList<>(); + + if (emitStableRpcSemconv()) { + expectedEntries.add(entry(RPC_SYSTEM_NAME, "test")); + expectedEntries.add(entry(RPC_METHOD, "my.Service/Method")); + } + + if (emitOldRpcSemconv()) { + expectedEntries.add(entry(RPC_SYSTEM, "test")); + expectedEntries.add(entry(RPC_SERVICE, "my.Service")); + if (!emitStableRpcSemconv()) { + expectedEntries.add(entry(RPC_METHOD, "Method")); + } + } + + // safe conversion for test assertions + @SuppressWarnings({"unchecked", "rawtypes"}) + Map.Entry, ?>[] expectedArray = + (Map.Entry, ?>[]) expectedEntries.toArray(new Map.Entry[0]); + assertThat(attributes.build()).containsOnly(expectedArray); + + extractor.onEnd(attributes, context, request, null, null); + assertThat(attributes.build()).containsOnly(expectedArray); + } + + @Test + void shouldExtractErrorType_getter() { + Map request = new HashMap<>(); + request.put("service", "my.Service"); + request.put("method", "Method"); + request.put("errorType", "CANCELLED"); + + AttributesExtractor, Void> extractor = + RpcServerAttributesExtractor.create(new TestGetter()); + + Context context = Context.root(); + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, context, request); extractor.onEnd(attributes, context, request, null, null); - assertThat(attributes.build()) - .containsOnly( - entry(RPC_SYSTEM, "test"), - entry(RPC_SERVICE, "my.Service"), - entry(RPC_METHOD, "Method")); + + if (emitStableRpcSemconv()) { + assertThat(attributes.build()).containsEntry(ERROR_TYPE, "CANCELLED"); + } + } + + @Test + void shouldExtractErrorType_exceptionClassName() { + Map request = new HashMap<>(); + request.put("service", "my.Service"); + request.put("method", "Method"); + + AttributesExtractor, Void> extractor = + RpcServerAttributesExtractor.create(new TestGetter()); + + Context context = Context.root(); + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, context, request); + extractor.onEnd(attributes, context, request, null, new IllegalArgumentException()); + + if (emitStableRpcSemconv()) { + assertThat(attributes.build()) + .containsEntry(ERROR_TYPE, "java.lang.IllegalArgumentException"); + } + } + + @Test + void shouldNotExtractErrorType_noError() { + Map request = new HashMap<>(); + request.put("service", "my.Service"); + request.put("method", "Method"); + + AttributesExtractor, Void> extractor = + RpcServerAttributesExtractor.create(new TestGetter()); + + Context context = Context.root(); + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, context, request); + extractor.onEnd(attributes, context, request, null, null); + + if (emitStableRpcSemconv()) { + assertThat(attributes.build()).doesNotContainKey(ERROR_TYPE); + } } } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractorTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractorTest.java index 27e34a448341..070614c25a6f 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractorTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractorTest.java @@ -5,7 +5,9 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.rpc; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableRpcSemconv; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.mockito.Mockito.when; import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; @@ -17,23 +19,29 @@ @ExtendWith(MockitoExtension.class) class RpcSpanNameExtractorTest { - @Mock RpcAttributesGetter getter; + @Mock RpcAttributesGetter getter; - @SuppressWarnings("deprecation") // testing deprecated method @Test + @SuppressWarnings("deprecation") // testing deprecated method void normal() { RpcRequest request = new RpcRequest(); - when(getter.getService(request)).thenReturn("my.Service"); - when(getter.getMethod(request)).thenReturn("Method"); + if (emitStableRpcSemconv()) { + when(getter.getRpcMethod(request)).thenReturn("my.Service/Method"); + } else { + when(getter.getService(request)).thenReturn("my.Service"); + when(getter.getMethod(request)).thenReturn("Method"); + } SpanNameExtractor extractor = RpcSpanNameExtractor.create(getter); assertThat(extractor.extract(request)).isEqualTo("my.Service/Method"); } - @SuppressWarnings("deprecation") // testing deprecated method @Test + @SuppressWarnings("deprecation") // testing deprecated method void serviceNull() { + assumeTrue(!emitStableRpcSemconv()); + RpcRequest request = new RpcRequest(); when(getter.getMethod(request)).thenReturn("Method"); @@ -44,6 +52,8 @@ void serviceNull() { @Test void methodNull() { + assumeTrue(!emitStableRpcSemconv()); + RpcRequest request = new RpcRequest(); when(getter.getService(request)).thenReturn("my.Service"); @@ -52,5 +62,31 @@ void methodNull() { assertThat(extractor.extract(request)).isEqualTo("RPC request"); } + @Test + void rpcMethodNull_fallsBackToSystemName() { + assumeTrue(emitStableRpcSemconv()); + + RpcRequest request = new RpcRequest(); + + when(getter.getRpcSystemName(request)).thenReturn("grpc"); + + SpanNameExtractor extractor = RpcSpanNameExtractor.create(getter); + assertThat(extractor.extract(request)).isEqualTo("grpc"); + } + + @Test + @SuppressWarnings("deprecation") // testing deprecated method fallback + void rpcMethodNull_fallsBackToSystemName_viaGetSystem() { + assumeTrue(emitStableRpcSemconv()); + + RpcRequest request = new RpcRequest(); + + when(getter.getRpcSystemName(request)).thenCallRealMethod(); + when(getter.getSystem(request)).thenReturn("grpc"); + + SpanNameExtractor extractor = RpcSpanNameExtractor.create(getter); + assertThat(extractor.extract(request)).isEqualTo("grpc"); + } + static class RpcRequest {} }