Extract net attributes both on start and on end in netty HTTP client (#…

…4420) * Extract net attributes both on start and on end in netty HTTP client * storing remote address in HttpRequestAndChannel * Update instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyNetClientAttributesExtractor.java Co-authored-by: Trask Stalnaker <[email protected]> Co-authored-by: Trask Stalnaker <[email protected]>
open-telemetry · Oct 21, 2021 · c490b29 · c490b29
1 parent b1d638f
commit c490b29
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 11 deletions.
diff --git a/...n/java/io/opentelemetry/javaagent/instrumentation/netty/common/HttpRequestAndChannel.java b/...n/java/io/opentelemetry/javaagent/instrumentation/netty/common/HttpRequestAndChannel.java
@@ -8,15 +8,22 @@
 import com.google.auto.value.AutoValue;
 import io.netty.channel.Channel;
 import io.netty.handler.codec.http.HttpRequest;
+import java.net.SocketAddress;
+import javax.annotation.Nullable;
 
 @AutoValue
 public abstract class HttpRequestAndChannel {
 
   public static HttpRequestAndChannel create(HttpRequest request, Channel channel) {
-    return new AutoValue_HttpRequestAndChannel(request, channel);
+    return new AutoValue_HttpRequestAndChannel(request, channel, channel.remoteAddress());
   }
 
   public abstract HttpRequest request();
 
   public abstract Channel channel();
+
+  // we're capturing the remote address early because in case of timeouts or other connection issues
+  // netty may return null when calling Channel.remoteAddress() at the end of processing
+  @Nullable
+  public abstract SocketAddress remoteAddress();
 }
diff --git a/...lemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java b/...lemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java
@@ -8,11 +8,11 @@
 import io.netty.handler.codec.http.HttpResponse;
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientMetrics;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
 import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
-import io.opentelemetry.javaagent.instrumentation.netty.common.NettyCommonNetAttributesExtractor;
 
 public final class NettyClientInstrumenterFactory {
 
@@ -25,16 +25,17 @@ public NettyClientInstrumenterFactory(String instrumentationName) {
   public Instrumenter<HttpRequestAndChannel, HttpResponse> createHttpInstrumenter() {
     NettyHttpClientAttributesExtractor httpClientAttributesExtractor =
         new NettyHttpClientAttributesExtractor();
+    NettyNetClientAttributesExtractor netClientAttributesExtractor =
+        new NettyNetClientAttributesExtractor();
 
     return Instrumenter.<HttpRequestAndChannel, HttpResponse>newBuilder(
             GlobalOpenTelemetry.get(),
             instrumentationName,
             HttpSpanNameExtractor.create(httpClientAttributesExtractor))
         .setSpanStatusExtractor(HttpSpanStatusExtractor.create(httpClientAttributesExtractor))
         .addAttributesExtractor(httpClientAttributesExtractor)
-        .addAttributesExtractor(new NettyCommonNetAttributesExtractor())
-        // TODO: add peer extractor attributes once Net*AttributesExtractors are refactored
-        // .addAttributesExtractor(PeerServiceAttributesExtractor.create(netClientAttributesExtractor))
+        .addAttributesExtractor(netClientAttributesExtractor)
+        .addAttributesExtractor(PeerServiceAttributesExtractor.create(netClientAttributesExtractor))
         .addRequestMetrics(HttpClientMetrics.get())
         .newClientInstrumenter(new HttpRequestHeadersSetter());
   }

diff --git a/...etry/javaagent/instrumentation/netty/common/client/NettyNetClientAttributesExtractor.java b/...etry/javaagent/instrumentation/netty/common/client/NettyNetClientAttributesExtractor.java
@@ -19,7 +19,7 @@ final class NettyNetClientAttributesExtractor
   @Nullable
   public InetSocketAddress getAddress(
       HttpRequestAndChannel requestAndChannel, @Nullable HttpResponse response) {
-    SocketAddress address = requestAndChannel.channel().remoteAddress();
+    SocketAddress address = requestAndChannel.remoteAddress();
     if (address instanceof InetSocketAddress) {
       return (InetSocketAddress) address;
     }

diff --git a/...on/NettyCommonNetAttributesExtractor.java → ...er/NettyNetServerAttributesExtractor.java b/...on/NettyCommonNetAttributesExtractor.java → ...er/NettyNetServerAttributesExtractor.java
@@ -3,15 +3,16 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.netty.common;
+package io.opentelemetry.javaagent.instrumentation.netty.common.server;
 
 import io.netty.handler.codec.http.HttpResponse;
 import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
+import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import javax.annotation.Nullable;
 
-public final class NettyCommonNetAttributesExtractor
+final class NettyNetServerAttributesExtractor
     extends InetSocketAddressNetServerAttributesExtractor<HttpRequestAndChannel, HttpResponse> {
 
   @Override
@@ -23,7 +24,7 @@ public String transport(HttpRequestAndChannel requestAndChannel) {
   @Override
   @Nullable
   public InetSocketAddress getAddress(HttpRequestAndChannel requestAndChannel) {
-    SocketAddress address = requestAndChannel.channel().remoteAddress();
+    SocketAddress address = requestAndChannel.remoteAddress();
     if (address instanceof InetSocketAddress) {
       return (InetSocketAddress) address;
     }

diff --git a/...lemetry/javaagent/instrumentation/netty/common/server/NettyServerInstrumenterFactory.java b/...lemetry/javaagent/instrumentation/netty/common/server/NettyServerInstrumenterFactory.java
@@ -13,7 +13,6 @@
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
 import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
 import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
-import io.opentelemetry.javaagent.instrumentation.netty.common.NettyCommonNetAttributesExtractor;
 import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
 
 public final class NettyServerInstrumenterFactory {
@@ -30,7 +29,7 @@ public static Instrumenter<HttpRequestAndChannel, HttpResponse> create(
             HttpSpanNameExtractor.create(httpAttributesExtractor))
         .setSpanStatusExtractor(HttpSpanStatusExtractor.create(httpAttributesExtractor))
         .addAttributesExtractor(httpAttributesExtractor)
-        .addAttributesExtractor(new NettyCommonNetAttributesExtractor())
+        .addAttributesExtractor(new NettyNetServerAttributesExtractor())
         .addRequestMetrics(HttpServerMetrics.get())
         .addContextCustomizer(
             (context, request, attributes) -> {