[WIP] Add peer.service to Instrumenter API (open-telemetry#3050)

* [WIP] Add peer.service to Instrumenter API * Move PeerServiceAttributesExtractor to javaagent-api and use reflection to add it * Finish PeerServiceAttributesExtractor * Fix tests * Add peer.service to apache-httpclient-5.0, jedis-1.4, lettuce-4.0
robododge · Jun 17, 2021 · 6fa0bdc · 6fa0bdc
1 parent 452ec34
commit 6fa0bdc
Show file tree

Hide file tree

Showing 21 changed files with 366 additions and 54 deletions.
diff --git a/...lemetry/instrumentation/api/instrumenter/net/InetSocketAddressNetAttributesExtractor.java b/...lemetry/instrumentation/api/instrumenter/net/InetSocketAddressNetAttributesExtractor.java
@@ -25,11 +25,11 @@ public abstract class InetSocketAddressNetAttributesExtractor<REQUEST, RESPONSE>
    * phases of processing.
    */
   @Nullable
-  protected abstract InetSocketAddress getAddress(REQUEST request, @Nullable RESPONSE response);
+  public abstract InetSocketAddress getAddress(REQUEST request, @Nullable RESPONSE response);
 
   @Override
   @Nullable
-  protected final String peerName(REQUEST request, @Nullable RESPONSE response) {
+  public final String peerName(REQUEST request, @Nullable RESPONSE response) {
     InetSocketAddress address = getAddress(request, response);
     if (address == null) {
       return null;
@@ -42,7 +42,7 @@ protected final String peerName(REQUEST request, @Nullable RESPONSE response) {
 
   @Override
   @Nullable
-  protected final Integer peerPort(REQUEST request, @Nullable RESPONSE response) {
+  public final Integer peerPort(REQUEST request, @Nullable RESPONSE response) {
     InetSocketAddress address = getAddress(request, response);
     if (address == null) {
       return null;
@@ -52,7 +52,7 @@ protected final Integer peerPort(REQUEST request, @Nullable RESPONSE response) {
 
   @Override
   @Nullable
-  protected final String peerIp(REQUEST request, @Nullable RESPONSE response) {
+  public final String peerIp(REQUEST request, @Nullable RESPONSE response) {
     InetSocketAddress address = getAddress(request, response);
     if (address == null) {
       return null;

diff --git a/...in/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetAttributesExtractor.java b/...in/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetAttributesExtractor.java
@@ -42,29 +42,29 @@ protected final void onEnd(
   }
 
   @Nullable
-  protected abstract String transport(REQUEST request);
+  public abstract String transport(REQUEST request);
 
   /**
    * This method will be called twice: both when the request starts ({@code response} is always null
    * then) and when the response ends. This way it is possible to capture net attributes in both
    * phases of processing.
    */
   @Nullable
-  protected abstract String peerName(REQUEST request, @Nullable RESPONSE response);
+  public abstract String peerName(REQUEST request, @Nullable RESPONSE response);
 
   /**
    * This method will be called twice: both when the request starts ({@code response} is always null
    * then) and when the response ends. This way it is possible to capture net attributes in both
    * phases of processing.
    */
   @Nullable
-  protected abstract Integer peerPort(REQUEST request, @Nullable RESPONSE response);
+  public abstract Integer peerPort(REQUEST request, @Nullable RESPONSE response);
 
   /**
    * This method will be called twice: both when the request starts ({@code response} is always null
    * then) and when the response ends. This way it is possible to capture net attributes in both
    * phases of processing.
    */
   @Nullable
-  protected abstract String peerIp(REQUEST request, @Nullable RESPONSE response);
+  public abstract String peerIp(REQUEST request, @Nullable RESPONSE response);
 }
diff --git a/...try/instrumentation/api/instrumenter/net/InetSocketAddressNetAttributesExtractorTest.java b/...try/instrumentation/api/instrumenter/net/InetSocketAddressNetAttributesExtractorTest.java
@@ -23,13 +23,13 @@ class InetSocketAddressNetAttributesExtractorTest {
       extractor =
           new InetSocketAddressNetAttributesExtractor<InetSocketAddress, InetSocketAddress>() {
             @Override
-            protected InetSocketAddress getAddress(
+            public InetSocketAddress getAddress(
                 InetSocketAddress request, InetSocketAddress response) {
               return response != null ? response : request;
             }
 
             @Override
-            protected String transport(InetSocketAddress inetSocketAddress) {
+            public String transport(InetSocketAddress inetSocketAddress) {
               return SemanticAttributes.NetTransportValues.IP_TCP;
             }
           };

diff --git a/...ava/io/opentelemetry/instrumentation/api/instrumenter/net/NetAttributesExtractorTest.java b/...ava/io/opentelemetry/instrumentation/api/instrumenter/net/NetAttributesExtractorTest.java
@@ -21,28 +21,28 @@ static class TestNetAttributesExtractor
       extends NetAttributesExtractor<Map<String, String>, Map<String, String>> {
 
     @Override
-    protected String transport(Map<String, String> request) {
+    public String transport(Map<String, String> request) {
       return request.get("transport");
     }
 
     @Override
-    protected String peerName(Map<String, String> request, Map<String, String> response) {
+    public String peerName(Map<String, String> request, Map<String, String> response) {
       if (response != null) {
         return response.get("peerName");
       }
       return request.get("peerName");
     }
 
     @Override
-    protected Integer peerPort(Map<String, String> request, Map<String, String> response) {
+    public Integer peerPort(Map<String, String> request, Map<String, String> response) {
       if (response != null) {
         return Integer.valueOf(response.get("peerPort"));
       }
       return Integer.valueOf(request.get("peerPort"));
     }
 
     @Override
-    protected String peerIp(Map<String, String> request, Map<String, String> response) {
+    public String peerIp(Map<String, String> request, Map<String, String> response) {
       if (response != null) {
         return response.get("peerIp");
       }

diff --git a/...emetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumenters.java b/...emetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientInstrumenters.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
+import io.opentelemetry.javaagent.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 import org.apache.commons.httpclient.HttpMethod;
 
 public final class ApacheHttpClientInstrumenters {
@@ -27,13 +28,16 @@ public final class ApacheHttpClientInstrumenters {
         HttpSpanNameExtractor.create(httpAttributesExtractor);
     SpanStatusExtractor<? super HttpMethod, ? super HttpMethod> spanStatusExtractor =
         HttpSpanStatusExtractor.create(httpAttributesExtractor);
+    ApacheHttpClientNetAttributesExtractor netAttributesExtractor =
+        new ApacheHttpClientNetAttributesExtractor();
 
     INSTRUMENTER =
         Instrumenter.<HttpMethod, HttpMethod>newBuilder(
                 GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanNameExtractor)
             .setSpanStatusExtractor(spanStatusExtractor)
             .addAttributesExtractor(httpAttributesExtractor)
-            .addAttributesExtractor(new ApacheHttpClientNetAttributesExtractor())
+            .addAttributesExtractor(netAttributesExtractor)
+            .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesExtractor))
             .newClientInstrumenter(new HttpHeaderSetter());
   }
 

diff --git a/...vaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientNetAttributesExtractor.java b/...vaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientNetAttributesExtractor.java
@@ -15,24 +15,24 @@ final class ApacheHttpClientNetAttributesExtractor
     extends NetAttributesExtractor<HttpMethod, HttpMethod> {
 
   @Override
-  protected String transport(HttpMethod request) {
+  public String transport(HttpMethod request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
   @Override
-  protected @Nullable String peerName(HttpMethod request, @Nullable HttpMethod response) {
+  public @Nullable String peerName(HttpMethod request, @Nullable HttpMethod response) {
     HostConfiguration hostConfiguration = request.getHostConfiguration();
     return hostConfiguration != null ? hostConfiguration.getHost() : null;
   }
 
   @Override
-  protected @Nullable Integer peerPort(HttpMethod request, @Nullable HttpMethod response) {
+  public @Nullable Integer peerPort(HttpMethod request, @Nullable HttpMethod response) {
     HostConfiguration hostConfiguration = request.getHostConfiguration();
     return hostConfiguration != null ? hostConfiguration.getPort() : null;
   }
 
   @Override
-  protected @Nullable String peerIp(HttpMethod request, @Nullable HttpMethod response) {
+  public @Nullable String peerIp(HttpMethod request, @Nullable HttpMethod response) {
     return null;
   }
 }
diff --git a/...emetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumenters.java b/...emetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumenters.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
+import io.opentelemetry.javaagent.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
 
@@ -28,13 +29,16 @@ public final class ApacheHttpClientInstrumenters {
         HttpSpanNameExtractor.create(httpAttributesExtractor);
     SpanStatusExtractor<? super HttpUriRequest, ? super HttpResponse> spanStatusExtractor =
         HttpSpanStatusExtractor.create(httpAttributesExtractor);
+    ApacheHttpClientNetAttributesExtractor netAttributesExtractor =
+        new ApacheHttpClientNetAttributesExtractor();
 
     INSTRUMENTER =
         Instrumenter.<HttpUriRequest, HttpResponse>newBuilder(
                 GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanNameExtractor)
             .setSpanStatusExtractor(spanStatusExtractor)
             .addAttributesExtractor(httpAttributesExtractor)
-            .addAttributesExtractor(new ApacheHttpClientNetAttributesExtractor())
+            .addAttributesExtractor(netAttributesExtractor)
+            .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesExtractor))
             .newClientInstrumenter(new HttpHeaderSetter());
   }
 

diff --git a/...vaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientNetAttributesExtractor.java b/...vaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientNetAttributesExtractor.java
@@ -21,17 +21,17 @@ final class ApacheHttpClientNetAttributesExtractor
       LoggerFactory.getLogger(ApacheHttpClientNetAttributesExtractor.class);
 
   @Override
-  protected String transport(HttpUriRequest request) {
+  public String transport(HttpUriRequest request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
   @Override
-  protected @Nullable String peerName(HttpUriRequest request, @Nullable HttpResponse response) {
+  public @Nullable String peerName(HttpUriRequest request, @Nullable HttpResponse response) {
     return request.getURI().getHost();
   }
 
   @Override
-  protected Integer peerPort(HttpUriRequest request, @Nullable HttpResponse response) {
+  public Integer peerPort(HttpUriRequest request, @Nullable HttpResponse response) {
     URI uri = request.getURI();
     int port = uri.getPort();
     if (port != -1) {
@@ -49,7 +49,7 @@ protected Integer peerPort(HttpUriRequest request, @Nullable HttpResponse respon
   }
 
   @Override
-  protected @Nullable String peerIp(HttpUriRequest request, @Nullable HttpResponse response) {
+  public @Nullable String peerIp(HttpUriRequest request, @Nullable HttpResponse response) {
     return null;
   }
 }
diff --git a/...emetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumenters.java b/...emetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumenters.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
+import io.opentelemetry.javaagent.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
 
@@ -28,13 +29,16 @@ public final class ApacheHttpClientInstrumenters {
         HttpSpanNameExtractor.create(httpAttributesExtractor);
     SpanStatusExtractor<? super ClassicHttpRequest, ? super HttpResponse> spanStatusExtractor =
         HttpSpanStatusExtractor.create(httpAttributesExtractor);
+    ApacheHttpClientNetAttributesExtractor netAttributesExtractor =
+        new ApacheHttpClientNetAttributesExtractor();
 
     INSTRUMENTER =
         Instrumenter.<ClassicHttpRequest, HttpResponse>newBuilder(
                 GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanNameExtractor)
             .setSpanStatusExtractor(spanStatusExtractor)
             .addAttributesExtractor(httpAttributesExtractor)
-            .addAttributesExtractor(new ApacheHttpClientNetAttributesExtractor())
+            .addAttributesExtractor(netAttributesExtractor)
+            .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesExtractor))
             .newClientInstrumenter(new HttpHeaderSetter());
   }
 

diff --git a/...vaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesExtractor.java b/...vaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesExtractor.java
@@ -20,17 +20,17 @@ final class ApacheHttpClientNetAttributesExtractor
       LoggerFactory.getLogger(ApacheHttpClientNetAttributesExtractor.class);
 
   @Override
-  protected String transport(ClassicHttpRequest request) {
+  public String transport(ClassicHttpRequest request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
   @Override
-  protected @Nullable String peerName(ClassicHttpRequest request, @Nullable HttpResponse response) {
+  public @Nullable String peerName(ClassicHttpRequest request, @Nullable HttpResponse response) {
     return request.getAuthority().getHostName();
   }
 
   @Override
-  protected Integer peerPort(ClassicHttpRequest request, @Nullable HttpResponse response) {
+  public Integer peerPort(ClassicHttpRequest request, @Nullable HttpResponse response) {
     int port = request.getAuthority().getPort();
     if (port != -1) {
       return port;
@@ -51,7 +51,7 @@ protected Integer peerPort(ClassicHttpRequest request, @Nullable HttpResponse re
   }
 
   @Override
-  protected @Nullable String peerIp(ClassicHttpRequest request, @Nullable HttpResponse response) {
+  public @Nullable String peerIp(ClassicHttpRequest request, @Nullable HttpResponse response) {
     return null;
   }
 }
diff --git a/.../main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaDecorators.java b/.../main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaDecorators.java
@@ -6,20 +6,35 @@
 package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
 import com.linecorp.armeria.client.HttpClient;
+import com.linecorp.armeria.common.RequestContext;
+import com.linecorp.armeria.common.logging.RequestLog;
 import com.linecorp.armeria.server.HttpService;
 import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.armeria.v1_3.ArmeriaTracing;
+import io.opentelemetry.instrumentation.armeria.v1_3.ArmeriaTracingBuilder;
+import io.opentelemetry.javaagent.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 import java.util.function.Function;
 
 // Holds singleton references to decorators to match against during suppression.
 // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/903
 public final class ArmeriaDecorators {
+  public static final Function<? super HttpClient, ? extends HttpClient> CLIENT_DECORATOR;
 
-  private static final ArmeriaTracing TRACING = ArmeriaTracing.create(GlobalOpenTelemetry.get());
+  public static final Function<? super HttpService, ? extends HttpService> SERVER_DECORATOR;
 
-  public static final Function<? super HttpClient, ? extends HttpClient> CLIENT_DECORATOR =
-      TRACING.newClientDecorator();
+  static {
+    ArmeriaTracingBuilder builder = ArmeriaTracing.newBuilder(GlobalOpenTelemetry.get());
 
-  public static final Function<? super HttpService, ? extends HttpService> SERVER_DECORATOR =
-      TRACING.newServiceDecorator();
+    AttributesExtractor<RequestContext, RequestLog> peerServiceAttributesExtractor =
+        PeerServiceAttributesExtractor.createUsingReflection(
+            "io.opentelemetry.instrumentation.armeria.v1_3.ArmeriaNetAttributesExtractor");
+    if (peerServiceAttributesExtractor != null) {
+      builder.addAttributeExtractor(peerServiceAttributesExtractor);
+    }
+
+    ArmeriaTracing tracing = builder.build();
+    CLIENT_DECORATOR = tracing.newClientDecorator();
+    SERVER_DECORATOR = tracing.newServiceDecorator();
+  }
 }
diff --git a/...ain/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaNetAttributesExtractor.java b/...ain/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaNetAttributesExtractor.java
@@ -17,13 +17,13 @@ final class ArmeriaNetAttributesExtractor
     extends InetSocketAddressNetAttributesExtractor<RequestContext, RequestLog> {
 
   @Override
-  protected String transport(RequestContext requestContext) {
+  public String transport(RequestContext requestContext) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
   @Override
   @Nullable
-  protected InetSocketAddress getAddress(
+  public InetSocketAddress getAddress(
       RequestContext requestContext, @Nullable RequestLog requestLog) {
     SocketAddress address = requestContext.remoteAddress();
     if (address instanceof InetSocketAddress) {

diff --git a/...gent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcInstrumenters.java b/...gent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcInstrumenters.java
@@ -11,21 +11,24 @@
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.db.DbAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.db.DbSpanNameExtractor;
+import io.opentelemetry.javaagent.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 
 public final class JdbcInstrumenters {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.javaagent.jdbc";
 
   private static final Instrumenter<DbRequest, Void> INSTRUMENTER;
 
   static {
-    DbAttributesExtractor<DbRequest> attributesExtractor = new JdbcAttributesExtractor();
-    SpanNameExtractor<DbRequest> spanName = DbSpanNameExtractor.create(attributesExtractor);
+    DbAttributesExtractor<DbRequest> dbAttributesExtractor = new JdbcAttributesExtractor();
+    SpanNameExtractor<DbRequest> spanName = DbSpanNameExtractor.create(dbAttributesExtractor);
+    JdbcNetAttributesExtractor netAttributesExtractor = new JdbcNetAttributesExtractor();
 
     INSTRUMENTER =
         Instrumenter.<DbRequest, Void>newBuilder(
                 GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanName)
-            .addAttributesExtractor(attributesExtractor)
-            .addAttributesExtractor(new JdbcNetAttributesExtractor())
+            .addAttributesExtractor(dbAttributesExtractor)
+            .addAttributesExtractor(netAttributesExtractor)
+            .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesExtractor))
             .newInstrumenter(SpanKindExtractor.alwaysClient());
   }
 

diff --git a/...main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcNetAttributesExtractor.java b/...main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcNetAttributesExtractor.java
@@ -12,25 +12,25 @@ final class JdbcNetAttributesExtractor extends NetAttributesExtractor<DbRequest,
 
   @Nullable
   @Override
-  protected String transport(DbRequest dbRequest) {
+  public String transport(DbRequest dbRequest) {
     return null;
   }
 
   @Nullable
   @Override
-  protected String peerName(DbRequest request, @Nullable Void response) {
+  public String peerName(DbRequest request, @Nullable Void response) {
     return request.getDbInfo().getHost();
   }
 
   @Nullable
   @Override
-  protected Integer peerPort(DbRequest request, @Nullable Void response) {
+  public Integer peerPort(DbRequest request, @Nullable Void response) {
     return request.getDbInfo().getPort();
   }
 
   @Nullable
   @Override
-  protected String peerIp(DbRequest request, @Nullable Void response) {
+  public String peerIp(DbRequest request, @Nullable Void response) {
     return null;
   }
 }