Update net semantic convention changes (open-telemetry#6268)

* New net conventions: option a * Feedback + sock.family + sock.peer.name * peer.service + tests * server net attributes attempt 1 * server net attributes attempt 2 * Javadoc * Revisions * Apply to instrumentations * Feedback * One more default method * Spotless * Fix javadoc
helios · Oct 23, 2022 · 2d5dfd2 · 2d5dfd2
1 parent 5abfd8d
commit 2d5dfd2
Show file tree

Hide file tree

Showing 106 changed files with 560 additions and 732 deletions.
diff --git a/...ava/io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractor.java b/...ava/io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractor.java
@@ -76,10 +76,6 @@ public void onEnd(
 
     String peerName = attributesGetter.peerName(request, response);
     String peerService = mapToPeerService(peerName);
-    if (peerService == null) {
-      String peerIp = attributesGetter.peerIp(request, response);
-      peerService = mapToPeerService(peerIp);
-    }
     if (peerService != null) {
       attributes.put(SemanticAttributes.PEER_SERVICE, peerService);
     }

diff --git a/...etry/instrumentation/api/instrumenter/net/InetSocketAddressNetClientAttributesGetter.java b/...etry/instrumentation/api/instrumenter/net/InetSocketAddressNetClientAttributesGetter.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.net;
 
+import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import javax.annotation.Nullable;
@@ -44,7 +45,7 @@ public final Integer peerPort(REQUEST request, @Nullable RESPONSE response) {
 
   @Override
   @Nullable
-  public final String peerIp(REQUEST request, @Nullable RESPONSE response) {
+  public final String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
     InetSocketAddress address = getAddress(request, response);
     if (address == null) {
       return null;
@@ -55,4 +56,28 @@ public final String peerIp(REQUEST request, @Nullable RESPONSE response) {
     }
     return null;
   }
+
+  @Nullable
+  @Override
+  public Integer sockPeerPort(REQUEST request, @Nullable RESPONSE response) {
+    InetSocketAddress address = getAddress(request, response);
+    if (address == null) {
+      return null;
+    }
+    return address.getPort();
+  }
+
+  @Nullable
+  @Override
+  public String sockFamily(REQUEST request, @Nullable RESPONSE response) {
+    InetSocketAddress address = getAddress(request, response);
+    if (address == null) {
+      return null;
+    }
+    InetAddress remoteAddress = address.getAddress();
+    if (remoteAddress instanceof Inet6Address) {
+      return "inet6";
+    }
+    return null;
+  }
 }
diff --git a/...etry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetter.java b/...etry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetter.java
@@ -24,7 +24,7 @@ public abstract class InetSocketAddressNetServerAttributesGetter<REQUEST>
 
   @Override
   @Nullable
-  public final Integer peerPort(REQUEST request) {
+  public final Integer sockPeerPort(REQUEST request) {
     InetSocketAddress address = getAddress(request);
     if (address == null) {
       return null;
@@ -34,7 +34,7 @@ public final Integer peerPort(REQUEST request) {
 
   @Override
   @Nullable
-  public final String peerIp(REQUEST request) {
+  public final String sockPeerAddr(REQUEST request) {
     InetSocketAddress address = getAddress(request);
     if (address == null) {
       return null;

diff --git a/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java b/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java
@@ -7,6 +7,7 @@
 
 import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;
 
+import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
@@ -25,6 +26,15 @@
 public final class NetClientAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
+  private static final AttributeKey<String> NET_SOCK_PEER_ADDR =
+      AttributeKey.stringKey("net.sock.peer.addr");
+  public static final AttributeKey<Long> NET_SOCK_PEER_PORT =
+      AttributeKey.longKey("net.sock.peer.port");
+  public static final AttributeKey<String> NET_SOCK_FAMILY =
+      AttributeKey.stringKey("net.sock.family");
+  public static final AttributeKey<String> NET_SOCK_PEER_NAME =
+      AttributeKey.stringKey("net.sock.peer.name");
+
   private final NetClientAttributesGetter<REQUEST, RESPONSE> getter;
 
   public static <REQUEST, RESPONSE> NetClientAttributesExtractor<REQUEST, RESPONSE> create(
@@ -49,17 +59,30 @@ public void onEnd(
 
     internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request, response));
 
-    String peerIp = getter.peerIp(request, response);
     String peerName = getter.peerName(request, response);
-
-    if (peerName != null && !peerName.equals(peerIp)) {
+    Integer peerPort = getter.peerPort(request, response);
+    if (peerName != null) {
       internalSet(attributes, SemanticAttributes.NET_PEER_NAME, peerName);
+      if (peerPort != null && peerPort > 0) {
+        internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
+      }
     }
-    internalSet(attributes, SemanticAttributes.NET_PEER_IP, peerIp);
 
-    Integer peerPort = getter.peerPort(request, response);
-    if (peerPort != null && peerPort > 0) {
-      internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
+    String sockPeerAddr = getter.sockPeerAddr(request, response);
+    if (sockPeerAddr != null && !sockPeerAddr.equals(peerName)) {
+      internalSet(attributes, NET_SOCK_PEER_ADDR, sockPeerAddr);
+
+      Integer sockPeerPort = getter.sockPeerPort(request, response);
+      if (sockPeerPort != null && sockPeerPort > 0 && !sockPeerPort.equals(peerPort)) {
+        internalSet(attributes, NET_SOCK_PEER_PORT, (long) sockPeerPort);
+      }
+
+      internalSet(attributes, NET_SOCK_FAMILY, getter.sockFamily(request, response));
+
+      String sockPeerName = getter.sockPeerName(request, response);
+      if (sockPeerName != null && !sockPeerName.equals(peerName)) {
+        internalSet(attributes, NET_SOCK_PEER_NAME, sockPeerName);
+      }
     }
   }
 }
diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java
@@ -26,6 +26,34 @@ public interface NetClientAttributesGetter<REQUEST, RESPONSE> {
   @Nullable
   Integer peerPort(REQUEST request, @Nullable RESPONSE response);
 
+  /**
+   * Returns the peerIp.
+   *
+   * @deprecated implement {@link #sockPeerAddr(Object, Object)} instead.
+   */
+  @Deprecated
   @Nullable
-  String peerIp(REQUEST request, @Nullable RESPONSE response);
+  default String peerIp(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
+
+  @Nullable
+  default String sockPeerAddr(REQUEST request, @Nullable RESPONSE response) {
+    return peerIp(request, response);
+  }
+
+  @Nullable
+  default Integer sockPeerPort(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
+
+  @Nullable
+  default String sockFamily(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
+
+  @Nullable
+  default String sockPeerName(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
 }
diff --git a/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java b/...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java
@@ -7,6 +7,7 @@
 
 import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;
 
+import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
@@ -22,6 +23,10 @@
 public final class NetServerAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
+  public static final AttributeKey<String> NET_SOCK_PEER_ADDR =
+      AttributeKey.stringKey("net.sock.peer.addr");
+  public static final AttributeKey<Long> NET_SOCK_PEER_PORT =
+      AttributeKey.longKey("net.sock.peer.port");
   private final NetServerAttributesGetter<REQUEST> getter;
 
   public static <REQUEST, RESPONSE> NetServerAttributesExtractor<REQUEST, RESPONSE> create(
@@ -37,13 +42,13 @@ private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST> getter)
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request));
 
-    String peerIp = getter.peerIp(request);
+    String sockPeerAddr = getter.sockPeerAddr(request);
 
-    internalSet(attributes, SemanticAttributes.NET_PEER_IP, peerIp);
+    internalSet(attributes, NET_SOCK_PEER_ADDR, sockPeerAddr);
 
-    Integer peerPort = getter.peerPort(request);
-    if (peerPort != null && peerPort > 0) {
-      internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
+    Integer sockPeerPort = getter.sockPeerPort(request);
+    if (sockPeerPort != null && sockPeerPort > 0) {
+      internalSet(attributes, NET_SOCK_PEER_PORT, (long) sockPeerPort);
     }
   }
 

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java
@@ -8,8 +8,8 @@
 import javax.annotation.Nullable;
 
 /**
- * An interface for getting server-based network attributes. It adapts a vendor-specific request
- * type into the 3 common attributes (transport, peerPort, peerIp).
+ * An interface for getting server-based network attributes. It adapts an instrumentation-specific
+ * request type into the 3 common attributes (transport, sockPeerPort, sockPeerAddr).
  *
  * <p>Instrumentation authors will create implementations of this interface for their specific
  * server library/framework. It will be used by the {@link NetServerAttributesExtractor} to obtain
@@ -20,9 +20,37 @@ public interface NetServerAttributesGetter<REQUEST> {
   @Nullable
   String transport(REQUEST request);
 
+  /**
+   * Returns the peerPort.
+   *
+   * @deprecated implement {@link #sockPeerPort(Object)} instead.
+   */
+  @Deprecated
   @Nullable
-  Integer peerPort(REQUEST request);
+  default Integer peerPort(REQUEST request) {
+    return null;
+  }
 
+  /**
+   * Returns the peerIp.
+   *
+   * @deprecated implement {@link #sockPeerAddr(Object)} instead.
+   */
+  @Deprecated
   @Nullable
-  String peerIp(REQUEST request);
+  default String peerIp(REQUEST request) {
+    return null;
+  }
+
+  @Nullable
+  default Integer sockPeerPort(REQUEST request) {
+    // TODO (trask) remove default after removing peerPort() method
+    return peerPort(request);
+  }
+
+  @Nullable
+  default String sockPeerAddr(REQUEST request) {
+    // TODO (trask) remove default after removing peerIp() method
+    return peerIp(request);
+  }
 }
diff --git a/...conv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/MetricsView.java b/...conv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/MetricsView.java
@@ -20,7 +20,6 @@ final class MetricsView {
 
   private static final Set<AttributeKey> alwaysInclude = buildAlwaysInclude();
   private static final Set<AttributeKey> clientView = buildClientView();
-  private static final Set<AttributeKey> clientFallbackView = buildClientFallbackView();
   private static final Set<AttributeKey> serverView = buildServerView();
   private static final Set<AttributeKey> serverFallbackView = buildServerFallbackView();
 
@@ -44,16 +43,6 @@ private static Set<AttributeKey> buildClientView() {
     return view;
   }
 
-  private static Set<AttributeKey> buildClientFallbackView() {
-    // the list of rpc client metrics attributes is from
-    // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md#attributes
-    Set<AttributeKey> view = new HashSet<>(alwaysInclude);
-    view.add(SemanticAttributes.NET_PEER_IP);
-    view.add(SemanticAttributes.NET_PEER_PORT);
-    view.add(SemanticAttributes.NET_TRANSPORT);
-    return view;
-  }
-
   private static Set<AttributeKey> buildServerView() {
     // the list of rpc server metrics attributes is from
     // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md#attributes
@@ -78,11 +67,7 @@ private static <T> boolean containsAttribute(
   }
 
   static Attributes applyClientView(Attributes startAttributes, Attributes endAttributes) {
-    Set<AttributeKey> fullSet = clientView;
-    if (!containsAttribute(SemanticAttributes.NET_PEER_NAME, startAttributes, endAttributes)) {
-      fullSet = clientFallbackView;
-    }
-    return applyView(fullSet, startAttributes, endAttributes);
+    return applyView(clientView, startAttributes, endAttributes);
   }
 
   static Attributes applyServerView(Attributes startAttributes, Attributes endAttributes) {

diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractorTest.java b/...io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractorTest.java
@@ -70,29 +70,6 @@ void shouldNotSetAnyValueIfPeerNameDoesNotMatch() {
     assertTrue(endAttributes.build().isEmpty());
   }
 
-  @Test
-  void shouldNotSetAnyValueIfPeerIpDoesNotMatch() {
-    // given
-    Map<String, String> peerServiceMapping = singletonMap("1.2.3.4", "myService");
-
-    PeerServiceAttributesExtractor<String, String> underTest =
-        new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceMapping);
-
-    given(netAttributesExtractor.peerIp(any(), any())).willReturn("1.2.3.5");
-
-    Context context = Context.root();
-
-    // when
-    AttributesBuilder startAttributes = Attributes.builder();
-    underTest.onStart(startAttributes, context, "request");
-    AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, context, "request", "response", null);
-
-    // then
-    assertTrue(startAttributes.build().isEmpty());
-    assertTrue(endAttributes.build().isEmpty());
-  }
-
   @Test
   void shouldSetPeerNameIfItMatches() {
     // given
@@ -118,31 +95,4 @@ void shouldSetPeerNameIfItMatches() {
     assertThat(endAttributes.build())
         .containsOnly(entry(SemanticAttributes.PEER_SERVICE, "myService"));
   }
-
-  @Test
-  void shouldSetPeerIpIfItMatchesAndNameDoesNot() {
-    // given
-    Map<String, String> peerServiceMapping = new HashMap<>();
-    peerServiceMapping.put("example.com", "myService");
-    peerServiceMapping.put("1.2.3.4", "someOtherService");
-
-    PeerServiceAttributesExtractor<String, String> underTest =
-        new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceMapping);
-
-    given(netAttributesExtractor.peerName(any(), any())).willReturn("test.com");
-    given(netAttributesExtractor.peerIp(any(), any())).willReturn("1.2.3.4");
-
-    Context context = Context.root();
-
-    // when
-    AttributesBuilder startAttributes = Attributes.builder();
-    underTest.onStart(startAttributes, context, "request");
-    AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, context, "request", "response", null);
-
-    // then
-    assertThat(startAttributes.build()).isEmpty();
-    assertThat(endAttributes.build())
-        .containsOnly(entry(SemanticAttributes.PEER_SERVICE, "someOtherService"));
-  }
 }
diff --git a/...st/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java b/...st/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java
@@ -39,7 +39,6 @@ void collectsMetrics() {
             .put("http.target", "/")
             .put("http.scheme", "https")
             .put("net.peer.name", "localhost")
-            .put("net.peer.ip", "0.0.0.0")
             .put("net.peer.port", 1234)
             .put("http.request_content_length", 100)
             .build();

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsViewTest.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsViewTest.java
@@ -34,7 +34,6 @@ void shouldApplyClientDurationAndSizeView() {
         Attributes.builder()
             .put(SemanticAttributes.HTTP_STATUS_CODE, 500)
             .put(SemanticAttributes.NET_PEER_NAME, "somehost2")
-            .put(SemanticAttributes.NET_PEER_IP, "127.0.0.1")
             .put(SemanticAttributes.NET_PEER_PORT, 443)
             .build();
 
@@ -70,7 +69,6 @@ void shouldApplyServerDurationAndSizeView() {
         Attributes.builder()
             .put(SemanticAttributes.HTTP_STATUS_CODE, 500)
             .put(SemanticAttributes.NET_PEER_NAME, "somehost2")
-            .put(SemanticAttributes.NET_PEER_IP, "127.0.0.1")
             .put(SemanticAttributes.NET_PEER_PORT, 443)
             .build();