Add protocol name&version to net attribute getters (#7994)

In preparation for open-telemetry/opentelemetry-specification#3272
open-telemetry · Mar 9, 2023 · 1649a90 · 1649a90
1 parent 998e781
commit 1649a90
Show file tree

Hide file tree

Showing 24 changed files with 111 additions and 201 deletions.
diff --git a/...pentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractor.java b/...pentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractor.java
@@ -81,11 +81,6 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
           SemanticAttributes.MESSAGING_DESTINATION_NAME,
           getter.getDestination(request));
     }
-    internalSet(attributes, SemanticAttributes.NET_APP_PROTOCOL_NAME, getter.getProtocol(request));
-    internalSet(
-        attributes,
-        SemanticAttributes.NET_APP_PROTOCOL_VERSION,
-        getter.getProtocolVersion(request));
     internalSet(
         attributes,
         SemanticAttributes.MESSAGING_MESSAGE_CONVERSATION_ID,

diff --git a/...o/opentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesGetter.java b/...o/opentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesGetter.java
@@ -7,6 +7,7 @@
 
 import static java.util.Collections.emptyList;
 
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import java.util.List;
 import javax.annotation.Nullable;
 
@@ -30,11 +31,27 @@ public interface MessagingAttributesGetter<REQUEST, RESPONSE> {
 
   boolean isTemporaryDestination(REQUEST request);
 
+  /**
+   * Returns the application protocol used.
+   *
+   * @deprecated Use {@link NetClientAttributesGetter#getProtocolName(Object, Object)} instead.
+   */
+  @Deprecated
   @Nullable
-  String getProtocol(REQUEST request);
+  default String getProtocol(REQUEST request) {
+    return null;
+  }
 
+  /**
+   * Returns the version of the application protocol used.
+   *
+   * @deprecated Use {@link NetClientAttributesGetter#getProtocolVersion(Object, Object)} instead.
+   */
+  @Deprecated
   @Nullable
-  String getProtocolVersion(REQUEST request);
+  default String getProtocolVersion(REQUEST request) {
+    return null;
+  }
 
   /**
    * Returns the application protocol used.

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java
@@ -20,6 +20,28 @@ public interface NetClientAttributesGetter<REQUEST, RESPONSE> {
   @Nullable
   String getTransport(REQUEST request, @Nullable RESPONSE response);
 
+  // TODO: make required after the 1.24 release
+  /**
+   * Returns the application protocol used.
+   *
+   * <p>Examples: `amqp`, `http`, `mqtt`.
+   */
+  @Nullable
+  default String getProtocolName(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
+
+  // TODO: make required after the 1.24 release
+  /**
+   * Returns the version of the application protocol used.
+   *
+   * <p>Examples: `3.1.1`.
+   */
+  @Nullable
+  default String getProtocolVersion(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
+
   @Nullable
   String getPeerName(REQUEST request);
 

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java
@@ -20,6 +20,26 @@ public interface NetServerAttributesGetter<REQUEST> {
   @Nullable
   String getTransport(REQUEST request);
 
+  /**
+   * Returns the application protocol used.
+   *
+   * <p>Examples: `amqp`, `http`, `mqtt`.
+   */
+  @Nullable
+  default String getProtocolName(REQUEST request) {
+    return null;
+  }
+
+  /**
+   * Returns the version of the application protocol used.
+   *
+   * <p>Examples: `3.1.1`.
+   */
+  @Nullable
+  default String getProtocolVersion(REQUEST request) {
+    return null;
+  }
+
   @Nullable
   String getHostName(REQUEST request);
 

diff --git a/...y/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java b/...y/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java
@@ -49,6 +49,14 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO
 
     internalSet(
         attributes, SemanticAttributes.NET_TRANSPORT, getter.getTransport(request, response));
+    internalSet(
+        attributes,
+        SemanticAttributes.NET_APP_PROTOCOL_NAME,
+        getter.getProtocolName(request, response));
+    internalSet(
+        attributes,
+        SemanticAttributes.NET_APP_PROTOCOL_VERSION,
+        getter.getProtocolVersion(request, response));
 
     String peerName = extractPeerName(request);
 

diff --git a/...y/instrumentation/api/instrumenter/net/internal/InternalNetServerAttributesExtractor.java b/...y/instrumentation/api/instrumenter/net/internal/InternalNetServerAttributesExtractor.java
@@ -32,7 +32,14 @@ public InternalNetServerAttributesExtractor(
   }
 
   public void onStart(AttributesBuilder attributes, REQUEST request) {
+
     internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.getTransport(request));
+    internalSet(
+        attributes, SemanticAttributes.NET_APP_PROTOCOL_NAME, getter.getProtocolName(request));
+    internalSet(
+        attributes,
+        SemanticAttributes.NET_APP_PROTOCOL_VERSION,
+        getter.getProtocolVersion(request));
 
     boolean setSockFamily = false;
 

diff --git a/...elemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractorTest.java b/...elemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractorTest.java
@@ -43,8 +43,6 @@ void shouldExtractAllAvailableAttributes(
     if (temporary) {
       request.put("temporaryDestination", "y");
     }
-    request.put("protocol", "AMQP");
-    request.put("protocolVersion", "1.0.0");
     request.put("url", "http://broker/topic");
     request.put("conversationId", "42");
     request.put("payloadSize", "100");
@@ -70,8 +68,6 @@ void shouldExtractAllAvailableAttributes(
     if (temporary) {
       expectedEntries.add(entry(SemanticAttributes.MESSAGING_DESTINATION_TEMPORARY, true));
     }
-    expectedEntries.add(entry(SemanticAttributes.NET_APP_PROTOCOL_NAME, "AMQP"));
-    expectedEntries.add(entry(SemanticAttributes.NET_APP_PROTOCOL_VERSION, "1.0.0"));
     expectedEntries.add(entry(SemanticAttributes.MESSAGING_MESSAGE_CONVERSATION_ID, "42"));
     expectedEntries.add(entry(SemanticAttributes.MESSAGING_MESSAGE_PAYLOAD_SIZE_BYTES, 100L));
     expectedEntries.add(
@@ -137,16 +133,6 @@ public boolean isTemporaryDestination(Map<String, String> request) {
       return request.containsKey("temporaryDestination");
     }
 
-    @Override
-    public String getProtocol(Map<String, String> request) {
-      return request.get("protocol");
-    }
-
-    @Override
-    public String getProtocolVersion(Map<String, String> request) {
-      return request.get("protocolVersion");
-    }
-
     @Override
     public String getConversationId(Map<String, String> request) {
       return request.get("conversationId");

diff --git a/.../opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractorTest.java b/.../opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractorTest.java
@@ -17,6 +17,7 @@
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.HashMap;
 import java.util.Map;
+import javax.annotation.Nullable;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
@@ -30,6 +31,20 @@ public String getTransport(Map<String, String> request, Map<String, String> resp
       return response.get("transport");
     }
 
+    @Nullable
+    @Override
+    public String getProtocolName(
+        Map<String, String> request, @Nullable Map<String, String> response) {
+      return request.get("protocolName");
+    }
+
+    @Nullable
+    @Override
+    public String getProtocolVersion(
+        Map<String, String> request, @Nullable Map<String, String> response) {
+      return request.get("protocolVersion");
+    }
+
     @Override
     public String getPeerName(Map<String, String> request) {
       return request.get("peerName");
@@ -71,6 +86,8 @@ void normal() {
     // given
     Map<String, String> map = new HashMap<>();
     map.put("transport", IP_TCP);
+    map.put("protocolName", "http");
+    map.put("protocolVersion", "1.1");
     map.put("peerName", "opentelemetry.io");
     map.put("peerPort", "42");
     map.put("sockFamily", "inet6");
@@ -96,6 +113,8 @@ void normal() {
     assertThat(endAttributes.build())
         .containsOnly(
             entry(SemanticAttributes.NET_TRANSPORT, IP_TCP),
+            entry(SemanticAttributes.NET_APP_PROTOCOL_NAME, "http"),
+            entry(SemanticAttributes.NET_APP_PROTOCOL_VERSION, "1.1"),
             entry(SemanticAttributes.NET_SOCK_FAMILY, "inet6"),
             entry(SemanticAttributes.NET_SOCK_PEER_ADDR, "1:2:3:4::"),
             entry(SemanticAttributes.NET_SOCK_PEER_NAME, "proxy.opentelemetry.io"),

diff --git a/.../opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractorTest.java b/.../opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractorTest.java
@@ -31,6 +31,18 @@ public String getTransport(Map<String, String> request) {
       return request.get("transport");
     }
 
+    @Nullable
+    @Override
+    public String getProtocolName(Map<String, String> request) {
+      return request.get("protocolName");
+    }
+
+    @Nullable
+    @Override
+    public String getProtocolVersion(Map<String, String> request) {
+      return request.get("protocolVersion");
+    }
+
     @Nullable
     @Override
     public String getHostName(Map<String, String> request) {
@@ -83,6 +95,8 @@ void normal() {
     // given
     Map<String, String> map = new HashMap<>();
     map.put("transport", IP_TCP);
+    map.put("protocolName", "http");
+    map.put("protocolVersion", "1.1");
     map.put("hostName", "opentelemetry.io");
     map.put("hostPort", "80");
     map.put("sockFamily", "inet6");
@@ -104,6 +118,8 @@ void normal() {
     assertThat(startAttributes.build())
         .containsOnly(
             entry(SemanticAttributes.NET_TRANSPORT, IP_TCP),
+            entry(SemanticAttributes.NET_APP_PROTOCOL_NAME, "http"),
+            entry(SemanticAttributes.NET_APP_PROTOCOL_VERSION, "1.1"),
             entry(SemanticAttributes.NET_HOST_NAME, "opentelemetry.io"),
             entry(SemanticAttributes.NET_HOST_PORT, 80L),
             entry(SemanticAttributes.NET_SOCK_FAMILY, "inet6"),

diff --git a/...etry/javaagent/instrumentation/pulsar/v2_8/telemetry/PulsarMessagingAttributesGetter.java b/...etry/javaagent/instrumentation/pulsar/v2_8/telemetry/PulsarMessagingAttributesGetter.java
@@ -35,18 +35,6 @@ public boolean isTemporaryDestination(Message<?> message) {
     return false;
   }
 
-  @Nullable
-  @Override
-  public String getProtocol(Message<?> message) {
-    return null;
-  }
-
-  @Nullable
-  @Override
-  public String getProtocolVersion(Message<?> message) {
-    return null;
-  }
-
   @Nullable
   @Override
   public String getConversationId(Message<?> message) {

diff --git a/.../main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageAttributesGetter.java b/.../main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageAttributesGetter.java
@@ -40,18 +40,6 @@ public boolean isTemporaryDestination(MessageWithDestination messageWithDestinat
     return messageWithDestination.isTemporaryDestination();
   }
 
-  @Nullable
-  @Override
-  public String getProtocol(MessageWithDestination messageWithDestination) {
-    return null;
-  }
-
-  @Nullable
-  @Override
-  public String getProtocolVersion(MessageWithDestination messageWithDestination) {
-    return null;
-  }
-
   @Nullable
   @Override
   public String getConversationId(MessageWithDestination messageWithDestination) {

diff --git a/...n/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerAttributesGetter.java b/...n/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerAttributesGetter.java
@@ -36,18 +36,6 @@ public boolean isTemporaryDestination(KafkaProcessRequest request) {
     return false;
   }
 
-  @Override
-  @Nullable
-  public String getProtocol(KafkaProcessRequest request) {
-    return null;
-  }
-
-  @Override
-  @Nullable
-  public String getProtocolVersion(KafkaProcessRequest request) {
-    return null;
-  }
-
   @Override
   @Nullable
   public String getConversationId(KafkaProcessRequest request) {

diff --git a/...n/java/io/opentelemetry/instrumentation/kafka/internal/KafkaProducerAttributesGetter.java b/...n/java/io/opentelemetry/instrumentation/kafka/internal/KafkaProducerAttributesGetter.java
@@ -42,18 +42,6 @@ public boolean isTemporaryDestination(KafkaProducerRequest request) {
     return false;
   }
 
-  @Override
-  @Nullable
-  public String getProtocol(KafkaProducerRequest request) {
-    return null;
-  }
-
-  @Override
-  @Nullable
-  public String getProtocolVersion(KafkaProducerRequest request) {
-    return null;
-  }
-
   @Override
   @Nullable
   public String getConversationId(KafkaProducerRequest request) {

diff --git a/...in/java/io/opentelemetry/instrumentation/kafka/internal/KafkaReceiveAttributesGetter.java b/...in/java/io/opentelemetry/instrumentation/kafka/internal/KafkaReceiveAttributesGetter.java
@@ -44,18 +44,6 @@ public boolean isTemporaryDestination(KafkaReceiveRequest request) {
     return false;
   }
 
-  @Override
-  @Nullable
-  public String getProtocol(KafkaReceiveRequest request) {
-    return null;
-  }
-
-  @Override
-  @Nullable
-  public String getProtocolVersion(KafkaReceiveRequest request) {
-    return null;
-  }
-
   @Override
   @Nullable
   public String getConversationId(KafkaReceiveRequest request) {

diff --git a/...va/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelAttributesGetter.java b/...va/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelAttributesGetter.java
@@ -35,18 +35,6 @@ public boolean isTemporaryDestination(ChannelAndMethod channelAndMethod) {
     return false;
   }
 
-  @Nullable
-  @Override
-  public String getProtocol(ChannelAndMethod channelAndMethod) {
-    return null;
-  }
-
-  @Nullable
-  @Override
-  public String getProtocolVersion(ChannelAndMethod channelAndMethod) {
-    return null;
-  }
-
   @Nullable
   @Override
   public String getConversationId(ChannelAndMethod channelAndMethod) {

diff --git a/...a/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitDeliveryAttributesGetter.java b/...a/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitDeliveryAttributesGetter.java
@@ -43,18 +43,6 @@ public boolean isTemporaryDestination(DeliveryRequest request) {
     return false;
   }
 
-  @Nullable
-  @Override
-  public String getProtocol(DeliveryRequest request) {
-    return null;
-  }
-
-  @Nullable
-  @Override
-  public String getProtocolVersion(DeliveryRequest request) {
-    return null;
-  }
-
   @Nullable
   @Override
   public String getConversationId(DeliveryRequest request) {