Split out RpcAttributesGetter (#5548)

* Split out RpcAttributesGetter

* code review comments

* go back to RpcAttributesGetter

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java

@@ -19,7 +19,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.db.DbClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessagingAttributesExtractor;
-import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.internal.SpanKey;
 import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
 import java.util.ArrayList;
@@ -171,7 +171,7 @@ public InstrumenterBuilder<REQUEST, RESPONSE> setDisabled(boolean disabled) {
    *
    * <ul>
    *   <li>CLIENT nested spans are suppressed depending on their type: {@linkplain
-   *       HttpClientAttributesExtractor HTTP}, {@linkplain RpcAttributesExtractor RPC} or
+   *       HttpClientAttributesExtractor HTTP}, {@linkplain RpcClientAttributesExtractor RPC} or
    *       {@linkplain DbClientAttributesExtractor database} clients. If a span with the same type
    *       is present in the parent context object, new span of the same type will not be started.
    * </ul>

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

@@ -0,0 +1,27 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.rpc;
+
+import javax.annotation.Nullable;
+
+/**
+ * An interface for getting RPC attributes.
+ *
+ * <p>Instrumentation authors will create implementations of this interface for their specific
+ * library/framework. It will be used by the {@link RpcClientAttributesExtractor} or {@link
+ * RpcServerAttributesExtractor} to obtain the various RPC attributes in a type-generic way.
+ */
+public interface RpcAttributesGetter<REQUEST> {
+
+  @Nullable
+  String system(REQUEST request);
+
+  @Nullable
+  String service(REQUEST request);
+
+  @Nullable
+  String method(REQUEST request);
+}

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/RpcClientAttributesExtractor.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.rpc;
+
+import io.opentelemetry.instrumentation.api.annotations.UnstableApi;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+
+/**
+ * Extractor of <a
+ * href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md">RPC
+ * client attributes</a>.
+ *
+ * <p>This class delegates to a type-specific {@link RpcAttributesGetter} for individual attribute
+ * extraction from request/response objects.
+ */
+public final class RpcClientAttributesExtractor<REQUEST, RESPONSE>
+    extends RpcCommonAttributesExtractor<REQUEST, RESPONSE> implements SpanKeyProvider {
+
+  /** Creates the RPC client attributes extractor. */
+  public static <REQUEST, RESPONSE> RpcClientAttributesExtractor<REQUEST, RESPONSE> create(
+      RpcAttributesGetter<REQUEST> getter) {
+    return new RpcClientAttributesExtractor<>(getter);
+  }
+
+  private RpcClientAttributesExtractor(RpcAttributesGetter<REQUEST> getter) {
+    super(getter);
+  }
+
+  /**
+   * This method is internal and is hence not for public use. Its API is unstable and can change at
+   * any time.
+   */
+  @UnstableApi
+  @Override
+  public SpanKey internalGetSpanKey() {
+    return SpanKey.RPC_CLIENT;
+  }
+}

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

@@ -0,0 +1,39 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.rpc;
+
+import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import javax.annotation.Nullable;
+
+abstract class RpcCommonAttributesExtractor<REQUEST, RESPONSE>
+    implements AttributesExtractor<REQUEST, RESPONSE> {
+
+  private final RpcAttributesGetter<REQUEST> getter;
+
+  RpcCommonAttributesExtractor(RpcAttributesGetter<REQUEST> getter) {
+    this.getter = getter;
+  }
+
+  @Override
+  public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
+    set(attributes, SemanticAttributes.RPC_SYSTEM, getter.system(request));
+    set(attributes, SemanticAttributes.RPC_SERVICE, getter.service(request));
+    set(attributes, SemanticAttributes.RPC_METHOD, getter.method(request));
+  }
+
+  @Override
+  public final void onEnd(
+      AttributesBuilder attributes,
+      Context context,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
+    // No response attributes
+  }
+}

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/RpcServerAttributesExtractor.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.rpc;
+
+/**
+ * Extractor of <a
+ * href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md">RPC
+ * server attributes</a>.
+ *
+ * <p>This class delegates to a type-specific {@link RpcAttributesGetter} for individual attribute
+ * extraction from request/response objects.
+ */
+public final class RpcServerAttributesExtractor<REQUEST, RESPONSE>
+    extends RpcCommonAttributesExtractor<REQUEST, RESPONSE> {
+
+  /** Creates the RPC server attributes extractor. */
+  public static <REQUEST, RESPONSE> RpcServerAttributesExtractor<REQUEST, RESPONSE> create(
+      RpcAttributesGetter<REQUEST> getter) {
+    return new RpcServerAttributesExtractor<>(getter);
+  }
+
+  private RpcServerAttributesExtractor(RpcAttributesGetter<REQUEST> getter) {
+    super(getter);
+  }
+}

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

@@ -15,20 +15,20 @@ public final class RpcSpanNameExtractor<REQUEST> implements SpanNameExtractor<RE
    * conventions: {@code <rpc.service>/<rpc.method>}.
    */
   public static <REQUEST> SpanNameExtractor<REQUEST> create(
-      RpcAttributesExtractor<REQUEST, ?> attributesExtractor) {
+      RpcAttributesGetter<REQUEST> attributesExtractor) {
     return new RpcSpanNameExtractor<>(attributesExtractor);
   }
 
-  private final RpcAttributesExtractor<REQUEST, ?> attributesExtractor;
+  private final RpcAttributesGetter<REQUEST> getter;
 
-  private RpcSpanNameExtractor(RpcAttributesExtractor<REQUEST, ?> attributesExtractor) {
-    this.attributesExtractor = attributesExtractor;
+  private RpcSpanNameExtractor(RpcAttributesGetter<REQUEST> getter) {
+    this.getter = getter;
   }
 
   @Override
   public String extract(REQUEST request) {
-    String service = attributesExtractor.service(request);
-    String method = attributesExtractor.method(request);
+    String service = getter.service(request);
+    String method = getter.method(request);
     if (service == null || method == null) {
       return "RPC request";
     }

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java

@@ -31,7 +31,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessagingAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter;
-import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.internal.SpanKey;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
 import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
@@ -176,7 +176,7 @@ public String get(Map<String, String> carrier, String key) {
   @Mock
   MessagingAttributesExtractor<Map<String, String>, Map<String, String>> mockMessagingAttributes;
 
-  @Mock RpcAttributesExtractor<Map<String, String>, Map<String, String>> mockRpcAttributes;
+  @Mock RpcClientAttributesExtractor<Map<String, String>, Map<String, String>> mockRpcAttributes;
   @Mock NetServerAttributesExtractor<Map<String, String>, Map<String, String>> mockNetAttributes;
 
   @Test

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

@@ -11,40 +11,50 @@
 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.semconv.trace.attributes.SemanticAttributes;
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
 
 class RpcAttributesExtractorTest {
 
-  static class TestExtractor extends RpcAttributesExtractor<Map<String, String>, Void> {
+  enum TestGetter implements RpcAttributesGetter<Map<String, String>> {
+    INSTANCE;
 
     @Override
-    protected String system(Map<String, String> request) {
+    public String system(Map<String, String> request) {
       return "test";
     }
 
     @Override
-    protected String service(Map<String, String> request) {
+    public String service(Map<String, String> request) {
       return request.get("service");
     }
 
     @Override
-    protected String method(Map<String, String> request) {
+    public String method(Map<String, String> request) {
       return request.get("method");
     }
   }
 
   @Test
-  void normal() {
+  void server() {
+    testExtractor(RpcServerAttributesExtractor.create(TestGetter.INSTANCE));
+  }
+
+  @Test
+  void client() {
+    testExtractor(RpcClientAttributesExtractor.create(TestGetter.INSTANCE));
+  }
+
+  private static void testExtractor(AttributesExtractor<Map<String, String>, Void> extractor) {
     Map<String, String> request = new HashMap<>();
     request.put("service", "my.Service");
     request.put("method", "Method");
 
     Context context = Context.root();
 
-    TestExtractor extractor = new TestExtractor();
     AttributesBuilder attributes = Attributes.builder();
     extractor.onStart(attributes, context, request);
     assertThat(attributes.build())

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

@@ -17,36 +17,36 @@
 @ExtendWith(MockitoExtension.class)
 class RpcSpanNameExtractorTest {
 
-  @Mock RpcAttributesExtractor<RpcRequest, Void> attributesExtractor;
+  @Mock RpcAttributesGetter<RpcRequest> getter;
 
   @Test
   void normal() {
     RpcRequest request = new RpcRequest();
 
-    when(attributesExtractor.service(request)).thenReturn("my.Service");
-    when(attributesExtractor.method(request)).thenReturn("Method");
+    when(getter.service(request)).thenReturn("my.Service");
+    when(getter.method(request)).thenReturn("Method");
 
-    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(attributesExtractor);
+    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(getter);
     assertThat(extractor.extract(request)).isEqualTo("my.Service/Method");
   }
 
   @Test
   void serviceNull() {
     RpcRequest request = new RpcRequest();
 
-    when(attributesExtractor.method(request)).thenReturn("Method");
+    when(getter.method(request)).thenReturn("Method");
 
-    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(attributesExtractor);
+    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(getter);
     assertThat(extractor.extract(request)).isEqualTo("RPC request");
   }
 
   @Test
   void methodNull() {
     RpcRequest request = new RpcRequest();
 
-    when(attributesExtractor.service(request)).thenReturn("my.Service");
+    when(getter.service(request)).thenReturn("my.Service");
 
-    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(attributesExtractor);
+    SpanNameExtractor<RpcRequest> extractor = RpcSpanNameExtractor.create(getter);
     assertThat(extractor.extract(request)).isEqualTo("RPC request");
   }
 

instrumentation/apache-dubbo-2.7/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/DubboRpcAttributesExtractor.java → instrumentation/apache-dubbo-2.7/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/DubboRpcAttributesGetter.java

@@ -5,22 +5,23 @@
 
 package io.opentelemetry.instrumentation.apachedubbo.v2_7;
 
-import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
-import org.apache.dubbo.rpc.Result;
+import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesGetter;
+
+enum DubboRpcAttributesGetter implements RpcAttributesGetter<DubboRequest> {
+  INSTANCE;
 
-final class DubboRpcAttributesExtractor extends RpcAttributesExtractor<DubboRequest, Result> {
   @Override
-  protected String system(DubboRequest request) {
+  public String system(DubboRequest request) {
     return "dubbo";
   }
 
   @Override
-  protected String service(DubboRequest request) {
+  public String service(DubboRequest request) {
     return request.invocation().getInvoker().getInterface().getName();
   }
 
   @Override
-  protected String method(DubboRequest request) {
+  public String method(DubboRequest request) {
     return request.invocation().getMethodName();
   }
 }