Deprecate old server/client socket getter methods (#9716)

open-telemetry · Oct 19, 2023 · e9026cd · e9026cd
1 parent eb92c5e
commit e9026cd
Show file tree

Hide file tree

Showing 50 changed files with 144 additions and 134 deletions.
diff --git a/...metry/instrumentation/api/instrumenter/http/HttpClientPeerServiceAttributesExtractor.java b/...metry/instrumentation/api/instrumenter/http/HttpClientPeerServiceAttributesExtractor.java
@@ -10,6 +10,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.PeerServiceResolver;
 import io.opentelemetry.instrumentation.api.instrumenter.url.UrlParser;
+import io.opentelemetry.instrumentation.api.internal.SemconvStability;
 import io.opentelemetry.semconv.SemanticAttributes;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
@@ -47,6 +48,7 @@ HttpClientPeerServiceAttributesExtractor<REQUEST, RESPONSE> create(
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {}
 
+  @SuppressWarnings("deprecation") // old semconv
   @Override
   public void onEnd(
       AttributesBuilder attributes,
@@ -64,7 +66,7 @@ public void onEnd(
     Integer serverPort = attributesGetter.getServerPort(request);
     Supplier<String> pathSupplier = () -> getUrlPath(attributesGetter, request);
     String peerService = mapToPeerService(serverAddress, serverPort, pathSupplier);
-    if (peerService == null) {
+    if (peerService == null && SemconvStability.emitOldHttpSemconv()) {
       String serverSocketDomain = attributesGetter.getServerSocketDomain(request, response);
       Integer serverSocketPort = attributesGetter.getServerSocketPort(request, response);
       peerService = mapToPeerService(serverSocketDomain, serverSocketPort, null);

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java
@@ -59,20 +59,20 @@ default String getNetworkType(REQUEST request, @Nullable RESPONSE response) {
 
   @Nullable
   @Override
-  default InetSocketAddress getNetworkPeerInetSocketAddress(
+  default InetSocketAddress getServerInetSocketAddress(
       REQUEST request, @Nullable RESPONSE response) {
-    return getServerInetSocketAddress(request, response);
+    return getNetworkPeerInetSocketAddress(request, response);
   }
 
   @Nullable
   @Override
-  default String getNetworkPeerAddress(REQUEST request, @Nullable RESPONSE response) {
-    return getServerSocketAddress(request, response);
+  default String getServerSocketAddress(REQUEST request, @Nullable RESPONSE response) {
+    return getNetworkPeerAddress(request, response);
   }
 
   @Nullable
   @Override
-  default Integer getNetworkPeerPort(REQUEST request, @Nullable RESPONSE response) {
-    return getServerSocketPort(request, response);
+  default Integer getServerSocketPort(REQUEST request, @Nullable RESPONSE response) {
+    return getNetworkPeerPort(request, response);
   }
 }
diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java
@@ -65,39 +65,39 @@ default String getNetworkType(REQUEST request, @Nullable RESPONSE response) {
 
   @Nullable
   @Override
-  default InetSocketAddress getNetworkLocalInetSocketAddress(
+  default InetSocketAddress getServerInetSocketAddress(
       REQUEST request, @Nullable RESPONSE response) {
-    return getServerInetSocketAddress(request, response);
+    return getNetworkLocalInetSocketAddress(request, response);
   }
 
   @Nullable
   @Override
-  default String getNetworkLocalAddress(REQUEST request, @Nullable RESPONSE response) {
-    return getServerSocketAddress(request, response);
+  default String getServerSocketAddress(REQUEST request, @Nullable RESPONSE response) {
+    return getNetworkLocalAddress(request, response);
   }
 
   @Nullable
   @Override
-  default Integer getNetworkLocalPort(REQUEST request, @Nullable RESPONSE response) {
-    return getServerSocketPort(request, response);
+  default Integer getServerSocketPort(REQUEST request, @Nullable RESPONSE response) {
+    return getNetworkLocalPort(request, response);
   }
 
   @Nullable
   @Override
-  default InetSocketAddress getNetworkPeerInetSocketAddress(
+  default InetSocketAddress getClientInetSocketAddress(
       REQUEST request, @Nullable RESPONSE response) {
-    return getClientInetSocketAddress(request, response);
+    return getNetworkPeerInetSocketAddress(request, response);
   }
 
   @Nullable
   @Override
-  default String getNetworkPeerAddress(REQUEST request, @Nullable RESPONSE response) {
-    return getClientSocketAddress(request, response);
+  default String getClientSocketAddress(REQUEST request, @Nullable RESPONSE response) {
+    return getNetworkPeerAddress(request, response);
   }
 
   @Nullable
   @Override
-  default Integer getNetworkPeerPort(REQUEST request, @Nullable RESPONSE response) {
-    return getClientSocketPort(request, response);
+  default Integer getClientSocketPort(REQUEST request, @Nullable RESPONSE response) {
+    return getNetworkPeerPort(request, response);
   }
 }
diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/net/PeerServiceAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/net/PeerServiceAttributesExtractor.java
@@ -45,6 +45,7 @@ public static <REQUEST, RESPONSE> AttributesExtractor<REQUEST, RESPONSE> create(
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {}
 
+  @SuppressWarnings("deprecation") // old semconv
   @Override
   public void onEnd(
       AttributesBuilder attributes,

diff --git a/...ava/io/opentelemetry/instrumentation/api/instrumenter/network/ClientAttributesGetter.java b/...ava/io/opentelemetry/instrumentation/api/instrumenter/network/ClientAttributesGetter.java
@@ -38,14 +38,16 @@ default Integer getClientPort(REQUEST request) {
     return null;
   }
 
-  // TODO deprecate
-
   /**
    * Returns an {@link InetSocketAddress} object representing the immediate client socket address.
    *
    * <p>Implementing this method is equivalent to implementing all of {@link
    * #getClientSocketAddress(Object, Object)} and {@link #getClientSocketPort(Object, Object)}.
+   *
+   * @deprecated Implement {@link NetworkAttributesGetter#getNetworkPeerInetSocketAddress(Object,
+   *     Object)} instead.
    */
+  @Deprecated
   @Nullable
   default InetSocketAddress getClientInetSocketAddress(
       REQUEST request, @Nullable RESPONSE response) {
@@ -62,7 +64,11 @@ default InetSocketAddress getClientInetSocketAddress(
    * simply return {@code null}. If the instrumented library does not expose {@link
    * InetSocketAddress} in its API, you might want to implement this method instead of {@link
    * #getClientInetSocketAddress(Object, Object)}.
+   *
+   * @deprecated Implement {@link NetworkAttributesGetter#getNetworkPeerAddress(Object, Object)}
+   *     instead.
    */
+  @Deprecated
   @Nullable
   default String getClientSocketAddress(REQUEST request, @Nullable RESPONSE response) {
     return InetSocketAddressUtil.getIpAddress(getClientInetSocketAddress(request, response));
@@ -78,7 +84,11 @@ default String getClientSocketAddress(REQUEST request, @Nullable RESPONSE respon
    * simply return {@code null}. If the instrumented library does not expose {@link
    * InetSocketAddress} in its API, you might want to implement this method instead of {@link
    * #getClientInetSocketAddress(Object, Object)}.
+   *
+   * @deprecated Implement {@link NetworkAttributesGetter#getNetworkPeerPort(Object, Object)}
+   *     instead.
    */
+  @Deprecated
   @Nullable
   default Integer getClientSocketPort(REQUEST request, @Nullable RESPONSE response) {
     return InetSocketAddressUtil.getPort(getClientInetSocketAddress(request, response));

diff --git a/...ava/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesGetter.java b/...ava/io/opentelemetry/instrumentation/api/instrumenter/network/ServerAttributesGetter.java
@@ -47,7 +47,12 @@ default Integer getServerPort(REQUEST request) {
    * <p>Implementing this method is equivalent to implementing all of {@link
    * #getServerSocketDomain(Object, Object)}, {@link #getServerSocketAddress(Object, Object)} and
    * {@link #getServerSocketPort(Object, Object)}.
+   *
+   * @deprecated Implement {@link NetworkAttributesGetter#getNetworkPeerInetSocketAddress(Object,
+   *     Object)} or {@link NetworkAttributesGetter#getNetworkLocalInetSocketAddress(Object,
+   *     Object)} instead.
    */
+  @Deprecated
   @Nullable
   default InetSocketAddress getServerInetSocketAddress(
       REQUEST request, @Nullable RESPONSE response) {
@@ -64,7 +69,10 @@ default InetSocketAddress getServerInetSocketAddress(
    * simply return {@code null}. If the instrumented library does not expose {@link
    * InetSocketAddress} in its API, you might want to implement this method instead of {@link
    * #getServerInetSocketAddress(Object, Object)}.
+   *
+   * @deprecated This method is deprecated and will be removed without replacement.
    */
+  @Deprecated
   @Nullable
   default String getServerSocketDomain(REQUEST request, @Nullable RESPONSE response) {
     return InetSocketAddressUtil.getDomainName(getServerInetSocketAddress(request, response));
@@ -80,7 +88,11 @@ default String getServerSocketDomain(REQUEST request, @Nullable RESPONSE respons
    * simply return {@code null}. If the instrumented library does not expose {@link
    * InetSocketAddress} in its API, you might want to implement this method instead of {@link
    * #getServerInetSocketAddress(Object, Object)}.
+   *
+   * @deprecated Implement {@link NetworkAttributesGetter#getNetworkPeerAddress(Object, Object)} or
+   *     {@link NetworkAttributesGetter#getNetworkLocalAddress(Object, Object)} instead.
    */
+  @Deprecated
   @Nullable
   default String getServerSocketAddress(REQUEST request, @Nullable RESPONSE response) {
     return InetSocketAddressUtil.getIpAddress(getServerInetSocketAddress(request, response));
@@ -96,7 +108,11 @@ default String getServerSocketAddress(REQUEST request, @Nullable RESPONSE respon
    * simply return {@code null}. If the instrumented library does not expose {@link
    * InetSocketAddress} in its API, you might want to implement this method instead of {@link
    * #getServerInetSocketAddress(Object, Object)}.
+   *
+   * @deprecated Implement {@link NetworkAttributesGetter#getNetworkPeerPort(Object, Object)} or
+   *     {@link NetworkAttributesGetter#getNetworkLocalPort(Object, Object)} instead.
    */
+  @Deprecated
   @Nullable
   default Integer getServerSocketPort(REQUEST request, @Nullable RESPONSE response) {
     return InetSocketAddressUtil.getPort(getServerInetSocketAddress(request, response));

diff --git a/...y/instrumentation/api/instrumenter/http/HttpClientPeerServiceAttributesExtractorTest.java b/...y/instrumentation/api/instrumenter/http/HttpClientPeerServiceAttributesExtractorTest.java
@@ -76,6 +76,7 @@ void shouldNotSetAnyValueIfPeerNameDoesNotMatch() {
     assertTrue(endAttributes.build().isEmpty());
   }
 
+  @SuppressWarnings("deprecation") // old semconv
   @Test
   void shouldSetPeerNameIfItMatches() {
     // given
@@ -106,6 +107,7 @@ void shouldSetPeerNameIfItMatches() {
     verify(httpAttributesExtractor, never()).getServerSocketDomain(any(), any());
   }
 
+  @SuppressWarnings("deprecation") // old semconv
   @Test
   void shouldSetSockPeerNameIfItMatchesAndNoPeerNameProvided() {
     // given

diff --git a/.../instrumentation/api/instrumenter/net/InetSocketAddressNetClientAttributesGetterTest.java b/.../instrumentation/api/instrumenter/net/InetSocketAddressNetClientAttributesGetterTest.java
@@ -38,7 +38,7 @@ public Integer getServerPort(InetSocketAddress request) {
     }
 
     @Override
-    public InetSocketAddress getServerInetSocketAddress(
+    public InetSocketAddress getNetworkPeerInetSocketAddress(
         InetSocketAddress request, InetSocketAddress response) {
       return response;
     }

diff --git a/.../instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetterTest.java b/.../instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetterTest.java
@@ -39,12 +39,14 @@ public Integer getServerPort(Addresses request) {
     }
 
     @Override
-    public InetSocketAddress getClientInetSocketAddress(Addresses request, Addresses response) {
+    public InetSocketAddress getNetworkPeerInetSocketAddress(
+        Addresses request, Addresses response) {
       return request.peer;
     }
 
     @Override
-    public InetSocketAddress getServerInetSocketAddress(Addresses request, Addresses response) {
+    public InetSocketAddress getNetworkLocalInetSocketAddress(
+        Addresses request, Addresses response) {
       return request.host;
     }
   }

diff --git a/.../opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractorTest.java b/.../opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractorTest.java
@@ -76,20 +76,12 @@ public String getSockFamily(Map<String, String> request, Map<String, String> res
     }
 
     @Override
-    public String getServerSocketDomain(Map<String, String> request, Map<String, String> response) {
-      // we only support the InetSocketAddress case here; nearly no instrumentations override this
-      // method anyway
-      return null;
-    }
-
-    @Override
-    public String getServerSocketAddress(
-        Map<String, String> request, Map<String, String> response) {
+    public String getNetworkPeerAddress(Map<String, String> request, Map<String, String> response) {
       return response.get("sockPeerAddr");
     }
 
     @Override
-    public Integer getServerSocketPort(Map<String, String> request, Map<String, String> response) {
+    public Integer getNetworkPeerPort(Map<String, String> request, Map<String, String> response) {
       String sockPeerPort = response.get("sockPeerPort");
       return sockPeerPort == null ? null : Integer.valueOf(sockPeerPort);
     }

diff --git a/.../opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractorTest.java b/.../opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractorTest.java
@@ -75,25 +75,25 @@ public String getSockFamily(Map<String, String> request) {
     }
 
     @Override
-    public String getClientSocketAddress(Map<String, String> request, Void response) {
+    public String getNetworkPeerAddress(Map<String, String> request, Void response) {
       return request.get("sockPeerAddr");
     }
 
     @Override
-    public Integer getClientSocketPort(Map<String, String> request, Void response) {
+    public Integer getNetworkPeerPort(Map<String, String> request, Void response) {
       String sockPeerPort = request.get("sockPeerPort");
       return sockPeerPort == null ? null : Integer.valueOf(sockPeerPort);
     }
 
     @Nullable
     @Override
-    public String getServerSocketAddress(Map<String, String> request, Void response) {
+    public String getNetworkLocalAddress(Map<String, String> request, Void response) {
       return request.get("sockHostAddr");
     }
 
     @Nullable
     @Override
-    public Integer getServerSocketPort(Map<String, String> request, Void response) {
+    public Integer getNetworkLocalPort(Map<String, String> request, Void response) {
       String sockHostPort = request.get("sockHostPort");
       return sockHostPort == null ? null : Integer.valueOf(sockHostPort);
     }

diff --git a/...pentelemetry/instrumentation/api/instrumenter/net/PeerServiceAttributesExtractorTest.java b/...pentelemetry/instrumentation/api/instrumenter/net/PeerServiceAttributesExtractorTest.java
@@ -77,6 +77,7 @@ void shouldNotSetAnyValueIfPeerNameDoesNotMatch() {
   }
 
   @Test
+  @SuppressWarnings("deprecation") // old semconv
   void shouldSetPeerNameIfItMatches() {
     // given
     Map<String, String> peerServiceMapping = new HashMap<>();
@@ -105,6 +106,7 @@ void shouldSetPeerNameIfItMatches() {
     verify(netAttributesExtractor, never()).getServerSocketDomain(any(), any());
   }
 
+  @SuppressWarnings("deprecation") // old semconv
   @Test
   void shouldSetSockPeerNameIfItMatchesAndNoPeerNameProvided() {
 

diff --git a/...try/instrumentation/api/instrumenter/net/NetClientAttributesExtractorBothSemconvTest.java b/...try/instrumentation/api/instrumenter/net/NetClientAttributesExtractorBothSemconvTest.java
@@ -76,18 +76,12 @@ public String getSockFamily(Map<String, String> request, Map<String, String> res
     }
 
     @Override
-    public String getServerSocketDomain(Map<String, String> request, Map<String, String> response) {
-      return response.get("sockPeerName");
-    }
-
-    @Override
-    public String getServerSocketAddress(
-        Map<String, String> request, Map<String, String> response) {
+    public String getNetworkPeerAddress(Map<String, String> request, Map<String, String> response) {
       return response.get("sockPeerAddr");
     }
 
     @Override
-    public Integer getServerSocketPort(Map<String, String> request, Map<String, String> response) {
+    public Integer getNetworkPeerPort(Map<String, String> request, Map<String, String> response) {
       String sockPeerPort = response.get("sockPeerPort");
       return sockPeerPort == null ? null : Integer.valueOf(sockPeerPort);
     }

diff --git a/...try/instrumentation/api/instrumenter/net/NetServerAttributesExtractorBothSemconvTest.java b/...try/instrumentation/api/instrumenter/net/NetServerAttributesExtractorBothSemconvTest.java
@@ -75,25 +75,25 @@ public String getSockFamily(Map<String, String> request) {
     }
 
     @Override
-    public String getClientSocketAddress(Map<String, String> request, Void response) {
+    public String getNetworkPeerAddress(Map<String, String> request, Void response) {
       return request.get("sockPeerAddr");
     }
 
     @Override
-    public Integer getClientSocketPort(Map<String, String> request, Void response) {
+    public Integer getNetworkPeerPort(Map<String, String> request, Void response) {
       String sockPeerPort = request.get("sockPeerPort");
       return sockPeerPort == null ? null : Integer.valueOf(sockPeerPort);
     }
 
     @Nullable
     @Override
-    public String getServerSocketAddress(Map<String, String> request, Void response) {
+    public String getNetworkLocalAddress(Map<String, String> request, Void response) {
       return request.get("sockHostAddr");
     }
 
     @Nullable
     @Override
-    public Integer getServerSocketPort(Map<String, String> request, Void response) {
+    public Integer getNetworkLocalPort(Map<String, String> request, Void response) {
       String sockHostPort = request.get("sockHostPort");
       return sockHostPort == null ? null : Integer.valueOf(sockHostPort);
     }

diff --git a/...y/instrumentation/api/instrumenter/net/NetClientAttributesExtractorStableSemconvTest.java b/...y/instrumentation/api/instrumenter/net/NetClientAttributesExtractorStableSemconvTest.java
@@ -76,13 +76,12 @@ public String getSockFamily(Map<String, String> request, Map<String, String> res
     }
 
     @Override
-    public String getServerSocketAddress(
-        Map<String, String> request, Map<String, String> response) {
+    public String getNetworkPeerAddress(Map<String, String> request, Map<String, String> response) {
       return response.get("sockPeerAddr");
     }
 
     @Override
-    public Integer getServerSocketPort(Map<String, String> request, Map<String, String> response) {
+    public Integer getNetworkPeerPort(Map<String, String> request, Map<String, String> response) {
       String sockPeerPort = response.get("sockPeerPort");
       return sockPeerPort == null ? null : Integer.valueOf(sockPeerPort);
     }