Add network peer address attributes to the span for the okhttp-3.0 …

…instrumentation
open-telemetry · Aug 15, 2024 · 282f890 · 282f890
1 parent 5d8b4f7
commit 282f890
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 38 deletions.
diff --git a/...c/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3Singletons.java b/...c/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3Singletons.java
@@ -15,13 +15,12 @@
 import io.opentelemetry.instrumentation.okhttp.v3_0.internal.TracingInterceptor;
 import io.opentelemetry.javaagent.bootstrap.internal.JavaagentHttpClientInstrumenters;
 import okhttp3.Interceptor;
-import okhttp3.Request;
 import okhttp3.Response;
 
 /** Holder of singleton interceptors for adding to instrumented clients. */
 public final class OkHttp3Singletons {
 
-  private static final Instrumenter<Request, Response> INSTRUMENTER =
+  private static final Instrumenter<Interceptor.Chain, Response> INSTRUMENTER =
       JavaagentHttpClientInstrumenters.create(
           OkHttpClientInstrumenterBuilderFactory.create(GlobalOpenTelemetry.get()));
 

diff --git a/...0/library/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttpTelemetry.java b/...0/library/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttpTelemetry.java
@@ -14,7 +14,6 @@
 import okhttp3.Callback;
 import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
-import okhttp3.Request;
 import okhttp3.Response;
 
 /** Entrypoint for instrumenting OkHttp clients. */
@@ -32,10 +31,11 @@ public static OkHttpTelemetryBuilder builder(OpenTelemetry openTelemetry) {
     return new OkHttpTelemetryBuilder(openTelemetry);
   }
 
-  private final Instrumenter<Request, Response> instrumenter;
+  private final Instrumenter<Interceptor.Chain, Response> instrumenter;
   private final ContextPropagators propagators;
 
-  OkHttpTelemetry(Instrumenter<Request, Response> instrumenter, ContextPropagators propagators) {
+  OkHttpTelemetry(
+      Instrumenter<Interceptor.Chain, Response> instrumenter, ContextPropagators propagators) {
     this.instrumenter = instrumenter;
     this.propagators = propagators;
   }

diff --git a/...ry/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttpTelemetryBuilder.java b/...ry/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttpTelemetryBuilder.java
@@ -15,13 +15,13 @@
 import java.util.List;
 import java.util.Set;
 import java.util.function.Function;
-import okhttp3.Request;
+import okhttp3.Interceptor;
 import okhttp3.Response;
 
 /** A builder of {@link OkHttpTelemetry}. */
 public final class OkHttpTelemetryBuilder {
 
-  private final DefaultHttpClientInstrumenterBuilder<Request, Response> builder;
+  private final DefaultHttpClientInstrumenterBuilder<Interceptor.Chain, Response> builder;
 
   OkHttpTelemetryBuilder(OpenTelemetry openTelemetry) {
     builder = OkHttpClientInstrumenterBuilderFactory.create(openTelemetry);
@@ -33,7 +33,7 @@ public final class OkHttpTelemetryBuilder {
    */
   @CanIgnoreReturnValue
   public OkHttpTelemetryBuilder addAttributeExtractor(
-      AttributesExtractor<? super Request, ? super Response> attributesExtractor) {
+      AttributesExtractor<? super Interceptor.Chain, ? super Response> attributesExtractor) {
     builder.addAttributeExtractor(attributesExtractor);
     return this;
   }
@@ -95,7 +95,9 @@ public OkHttpTelemetryBuilder setEmitExperimentalHttpClientMetrics(
   /** Sets custom {@link SpanNameExtractor} via transform function. */
   @CanIgnoreReturnValue
   public OkHttpTelemetryBuilder setSpanNameExtractor(
-      Function<SpanNameExtractor<? super Request>, ? extends SpanNameExtractor<? super Request>>
+      Function<
+              SpanNameExtractor<? super Interceptor.Chain>,
+              ? extends SpanNameExtractor<? super Interceptor.Chain>>
           spanNameExtractorTransformer) {
     builder.setSpanNameExtractor(spanNameExtractorTransformer);
     return this;

diff --git a/...io/opentelemetry/instrumentation/okhttp/v3_0/internal/ConnectionErrorSpanInterceptor.java b/...io/opentelemetry/instrumentation/okhttp/v3_0/internal/ConnectionErrorSpanInterceptor.java
@@ -21,9 +21,9 @@
  */
 public final class ConnectionErrorSpanInterceptor implements Interceptor {
 
-  private final Instrumenter<Request, Response> instrumenter;
+  private final Instrumenter<Chain, Response> instrumenter;
 
-  public ConnectionErrorSpanInterceptor(Instrumenter<Request, Response> instrumenter) {
+  public ConnectionErrorSpanInterceptor(Instrumenter<Chain, Response> instrumenter) {
     this.instrumenter = instrumenter;
   }
 
@@ -43,9 +43,9 @@ public Response intercept(Chain chain) throws IOException {
     } finally {
       // only create a span when there wasn't any HTTP request
       if (HttpClientRequestResendCount.get(parentContext) == 0) {
-        if (instrumenter.shouldStart(parentContext, request)) {
+        if (instrumenter.shouldStart(parentContext, chain)) {
           InstrumenterUtil.startAndEnd(
-              instrumenter, parentContext, request, response, error, startTime, Instant.now());
+              instrumenter, parentContext, chain, response, error, startTime, Instant.now());
         }
       }
     }

diff --git a/...in/java/io/opentelemetry/instrumentation/okhttp/v3_0/internal/OkHttpAttributesGetter.java b/...in/java/io/opentelemetry/instrumentation/okhttp/v3_0/internal/OkHttpAttributesGetter.java
@@ -6,47 +6,52 @@
 package io.opentelemetry.instrumentation.okhttp.v3_0.internal;
 
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesGetter;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
 import java.util.List;
 import javax.annotation.Nullable;
-import okhttp3.Request;
+import okhttp3.Connection;
+import okhttp3.Interceptor;
 import okhttp3.Response;
 
 /**
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
  * any time.
  */
-public enum OkHttpAttributesGetter implements HttpClientAttributesGetter<Request, Response> {
+public enum OkHttpAttributesGetter
+    implements HttpClientAttributesGetter<Interceptor.Chain, Response> {
   INSTANCE;
 
   @Override
-  public String getHttpRequestMethod(Request request) {
-    return request.method();
+  public String getHttpRequestMethod(Interceptor.Chain chain) {
+    return chain.request().method();
   }
 
   @Override
-  public String getUrlFull(Request request) {
-    return request.url().toString();
+  public String getUrlFull(Interceptor.Chain chain) {
+    return chain.request().url().toString();
   }
 
   @Override
-  public List<String> getHttpRequestHeader(Request request, String name) {
-    return request.headers(name);
+  public List<String> getHttpRequestHeader(Interceptor.Chain chain, String name) {
+    return chain.request().headers(name);
   }
 
   @Override
   public Integer getHttpResponseStatusCode(
-      Request request, Response response, @Nullable Throwable error) {
+      Interceptor.Chain chain, Response response, @Nullable Throwable error) {
     return response.code();
   }
 
   @Override
-  public List<String> getHttpResponseHeader(Request request, Response response, String name) {
+  public List<String> getHttpResponseHeader(
+      Interceptor.Chain chain, Response response, String name) {
     return response.headers(name);
   }
 
   @Nullable
   @Override
-  public String getNetworkProtocolName(Request request, @Nullable Response response) {
+  public String getNetworkProtocolName(Interceptor.Chain chain, @Nullable Response response) {
     if (response == null) {
       return null;
     }
@@ -67,7 +72,7 @@ public String getNetworkProtocolName(Request request, @Nullable Response respons
 
   @Nullable
   @Override
-  public String getNetworkProtocolVersion(Request request, @Nullable Response response) {
+  public String getNetworkProtocolVersion(Interceptor.Chain chain, @Nullable Response response) {
     if (response == null) {
       return null;
     }
@@ -90,12 +95,28 @@ public String getNetworkProtocolVersion(Request request, @Nullable Response resp
 
   @Override
   @Nullable
-  public String getServerAddress(Request request) {
-    return request.url().host();
+  public String getServerAddress(Interceptor.Chain chain) {
+    return chain.request().url().host();
   }
 
   @Override
-  public Integer getServerPort(Request request) {
-    return request.url().port();
+  public Integer getServerPort(Interceptor.Chain chain) {
+    return chain.request().url().port();
+  }
+
+  @Nullable
+  @Override
+  public InetSocketAddress getNetworkPeerInetSocketAddress(
+      Interceptor.Chain chain, @Nullable Response response) {
+    Connection connection = chain.connection();
+    if (connection == null) {
+      return null;
+    }
+    SocketAddress socketAddress = connection.socket().getRemoteSocketAddress();
+    if (socketAddress instanceof InetSocketAddress) {
+      return (InetSocketAddress) socketAddress;
+    } else {
+      return null;
+    }
   }
 }
diff --git a/...elemetry/instrumentation/okhttp/v3_0/internal/OkHttpClientInstrumenterBuilderFactory.java b/...elemetry/instrumentation/okhttp/v3_0/internal/OkHttpClientInstrumenterBuilderFactory.java
@@ -7,7 +7,7 @@
 
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.api.incubator.builder.internal.DefaultHttpClientInstrumenterBuilder;
-import okhttp3.Request;
+import okhttp3.Interceptor;
 import okhttp3.Response;
 
 /**
@@ -19,7 +19,7 @@ public class OkHttpClientInstrumenterBuilderFactory {
 
   private OkHttpClientInstrumenterBuilderFactory() {}
 
-  public static DefaultHttpClientInstrumenterBuilder<Request, Response> create(
+  public static DefaultHttpClientInstrumenterBuilder<Interceptor.Chain, Response> create(
       OpenTelemetry openTelemetry) {
     return new DefaultHttpClientInstrumenterBuilder<>(
         INSTRUMENTATION_NAME, openTelemetry, OkHttpAttributesGetter.INSTANCE);

diff --git a/...c/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/internal/TracingInterceptor.java b/...c/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/internal/TracingInterceptor.java
@@ -20,11 +20,11 @@
  */
 public final class TracingInterceptor implements Interceptor {
 
-  private final Instrumenter<Request, Response> instrumenter;
+  private final Instrumenter<Chain, Response> instrumenter;
   private final ContextPropagators propagators;
 
   public TracingInterceptor(
-      Instrumenter<Request, Response> instrumenter, ContextPropagators propagators) {
+      Instrumenter<Chain, Response> instrumenter, ContextPropagators propagators) {
     this.instrumenter = instrumenter;
     this.propagators = propagators;
   }
@@ -34,11 +34,11 @@ public Response intercept(Chain chain) throws IOException {
     Request request = chain.request();
     Context parentContext = Context.current();
 
-    if (!instrumenter.shouldStart(parentContext, request)) {
+    if (!instrumenter.shouldStart(parentContext, chain)) {
       return chain.proceed(chain.request());
     }
 
-    Context context = instrumenter.start(parentContext, request);
+    Context context = instrumenter.start(parentContext, chain);
     request = injectContextToRequest(request, context);
 
     Response response = null;
@@ -50,7 +50,7 @@ public Response intercept(Chain chain) throws IOException {
       error = e;
       throw e;
     } finally {
-      instrumenter.end(context, request, response, error);
+      instrumenter.end(context, chain, response, error);
     }
   }
 

diff --git a/...sting/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.java b/...sting/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
+import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
 import io.opentelemetry.semconv.NetworkAttributes;
 import java.io.IOException;
 import java.net.URI;
@@ -180,7 +181,16 @@ public void onResponse(Call call, Response response) {
         trace -> {
           trace.hasSpansSatisfyingExactly(
               span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(),
-              span -> span.hasName("GET").hasKind(SpanKind.CLIENT).hasParent(trace.getSpan(0)),
+              span ->
+                  span.hasName("GET")
+                      .hasKind(SpanKind.CLIENT)
+                      .hasParent(trace.getSpan(0))
+                      .hasAttribute(
+                          OpenTelemetryAssertions.satisfies(
+                              NetworkAttributes.NETWORK_PEER_ADDRESS,
+                              val -> val.isIn("127.0.0.1", "0:0:0:0:0:0:0:1")))
+                      .hasAttribute(
+                          NetworkAttributes.NETWORK_PEER_PORT, (long) serverAddress().getPort()),
               span ->
                   span.hasName("test-http-server")
                       .hasKind(SpanKind.SERVER)

diff --git a/...main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/...main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java
@@ -30,6 +30,7 @@
 import io.opentelemetry.semconv.ServerAttributes;
 import io.opentelemetry.semconv.UrlAttributes;
 import io.opentelemetry.semconv.UserAgentAttributes;
+import java.net.InetSocketAddress;
 import java.net.URI;
 import java.time.Duration;
 import java.util.ArrayList;
@@ -87,6 +88,10 @@ void setupOptions() {
     options = builder.build();
   }
 
+  protected InetSocketAddress serverAddress() {
+    return server.httpSocketAddress();
+  }
+
   /**
    * Override this method to configure the {@link HttpClientTestOptions} for the tested HTTP client.
    */
@@ -999,7 +1004,9 @@ SpanDataAssert assertClientSpan(
                 // TODO: Move to test knob rather than always treating as optional
                 if (attrs.get(NetworkAttributes.NETWORK_PEER_ADDRESS) != null) {
                   assertThat(attrs)
-                      .containsEntry(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1");
+                      .hasEntrySatisfying(
+                          NetworkAttributes.NETWORK_PEER_ADDRESS,
+                          addr -> assertThat(addr).isIn("127.0.0.1", "0:0:0:0:0:0:0:1"));
                 }
                 if (attrs.get(NetworkAttributes.NETWORK_PEER_PORT) != null) {
                   assertThat(attrs)