Make server.socket.* attributes on the HTTP server side opt-in

open-telemetry · Jun 16, 2023 · f3ae319 · f3ae319
1 parent 827236e
commit f3ae319
Show file tree

Hide file tree

Showing 25 changed files with 200 additions and 55 deletions.
diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java
@@ -98,7 +98,8 @@ public static <REQUEST, RESPONSE> HttpClientAttributesExtractorBuilder<REQUEST,
             namePortGetter,
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),
-            InternalServerAttributesExtractor.Mode.PEER);
+            InternalServerAttributesExtractor.Mode.PEER,
+            /* captureServerSocketAttributes= */ true);
     this.resendCountIncrementer = resendCountIncrementer;
   }
 

diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
@@ -78,12 +78,14 @@ public static <REQUEST, RESPONSE> HttpServerAttributesExtractorBuilder<REQUEST,
       HttpServerAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
       NetServerAttributesGetter<REQUEST, RESPONSE> netAttributesGetter,
       List<String> capturedRequestHeaders,
-      List<String> capturedResponseHeaders) {
+      List<String> capturedResponseHeaders,
+      boolean captureServerSocketAttributes) {
     this(
         httpAttributesGetter,
         netAttributesGetter,
         capturedRequestHeaders,
         capturedResponseHeaders,
+        captureServerSocketAttributes,
         HttpRouteHolder::getRoute);
   }
 
@@ -93,6 +95,7 @@ public static <REQUEST, RESPONSE> HttpServerAttributesExtractorBuilder<REQUEST,
       NetServerAttributesGetter<REQUEST, RESPONSE> netAttributesGetter,
       List<String> capturedRequestHeaders,
       List<String> capturedResponseHeaders,
+      boolean captureServerSocketAttributes,
       Function<Context, String> httpRouteHolderGetter) {
     super(httpAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
     HttpNetNamePortGetter<REQUEST> namePortGetter =
@@ -119,7 +122,8 @@ public static <REQUEST, RESPONSE> HttpServerAttributesExtractorBuilder<REQUEST,
             namePortGetter,
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),
-            InternalServerAttributesExtractor.Mode.HOST);
+            InternalServerAttributesExtractor.Mode.HOST,
+            captureServerSocketAttributes);
     internalClientExtractor =
         new InternalClientAttributesExtractor<>(
             netAttributesGetter,

diff --git a/...telemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java b/...telemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java
@@ -19,6 +19,7 @@ public final class HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> {
   final NetServerAttributesGetter<REQUEST, RESPONSE> netAttributesGetter;
   List<String> capturedRequestHeaders = emptyList();
   List<String> capturedResponseHeaders = emptyList();
+  boolean captureServerSocketAttributes = false;
 
   HttpServerAttributesExtractorBuilder(
       HttpServerAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
@@ -64,12 +65,30 @@ public HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> setCapturedRespon
     return this;
   }
 
+  /**
+   * Configures the extractor to capture the optional {@code server.socket.address} and {@code
+   * server.socket.port} attributes, which are not collected by default.
+   *
+   * @param captureServerSocketAttributes {@code true} if the extractor should collect the optional
+   *     {@code server.socket.address} and {@code server.socket.port} attributes.
+   */
+  @CanIgnoreReturnValue
+  public HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> setCaptureServerSocketAttributes(
+      boolean captureServerSocketAttributes) {
+    this.captureServerSocketAttributes = captureServerSocketAttributes;
+    return this;
+  }
+
   /**
    * Returns a new {@link HttpServerAttributesExtractor} with the settings of this {@link
    * HttpServerAttributesExtractorBuilder}.
    */
   public AttributesExtractor<REQUEST, RESPONSE> build() {
     return new HttpServerAttributesExtractor<>(
-        httpAttributesGetter, netAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
+        httpAttributesGetter,
+        netAttributesGetter,
+        capturedRequestHeaders,
+        capturedResponseHeaders,
+        captureServerSocketAttributes);
   }
 }
diff --git a/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java b/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java
@@ -54,7 +54,8 @@ private NetClientAttributesExtractor(NetClientAttributesGetter<REQUEST, RESPONSE
             FallbackNamePortGetter.noop(),
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),
-            InternalServerAttributesExtractor.Mode.PEER);
+            InternalServerAttributesExtractor.Mode.PEER,
+            /* captureServerSocketAttributes= */ true);
   }
 
   @Override

diff --git a/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java b/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java
@@ -52,7 +52,8 @@ private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST, RESPONSE
             FallbackNamePortGetter.noop(),
             SemconvStability.emitStableHttpSemconv(),
             SemconvStability.emitOldHttpSemconv(),
-            InternalServerAttributesExtractor.Mode.HOST);
+            InternalServerAttributesExtractor.Mode.HOST,
+            /* captureServerSocketAttributes= */ true);
     internalClientExtractor =
         new InternalClientAttributesExtractor<>(
             getter,

diff --git a/.../io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java b/.../io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesExtractor.java
@@ -41,7 +41,8 @@ public static <REQUEST, RESPONSE> ServerAttributesExtractor<REQUEST, RESPONSE> c
             /* emitStableUrlAttributes= */ true,
             /* emitOldHttpAttributes= */ false,
             // this param does not matter when old semconv is off
-            InternalServerAttributesExtractor.Mode.HOST);
+            InternalServerAttributesExtractor.Mode.HOST,
+            /* captureServerSocketAttributes= */ true);
   }
 
   @Override

diff --git a/.../instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java b/.../instrumentation/api/instrumenter/network/internal/InternalServerAttributesExtractor.java
@@ -27,20 +27,23 @@ public final class InternalServerAttributesExtractor<REQUEST, RESPONSE> {
   private final boolean emitStableUrlAttributes;
   private final boolean emitOldHttpAttributes;
   private final Mode oldSemconvMode;
+  private final boolean captureServerSocketAttributes;
 
   public InternalServerAttributesExtractor(
       ServerAttributesGetter<REQUEST, RESPONSE> getter,
       BiPredicate<Integer, REQUEST> captureServerPortCondition,
       FallbackNamePortGetter<REQUEST> fallbackNamePortGetter,
       boolean emitStableUrlAttributes,
       boolean emitOldHttpAttributes,
-      Mode oldSemconvMode) {
+      Mode oldSemconvMode,
+      boolean captureServerSocketAttributes) {
     this.getter = getter;
     this.captureServerPortCondition = captureServerPortCondition;
     this.fallbackNamePortGetter = fallbackNamePortGetter;
     this.emitStableUrlAttributes = emitStableUrlAttributes;
     this.emitOldHttpAttributes = emitOldHttpAttributes;
     this.oldSemconvMode = oldSemconvMode;
+    this.captureServerSocketAttributes = captureServerSocketAttributes;
   }
 
   public void onStart(AttributesBuilder attributes, REQUEST request) {
@@ -73,7 +76,7 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO
 
     String serverSocketAddress = getter.getServerSocketAddress(request, response);
     if (serverSocketAddress != null && !serverSocketAddress.equals(serverAddress)) {
-      if (emitStableUrlAttributes) {
+      if (emitStableUrlAttributes && captureServerSocketAttributes) {
         internalSet(attributes, NetworkAttributes.SERVER_SOCKET_ADDRESS, serverSocketAddress);
       }
       if (emitOldHttpAttributes) {
@@ -85,7 +88,7 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO
       if (serverSocketPort != null
           && serverSocketPort > 0
           && !serverSocketPort.equals(serverPort)) {
-        if (emitStableUrlAttributes) {
+        if (emitStableUrlAttributes && captureServerSocketAttributes) {
           internalSet(attributes, NetworkAttributes.SERVER_SOCKET_PORT, (long) serverSocketPort);
         }
         if (emitOldHttpAttributes) {
@@ -95,7 +98,7 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO
 
       String serverSocketDomain = getter.getServerSocketDomain(request, response);
       if (serverSocketDomain != null && !serverSocketDomain.equals(serverAddress)) {
-        if (emitStableUrlAttributes) {
+        if (emitStableUrlAttributes && captureServerSocketAttributes) {
           internalSet(attributes, NetworkAttributes.SERVER_SOCKET_DOMAIN, serverSocketDomain);
         }
         if (emitOldHttpAttributes && oldSemconvMode.socketDomain != null) {

diff --git a/...pentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java b/...pentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java
@@ -162,6 +162,7 @@ void normal() {
             new TestNetServerAttributesGetter(),
             singletonList("Custom-Request-Header"),
             singletonList("Custom-Response-Header"),
+            false,
             routeFromContext);
 
     AttributesBuilder startAttributes = Attributes.builder();

diff --git a/...y/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java b/...y/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java
@@ -156,6 +156,7 @@ void normal() {
             new TestNetServerAttributesGetter(),
             singletonList("Custom-Request-Header"),
             singletonList("Custom-Response-Header"),
+            false,
             routeFromContext);
 
     AttributesBuilder startAttributes = Attributes.builder();

diff --git a/...instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java b/...instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java
@@ -38,103 +38,119 @@
 class HttpServerAttributesExtractorStableSemconvTest {
 
   static class TestHttpServerAttributesGetter
-      implements HttpServerAttributesGetter<Map<String, Object>, Map<String, Object>> {
+      implements HttpServerAttributesGetter<Map<String, String>, Map<String, String>> {
 
     @Override
-    public String getHttpRequestMethod(Map<String, Object> request) {
-      return (String) request.get("method");
+    public String getHttpRequestMethod(Map<String, String> request) {
+      return request.get("method");
     }
 
     @Override
-    public String getUrlScheme(Map<String, Object> request) {
-      return (String) request.get("scheme");
+    public String getUrlScheme(Map<String, String> request) {
+      return request.get("scheme");
     }
 
     @Nullable
     @Override
-    public String getUrlPath(Map<String, Object> request) {
-      return (String) request.get("path");
+    public String getUrlPath(Map<String, String> request) {
+      return request.get("path");
     }
 
     @Nullable
     @Override
-    public String getUrlQuery(Map<String, Object> request) {
-      return (String) request.get("query");
+    public String getUrlQuery(Map<String, String> request) {
+      return request.get("query");
     }
 
     @Override
-    public String getHttpRoute(Map<String, Object> request) {
-      return (String) request.get("route");
+    public String getHttpRoute(Map<String, String> request) {
+      return request.get("route");
     }
 
     @Override
-    public List<String> getHttpRequestHeader(Map<String, Object> request, String name) {
-      String values = (String) request.get("header." + name);
+    public List<String> getHttpRequestHeader(Map<String, String> request, String name) {
+      String values = request.get("header." + name);
       return values == null ? emptyList() : asList(values.split(","));
     }
 
     @Override
     public Integer getHttpResponseStatusCode(
-        Map<String, Object> request, Map<String, Object> response, @Nullable Throwable error) {
-      String value = (String) response.get("statusCode");
+        Map<String, String> request, Map<String, String> response, @Nullable Throwable error) {
+      String value = response.get("statusCode");
       return value == null ? null : Integer.parseInt(value);
     }
 
     @Override
     public List<String> getHttpResponseHeader(
-        Map<String, Object> request, Map<String, Object> response, String name) {
-      String values = (String) response.get("header." + name);
+        Map<String, String> request, Map<String, String> response, String name) {
+      String values = response.get("header." + name);
       return values == null ? emptyList() : asList(values.split(","));
     }
   }
 
   static class TestNetServerAttributesGetter
-      implements NetServerAttributesGetter<Map<String, Object>, Map<String, Object>> {
+      implements NetServerAttributesGetter<Map<String, String>, Map<String, String>> {
 
     @Nullable
     @Override
     public String getNetworkTransport(
-        Map<String, Object> request, @Nullable Map<String, Object> response) {
-      return (String) request.get("transport");
+        Map<String, String> request, @Nullable Map<String, String> response) {
+      return request.get("transport");
     }
 
     @Nullable
     @Override
     public String getNetworkType(
-        Map<String, Object> request, @Nullable Map<String, Object> response) {
-      return (String) request.get("type");
+        Map<String, String> request, @Nullable Map<String, String> response) {
+      return request.get("type");
     }
 
     @Nullable
     @Override
     public String getNetworkProtocolName(
-        Map<String, Object> request, Map<String, Object> response) {
-      return (String) request.get("protocolName");
+        Map<String, String> request, Map<String, String> response) {
+      return request.get("protocolName");
     }
 
     @Nullable
     @Override
     public String getNetworkProtocolVersion(
-        Map<String, Object> request, Map<String, Object> response) {
-      return (String) request.get("protocolVersion");
+        Map<String, String> request, Map<String, String> response) {
+      return request.get("protocolVersion");
     }
 
     @Nullable
     @Override
-    public String getServerAddress(Map<String, Object> request) {
-      return (String) request.get("hostName");
+    public String getServerAddress(Map<String, String> request) {
+      return request.get("hostName");
     }
 
     @Nullable
     @Override
-    public Integer getServerPort(Map<String, Object> request) {
-      return (Integer) request.get("hostPort");
+    public Integer getServerPort(Map<String, String> request) {
+      String value = request.get("hostPort");
+      return value == null ? null : Integer.parseInt(value);
+    }
+
+    @Nullable
+    @Override
+    public String getServerSocketAddress(
+        Map<String, String> request, @Nullable Map<String, String> response) {
+      return request.get("serverSocketAddress");
+    }
+
+    @Nullable
+    @Override
+    public Integer getServerSocketPort(
+        Map<String, String> request, @Nullable Map<String, String> response) {
+      String value = request.get("serverSocketPort");
+      return value == null ? null : Integer.parseInt(value);
     }
   }
 
   @Test
   void normal() {
-    Map<String, Object> request = new HashMap<>();
+    Map<String, String> request = new HashMap<>();
     request.put("method", "POST");
     request.put("url", "http://github.com");
     request.put("path", "/repositories/1");
@@ -150,20 +166,23 @@ void normal() {
     request.put("type", "ipv4");
     request.put("protocolName", "http");
     request.put("protocolVersion", "2.0");
+    request.put("serverSocketAddress", "1.2.3.4");
+    request.put("serverSocketPort", "42");
 
-    Map<String, Object> response = new HashMap<>();
+    Map<String, String> response = new HashMap<>();
     response.put("statusCode", "202");
     response.put("header.content-length", "20");
     response.put("header.custom-response-header", "654,321");
 
     Function<Context, String> routeFromContext = ctx -> "/repositories/{repoId}";
 
-    HttpServerAttributesExtractor<Map<String, Object>, Map<String, Object>> extractor =
+    HttpServerAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
         new HttpServerAttributesExtractor<>(
             new TestHttpServerAttributesGetter(),
             new TestNetServerAttributesGetter(),
             singletonList("Custom-Request-Header"),
             singletonList("Custom-Response-Header"),
+            false,
             routeFromContext);
 
     AttributesBuilder startAttributes = Attributes.builder();
@@ -212,9 +231,8 @@ void skipNetworkTransportIfDefaultForProtocol(
     request.put("transport", observedTransport);
 
     AttributesExtractor<Map<String, String>, Map<String, String>> extractor =
-        HttpClientAttributesExtractor.create(
-            new HttpClientAttributesExtractorStableSemconvTest.TestHttpClientAttributesGetter(),
-            new HttpClientAttributesExtractorStableSemconvTest.TestNetClientAttributesGetter());
+        HttpServerAttributesExtractor.create(
+            new TestHttpServerAttributesGetter(), new TestNetServerAttributesGetter());
 
     AttributesBuilder attributes = Attributes.builder();
     extractor.onStart(attributes, Context.root(), request);
@@ -243,4 +261,26 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
           arguments("http", "42", "tcp", "tcp"));
     }
   }
+
+  @Test
+  void optInServerSocketAttributes() {
+    Map<String, String> request = new HashMap<>();
+    request.put("serverSocketAddress", "1.2.3.4");
+    request.put("serverSocketPort", "42");
+
+    AttributesExtractor<Map<String, String>, Map<String, String>> extractor =
+        HttpServerAttributesExtractor.builder(
+                new TestHttpServerAttributesGetter(), new TestNetServerAttributesGetter())
+            .setCaptureServerSocketAttributes(true)
+            .build();
+
+    AttributesBuilder attributes = Attributes.builder();
+    extractor.onStart(attributes, Context.root(), request);
+    extractor.onEnd(attributes, Context.root(), request, emptyMap(), null);
+
+    assertThat(attributes.build())
+        .containsOnly(
+            entry(NetworkAttributes.SERVER_SOCKET_ADDRESS, "1.2.3.4"),
+            entry(NetworkAttributes.SERVER_SOCKET_PORT, 42L));
+  }
 }