Add dual-semconv support to RPC attributes extractors

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesGetter.java

@@ -16,6 +16,23 @@
  */
 public interface RpcAttributesGetter<REQUEST, RESPONSE> {
 
+  /**
+   * Returns the stable semconv system name for the RPC framework (e.g. {@code "grpc"}, {@code
+   * "java_rmi"}, {@code "dotnet_wcf"}).
+   *
+   * @see <a
+   *     href="https://opentelemetry.io/docs/specs/semconv/attributes-registry/rpc/">rpc.system.name
+   *     spec</a>
+   */
+  @Nullable
+  default String getRpcSystemName(REQUEST request) {
+    return null;
+  }
+
+  /**
+   * @deprecated Use {@link #getRpcSystemName(REQUEST)}. To be removed in 3.0.
+   */
+  @Deprecated
   @Nullable
   String getSystem(REQUEST request);
 

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java

@@ -5,18 +5,27 @@
 
 package io.opentelemetry.instrumentation.api.incubator.semconv.rpc;
 
+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;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.instrumentation.api.internal.SemconvStability;
 import javax.annotation.Nullable;
 
 abstract class RpcCommonAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
-  // copied from RpcIncubatingAttributes
   static final AttributeKey<String> RPC_METHOD = AttributeKey.stringKey("rpc.method");
+
+  // Stable semconv keys
+  static final AttributeKey<String> RPC_SYSTEM_NAME = AttributeKey.stringKey("rpc.system.name");
+
+  // removed in stable semconv (merged into rpc.method)
   static final AttributeKey<String> RPC_SERVICE = AttributeKey.stringKey("rpc.service");
+
+  // use RPC_SYSTEM_NAME for stable semconv
   static final AttributeKey<String> RPC_SYSTEM = AttributeKey.stringKey("rpc.system");
 
   private final RpcAttributesGetter<REQUEST, RESPONSE> getter;
@@ -25,12 +34,23 @@ abstract class RpcCommonAttributesExtractor<REQUEST, RESPONSE>
     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 (SemconvStability.emitStableRpcSemconv()) {
+      attributes.put(RPC_SYSTEM_NAME, getter.getRpcSystemName(request));
+      attributes.put(RPC_METHOD, getter.getRpcMethod(request));
+    }
+
+    if (SemconvStability.emitOldRpcSemconv()) {
+      attributes.put(RPC_SYSTEM, getter.getSystem(request));
+      attributes.put(RPC_SERVICE, getter.getService(request));
+      if (!SemconvStability.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 +60,13 @@ public final void onEnd(
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {
-    // No response attributes
+    if (SemconvStability.emitStableRpcSemconv()) {
+      String errorType = getter.getErrorType(request, response, error);
+      // fall back to exception class name & _OTHER
+      if (errorType == null && error != null) {
+        errorType = error.getClass().getName();
+      }
+      attributes.put(ERROR_TYPE, errorType);
+    }
   }
 }

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractor.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.api.incubator.semconv.rpc;
 
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
+import io.opentelemetry.instrumentation.api.internal.SemconvStability;
 
 /** A {@link SpanNameExtractor} for RPC requests. */
 public final class RpcSpanNameExtractor<REQUEST> implements SpanNameExtractor<REQUEST> {
@@ -28,6 +29,14 @@ private RpcSpanNameExtractor(RpcAttributesGetter<REQUEST, ?> getter) {
   @SuppressWarnings("deprecation") // for getMethod()
   @Override
   public String extract(REQUEST request) {
+    if (SemconvStability.emitStableRpcSemconv()) {
+      String method = getter.getRpcMethod(request);
+      if (method != null) {
+        return method;
+      }
+      return "RPC request";
+    }
+
     String service = getter.getService(request);
     String method = getter.getMethod(request);
     if (service == null || method == null) {

instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcAttributesExtractorTest.java

@@ -6,24 +6,32 @@
 package io.opentelemetry.instrumentation.api.incubator.semconv.rpc;
 
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
-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 io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE;
 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 io.opentelemetry.instrumentation.api.internal.SemconvStability;
+import io.opentelemetry.semconv.incubating.RpcIncubatingAttributes;
+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<Map<String, String>, Void> {
-    INSTANCE;
+  private static class TestGetter implements RpcAttributesGetter<Map<String, String>, Void> {
+
+    @Override
+    public String getRpcSystemName(Map<String, String> request) {
+      return "test";
+    }
 
     @Override
     public String getSystem(Map<String, String> request) {
@@ -40,18 +48,47 @@ public String getService(Map<String, String> request) {
     public String getMethod(Map<String, String> request) {
       return request.get("method");
     }
+
+    @Nullable
+    @Override
+    public String getRpcMethod(Map<String, String> 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<String, String> 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<String> RPC_SYSTEM_NAME =
+      AttributeKey.stringKey("rpc.system.name");
+
+  // Old semconv keys (from RpcIncubatingAttributes)
+  private static final AttributeKey<String> RPC_SYSTEM = RpcIncubatingAttributes.RPC_SYSTEM;
+
+  private static final AttributeKey<String> RPC_SERVICE = RpcIncubatingAttributes.RPC_SERVICE;
+
+  private static final AttributeKey<String> RPC_METHOD = RpcIncubatingAttributes.RPC_METHOD;
+
   private static void testExtractor(AttributesExtractor<Map<String, String>, Void> extractor) {
     Map<String, String> request = new HashMap<>();
     request.put("service", "my.Service");
@@ -61,16 +98,89 @@ private static void testExtractor(AttributesExtractor<Map<String, String>, 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<Map.Entry<? extends AttributeKey<?>, ?>> expectedEntries = new ArrayList<>();
+
+    if (SemconvStability.emitStableRpcSemconv()) {
+      expectedEntries.add(entry(RPC_SYSTEM_NAME, "test"));
+      expectedEntries.add(entry(RPC_METHOD, "my.Service/Method"));
+    }
+
+    if (SemconvStability.emitOldRpcSemconv()) {
+      expectedEntries.add(entry(RPC_SYSTEM, "test"));
+      expectedEntries.add(entry(RPC_SERVICE, "my.Service"));
+      if (!SemconvStability.emitStableRpcSemconv()) {
+        expectedEntries.add(entry(RPC_METHOD, "Method"));
+      }
+    }
+
+    // safe conversion for test assertions
+    @SuppressWarnings({"unchecked", "rawtypes"})
+    Map.Entry<? extends AttributeKey<?>, ?>[] expectedArray =
+        (Map.Entry<? extends AttributeKey<?>, ?>[]) 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<String, String> request = new HashMap<>();
+    request.put("service", "my.Service");
+    request.put("method", "Method");
+    request.put("errorType", "CANCELLED");
+
+    AttributesExtractor<Map<String, String>, 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 (SemconvStability.emitStableRpcSemconv()) {
+      assertThat(attributes.build()).containsEntry(ERROR_TYPE, "CANCELLED");
+    }
+  }
+
+  @Test
+  void shouldExtractErrorType_exceptionClassName() {
+    Map<String, String> request = new HashMap<>();
+    request.put("service", "my.Service");
+    request.put("method", "Method");
+
+    AttributesExtractor<Map<String, String>, 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 (SemconvStability.emitStableRpcSemconv()) {
+      assertThat(attributes.build())
+          .containsEntry(ERROR_TYPE, "java.lang.IllegalArgumentException");
+    }
+  }
+
+  @Test
+  void shouldNotExtractErrorType_noError() {
+    Map<String, String> request = new HashMap<>();
+    request.put("service", "my.Service");
+    request.put("method", "Method");
+
+    AttributesExtractor<Map<String, String>, 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 (SemconvStability.emitStableRpcSemconv()) {
+      assertThat(attributes.build()).doesNotContainKey(ERROR_TYPE);
+    }
   }
 }

instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcSpanNameExtractorTest.java

@@ -6,9 +6,12 @@
 package io.opentelemetry.instrumentation.api.incubator.semconv.rpc;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.when;
 
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
+import io.opentelemetry.instrumentation.api.internal.SemconvStability;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
@@ -17,23 +20,26 @@
 @ExtendWith(MockitoExtension.class)
 class RpcSpanNameExtractorTest {
 
-  @Mock RpcAttributesGetter<RpcRequest, Void> getter;
+  @Mock RpcAttributesGetter<RpcRequest, Object> 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");
+    lenient().when(getter.getRpcMethod(request)).thenReturn("my.Service/Method");
+    lenient().when(getter.getService(request)).thenReturn("my.Service");
+    lenient().when(getter.getMethod(request)).thenReturn("Method");
 
     SpanNameExtractor<RpcRequest> 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(!SemconvStability.emitStableRpcSemconv());
+
     RpcRequest request = new RpcRequest();
 
     when(getter.getMethod(request)).thenReturn("Method");
@@ -44,6 +50,8 @@ void serviceNull() {
 
   @Test
   void methodNull() {
+    assumeTrue(!SemconvStability.emitStableRpcSemconv());
+
     RpcRequest request = new RpcRequest();
 
     when(getter.getService(request)).thenReturn("my.Service");
@@ -52,5 +60,15 @@ void methodNull() {
     assertThat(extractor.extract(request)).isEqualTo("RPC request");
   }
 
+  @Test
+  void rpcMethodNull() {
+    assumeTrue(SemconvStability.emitStableRpcSemconv());
+
+    RpcRequest request = new RpcRequest();
+
+    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(getter);
+    assertThat(extractor.extract(request)).isEqualTo("RPC request");
+  }
+
   static class RpcRequest {}
 }