Switch from http.flavor to net.protocol.* in HTTP client instrumentat… (

#8131)
open-telemetry · Apr 7, 2023 · f501569 · f501569
1 parent e79c46e
commit f501569
Show file tree

Hide file tree

Showing 114 changed files with 838 additions and 692 deletions.
diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java
@@ -156,8 +156,6 @@ public void onEnd(
       @Nullable Throwable error) {
     super.onEnd(attributes, context, request, response, error);
 
-    internalSet(attributes, SemanticAttributes.HTTP_FLAVOR, getter.getFlavor(request, response));
-
     internalNetExtractor.onEnd(attributes, request, response);
 
     int resendCount = resendCountIncrementer.applyAsInt(context);

diff --git a/...va/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesGetter.java b/...va/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesGetter.java
@@ -7,6 +7,7 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import javax.annotation.Nullable;
 
 /**
@@ -31,7 +32,13 @@ public interface HttpClientAttributesGetter<REQUEST, RESPONSE>
    *
    * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
    * {@code response} is {@code null} or not.
+   *
+   * @deprecated Use {@link NetClientAttributesGetter#getProtocolName(Object, Object)} and {@link
+   *     NetClientAttributesGetter#getProtocolVersion(Object, Object)} instead.
    */
+  @Deprecated
   @Nullable
-  String getFlavor(REQUEST request, @Nullable RESPONSE response);
+  default String getFlavor(REQUEST request, @Nullable RESPONSE response) {
+    return null;
+  }
 }
diff --git a/...ain/java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsView.java b/...ain/java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsView.java
@@ -8,6 +8,7 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.instrumentation.api.instrumenter.net.internal.NetAttributes;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.HashSet;
 import java.util.Set;
@@ -30,6 +31,8 @@ private static Set<AttributeKey> buildDurationAlwaysInclude() {
     view.add(SemanticAttributes.HTTP_METHOD);
     view.add(SemanticAttributes.HTTP_STATUS_CODE); // Optional
     view.add(SemanticAttributes.HTTP_FLAVOR); // Optional
+    view.add(NetAttributes.NET_PROTOCOL_NAME); // Optional
+    view.add(NetAttributes.NET_PROTOCOL_VERSION); // Optional
     return view;
   }
 

diff --git a/...o/opentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesGetter.java b/...o/opentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesGetter.java
@@ -7,7 +7,6 @@
 
 import static java.util.Collections.emptyList;
 
-import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import java.util.List;
 import javax.annotation.Nullable;
 
@@ -31,39 +30,6 @@ public interface MessagingAttributesGetter<REQUEST, RESPONSE> {
 
   boolean isTemporaryDestination(REQUEST request);
 
-  /**
-   * Returns the application protocol used.
-   *
-   * @deprecated Use {@link NetClientAttributesGetter#getProtocolName(Object, Object)} instead.
-   */
-  @Deprecated
-  @Nullable
-  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
-  default String getProtocolVersion(REQUEST request) {
-    return null;
-  }
-
-  /**
-   * Returns the application protocol used.
-   *
-   * @deprecated The {@code messaging.url} attribute was removed without replacement.
-   */
-  @Deprecated
-  @Nullable
-  default String getUrl(REQUEST request) {
-    return null;
-  }
-
   @Nullable
   String getConversationId(REQUEST request);
 

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesGetter.java
@@ -20,7 +20,6 @@ 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.
    *
@@ -31,7 +30,6 @@ 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.
    *

diff --git a/...y/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java b/...y/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java
@@ -10,6 +10,7 @@
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import java.util.Locale;
 import java.util.function.BiPredicate;
 import javax.annotation.Nullable;
 
@@ -49,13 +50,14 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO
 
     internalSet(
         attributes, SemanticAttributes.NET_TRANSPORT, getter.getTransport(request, response));
+    String protocolName = getter.getProtocolName(request, response);
+    if (protocolName != null) {
+      internalSet(
+          attributes, NetAttributes.NET_PROTOCOL_NAME, protocolName.toLowerCase(Locale.ROOT));
+    }
     internalSet(
         attributes,
-        SemanticAttributes.NET_APP_PROTOCOL_NAME,
-        getter.getProtocolName(request, response));
-    internalSet(
-        attributes,
-        SemanticAttributes.NET_APP_PROTOCOL_VERSION,
+        NetAttributes.NET_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
@@ -34,12 +34,8 @@ 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));
+    internalSet(attributes, NetAttributes.NET_PROTOCOL_NAME, getter.getProtocolName(request));
+    internalSet(attributes, NetAttributes.NET_PROTOCOL_VERSION, getter.getProtocolVersion(request));
 
     boolean setSockFamily = false;
 

diff --git a/...in/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/NetAttributes.java b/...in/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/NetAttributes.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.net.internal;
+
+import static io.opentelemetry.api.common.AttributeKey.stringKey;
+
+import io.opentelemetry.api.common.AttributeKey;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public final class NetAttributes {
+
+  // FIXME: remove this class and replace its usages with SemanticAttributes once schema 1.20 is
+  // released
+
+  public static final AttributeKey<String> NET_PROTOCOL_NAME = stringKey("net.protocol.name");
+
+  public static final AttributeKey<String> NET_PROTOCOL_VERSION = stringKey("net.protocol.version");
+
+  private NetAttributes() {}
+}
diff --git a/...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java
@@ -19,6 +19,7 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
+import io.opentelemetry.instrumentation.api.instrumenter.net.internal.NetAttributes;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.HashMap;
 import java.util.List;
@@ -60,11 +61,6 @@ public Integer getStatusCode(
       return Integer.parseInt(response.get("statusCode"));
     }
 
-    @Override
-    public String getFlavor(Map<String, String> request, Map<String, String> response) {
-      return request.get("flavor");
-    }
-
     @Override
     public List<String> getResponseHeader(
         Map<String, String> request, Map<String, String> response, String name) {
@@ -83,6 +79,20 @@ public String getTransport(
       return response == null ? null : 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");
+    }
+
     @Nullable
     @Override
     public String getPeerName(Map<String, String> request) {
@@ -103,9 +113,10 @@ void normal() {
     request.put("method", "POST");
     request.put("url", "http://github.com");
     request.put("header.content-length", "10");
-    request.put("flavor", "http/2");
     request.put("header.user-agent", "okhttp 3.x");
     request.put("header.custom-request-header", "123,456");
+    request.put("protocolName", "http");
+    request.put("protocolVersion", "1.1");
     request.put("peerName", "github.com");
     request.put("peerPort", "123");
 
@@ -143,14 +154,15 @@ void normal() {
     assertThat(endAttributes.build())
         .containsOnly(
             entry(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH, 10L),
-            entry(SemanticAttributes.HTTP_FLAVOR, "http/2"),
             entry(SemanticAttributes.HTTP_STATUS_CODE, 202L),
             entry(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH, 20L),
             entry(SemanticAttributes.HTTP_RESEND_COUNT, 2L),
             entry(
                 AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
                 asList("654", "321")),
-            entry(SemanticAttributes.NET_TRANSPORT, IP_TCP));
+            entry(SemanticAttributes.NET_TRANSPORT, IP_TCP),
+            entry(NetAttributes.NET_PROTOCOL_NAME, "http"),
+            entry(NetAttributes.NET_PROTOCOL_VERSION, "1.1"));
   }
 
   @ParameterizedTest

diff --git a/.../opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractorTest.java b/.../opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractorTest.java
@@ -14,6 +14,7 @@
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.net.internal.NetAttributes;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.HashMap;
 import java.util.Map;
@@ -113,8 +114,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(NetAttributes.NET_PROTOCOL_NAME, "http"),
+            entry(NetAttributes.NET_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
@@ -14,6 +14,7 @@
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.net.internal.NetAttributes;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.HashMap;
 import java.util.Map;
@@ -118,8 +119,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(NetAttributes.NET_PROTOCOL_NAME, "http"),
+            entry(NetAttributes.NET_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/...src/jmh/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBenchmark.java b/...src/jmh/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBenchmark.java
@@ -78,11 +78,6 @@ public List<String> getRequestHeader(Void unused, String name) {
       return Collections.emptyList();
     }
 
-    @Override
-    public String getFlavor(Void unused, @Nullable Void unused2) {
-      return SemanticAttributes.HttpFlavorValues.HTTP_2_0;
-    }
-
     @Override
     public Integer getStatusCode(Void unused, Void unused2, @Nullable Throwable error) {
       return 200;
@@ -106,6 +101,18 @@ public String getTransport(Void request, @Nullable Void response) {
       return SemanticAttributes.NetTransportValues.IP_TCP;
     }
 
+    @Nullable
+    @Override
+    public String getProtocolName(Void unused, @Nullable Void unused2) {
+      return "http";
+    }
+
+    @Nullable
+    @Override
+    public String getProtocolVersion(Void unused, @Nullable Void unused2) {
+      return "2.0";
+    }
+
     @Nullable
     @Override
     public String getPeerName(Void request) {

diff --git a/...agent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpUtil.java b/...agent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpUtil.java
@@ -7,7 +7,6 @@
 
 import akka.http.scaladsl.model.HttpRequest;
 import akka.http.scaladsl.model.HttpResponse;
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.Collections;
 import java.util.List;
 
@@ -33,16 +32,12 @@ public static List<String> responseHeader(HttpResponse httpResponse, String name
         .orElse(Collections.emptyList());
   }
 
-  public static String flavor(HttpRequest httpRequest) {
-    switch (httpRequest.protocol().value()) {
-      case "HTTP/1.0":
-        return SemanticAttributes.HttpFlavorValues.HTTP_1_0;
-      case "HTTP/2.0":
-        return SemanticAttributes.HttpFlavorValues.HTTP_2_0;
-      case "HTTP/1.1":
-      default:
-        return SemanticAttributes.HttpFlavorValues.HTTP_1_1;
+  public static String httpVersion(HttpRequest httpRequest) {
+    String protocol = httpRequest.protocol().value();
+    if (protocol.startsWith("HTTP/")) {
+      protocol = protocol.substring("HTTP/".length());
     }
+    return protocol;
   }
 
   private AkkaHttpUtil() {}

diff --git a/...entelemetry/javaagent/instrumentation/akkahttp/client/AkkaHttpClientAttributesGetter.java b/...entelemetry/javaagent/instrumentation/akkahttp/client/AkkaHttpClientAttributesGetter.java
@@ -20,11 +20,6 @@ public String getUrl(HttpRequest httpRequest) {
     return httpRequest.uri().toString();
   }
 
-  @Override
-  public String getFlavor(HttpRequest httpRequest, @Nullable HttpResponse httpResponse) {
-    return AkkaHttpUtil.flavor(httpRequest);
-  }
-
   @Override
   public String getMethod(HttpRequest httpRequest) {
     return httpRequest.method().value();

diff --git a/.../opentelemetry/javaagent/instrumentation/akkahttp/client/AkkaHttpNetAttributesGetter.java b/.../opentelemetry/javaagent/instrumentation/akkahttp/client/AkkaHttpNetAttributesGetter.java
@@ -8,6 +8,7 @@
 import akka.http.scaladsl.model.HttpRequest;
 import akka.http.scaladsl.model.HttpResponse;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
+import io.opentelemetry.javaagent.instrumentation.akkahttp.AkkaHttpUtil;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import javax.annotation.Nullable;
 
@@ -18,6 +19,18 @@ public String getTransport(HttpRequest httpRequest, @Nullable HttpResponse httpR
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
+  @Nullable
+  @Override
+  public String getProtocolName(HttpRequest httpRequest, @Nullable HttpResponse httpResponse) {
+    return "http";
+  }
+
+  @Nullable
+  @Override
+  public String getProtocolVersion(HttpRequest httpRequest, @Nullable HttpResponse httpResponse) {
+    return AkkaHttpUtil.httpVersion(httpRequest);
+  }
+
   @Override
   public String getPeerName(HttpRequest httpRequest) {
     return httpRequest.uri().authority().host().address();