Change user_agent.original from recommended to opt-in on HTTP clien…

…t spans (open-telemetry#9776)
Abhishekkr3003 · Nov 7, 2023 · 4f0cf16 · 4f0cf16
1 parent a6c74e3
commit 4f0cf16
Show file tree

Hide file tree

Showing 42 changed files with 20 additions and 277 deletions.
diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java
@@ -67,7 +67,6 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
     if (SemconvStability.emitOldHttpSemconv()) {
       internalSet(attributes, SemanticAttributes.HTTP_METHOD, method);
     }
-    internalSet(attributes, SemanticAttributes.USER_AGENT_ORIGINAL, userAgent(request));
 
     for (String name : capturedRequestHeaders) {
       List<String> values = getter.getHttpRequestHeader(request, name);
@@ -152,11 +151,6 @@ public void onEnd(
     }
   }
 
-  @Nullable
-  private String userAgent(REQUEST request) {
-    return firstHeaderValue(getter.getHttpRequestHeader(request, "user-agent"));
-  }
-
   @Nullable
   private Long requestBodySize(REQUEST request) {
     return parseNumber(firstHeaderValue(getter.getHttpRequestHeader(request, "content-length")));

diff --git a/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
@@ -115,6 +115,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
     internalClientExtractor.onStart(attributes, request);
 
     internalSet(attributes, SemanticAttributes.HTTP_ROUTE, getter.getHttpRoute(request));
+    internalSet(attributes, SemanticAttributes.USER_AGENT_ORIGINAL, userAgent(request));
   }
 
   @Override
@@ -140,4 +141,9 @@ public void onEnd(
   public SpanKey internalGetSpanKey() {
     return SpanKey.HTTP_SERVER;
   }
+
+  @Nullable
+  private String userAgent(REQUEST request) {
+    return firstHeaderValue(getter.getHttpRequestHeader(request, "user-agent"));
+  }
 }
diff --git a/...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java
@@ -144,7 +144,6 @@ void normal() {
         .containsOnly(
             entry(SemanticAttributes.HTTP_METHOD, "POST"),
             entry(SemanticAttributes.HTTP_URL, "http://github.com"),
-            entry(SemanticAttributes.USER_AGENT_ORIGINAL, "okhttp 3.x"),
             entry(
                 AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
                 asList("123", "456")),

diff --git a/...y/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorBothSemconvTest.java b/...y/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorBothSemconvTest.java
@@ -139,7 +139,6 @@ void normal() {
             entry(SemanticAttributes.HTTP_REQUEST_METHOD, "POST"),
             entry(SemanticAttributes.HTTP_URL, "http://github.com"),
             entry(SemanticAttributes.URL_FULL, "http://github.com"),
-            entry(SemanticAttributes.USER_AGENT_ORIGINAL, "okhttp 3.x"),
             entry(
                 AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
                 asList("123", "456")),

diff --git a/...instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java b/...instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java
@@ -171,7 +171,6 @@ void normal() {
         .containsOnly(
             entry(SemanticAttributes.HTTP_REQUEST_METHOD, "POST"),
             entry(SemanticAttributes.URL_FULL, "http://github.com"),
-            entry(SemanticAttributes.USER_AGENT_ORIGINAL, "okhttp 3.x"),
             entry(
                 AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
                 asList("123", "456")),

diff --git a/...ntelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientTest.java b/...ntelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientTest.java
@@ -9,7 +9,6 @@
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
-import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.net.URI;
@@ -93,11 +92,6 @@ public void sendRequestWithCallback(
         HttpClientResult httpClientResult) {
       getClient(uri).execute(request, responseCallback(httpClientResult));
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -132,11 +126,6 @@ public void sendRequestWithCallback(
               request,
               responseCallback(httpClientResult));
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -170,11 +159,6 @@ public void sendRequestWithCallback(
               request,
               responseCallback(httpClientResult));
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   HttpUriRequest configureRequest(HttpUriRequest request, Map<String, String> headers) {
@@ -220,10 +204,6 @@ public void cancelled() {
     };
   }
 
-  void configureTest(HttpClientTestOptions.Builder optionsBuilder) {
-    optionsBuilder.setUserAgent("httpasyncclient");
-  }
-
   static String fullPathFromUri(URI uri) {
     StringBuilder builder = new StringBuilder();
     if (uri.getPath() != null) {

diff --git a/...lemetry/javaagent/instrumentation/apachehttpclient/v4_0/AbstractApacheHttpClientTest.java b/...lemetry/javaagent/instrumentation/apachehttpclient/v4_0/AbstractApacheHttpClientTest.java
@@ -7,7 +7,6 @@
 
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
-import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
 import java.net.URI;
 import java.util.Map;
 import org.apache.http.HttpHost;
@@ -19,17 +18,10 @@
 abstract class AbstractApacheHttpClientTest<T extends HttpRequest>
     extends AbstractHttpClientTest<T> {
 
-  private static final String USER_AGENT = "apachehttpclient";
-
-  @Override
-  protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-    optionsBuilder.setUserAgent(USER_AGENT);
-  }
-
   @Override
   public T buildRequest(String method, URI uri, Map<String, String> headers) {
     T request = createRequest(method, uri);
-    request.addHeader("user-agent", USER_AGENT);
+    request.addHeader("user-agent", "apachehttpclient");
     headers.forEach(request::setHeader);
     return request;
   }

diff --git a/.../io/opentelemetry/instrumentation/apachehttpclient/v4_3/AbstractApacheHttpClientTest.java b/.../io/opentelemetry/instrumentation/apachehttpclient/v4_3/AbstractApacheHttpClientTest.java
@@ -8,7 +8,6 @@
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
-import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.net.URI;
@@ -90,11 +89,6 @@ public void sendRequestWithCallback(
         httpClientResult.complete(t);
       }
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -136,11 +130,6 @@ public void sendRequestWithCallback(
         httpClientResult.complete(t);
       }
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -177,11 +166,6 @@ public void sendRequestWithCallback(
         httpClientResult.complete(t);
       }
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -223,11 +207,6 @@ public void sendRequestWithCallback(
         httpClientResult.complete(t);
       }
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -259,11 +238,6 @@ public void sendRequestWithCallback(
         httpClientResult.complete(t);
       }
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   @Nested
@@ -295,11 +269,6 @@ public void sendRequestWithCallback(
         httpClientResult.complete(t);
       }
     }
-
-    @Override
-    protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-      configureTest(optionsBuilder);
-    }
   }
 
   static <T extends HttpRequest> T configureRequest(T request, Map<String, String> headers) {
@@ -331,10 +300,6 @@ static ResponseHandler<HttpResponse> responseCallback(HttpClientResult httpClien
     };
   }
 
-  static void configureTest(HttpClientTestOptions.Builder optionsBuilder) {
-    optionsBuilder.setUserAgent("apachehttpclient");
-  }
-
   static String fullPathFromUri(URI uri) {
     StringBuilder builder = new StringBuilder();
     if (uri.getPath() != null) {

diff --git a/...lemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java b/...lemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java
@@ -27,11 +27,9 @@
 
 abstract class AbstractApacheHttpClientTest<T extends HttpRequest>
     extends AbstractHttpClientTest<T> {
-  private static final String USER_AGENT = "apachehttpclient";
 
   @Override
   protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
-    optionsBuilder.setUserAgent(USER_AGENT);
     optionsBuilder.setHttpAttributes(this::getHttpAttributes);
   }
 
@@ -55,7 +53,7 @@ protected Set<AttributeKey<?>> getHttpAttributes(URI uri) {
   @Override
   public T buildRequest(String method, URI uri, Map<String, String> headers) {
     T request = createRequest(method, uri);
-    request.addHeader("user-agent", USER_AGENT);
+    request.addHeader("user-agent", "apachehttpclient");
     headers.forEach((key, value) -> request.setHeader(new BasicHeader(key, value)));
     return request;
   }

diff --git a/...nt/instrumentation/asynchttpclient/v2_0/AsyncHttpClientAdditionalAttributesExtractor.java b/...nt/instrumentation/asynchttpclient/v2_0/AsyncHttpClientAdditionalAttributesExtractor.java
diff --git a/...entelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientSingletons.java b/...entelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientSingletons.java
@@ -43,7 +43,6 @@ public final class AsyncHttpClientSingletons {
             .addAttributesExtractor(
                 HttpClientPeerServiceAttributesExtractor.create(
                     httpAttributesGetter, CommonConfig.get().getPeerServiceResolver()))
-            .addAttributesExtractor(new AsyncHttpClientAdditionalAttributesExtractor())
             .addOperationMetrics(HttpClientMetrics.get());
     if (CommonConfig.get().shouldEmitExperimentalHttpClientMetrics()) {
       builder.addOperationMetrics(HttpClientExperimentalMetrics.get());

diff --git a/.../io/opentelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientTest.java b/.../io/opentelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientTest.java
@@ -78,6 +78,5 @@ public void onThrowable(Throwable throwable) {
   @Override
   protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
     optionsBuilder.disableTestRedirects();
-    optionsBuilder.setUserAgent("AHC");
   }
 }
diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/S3TracingTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/S3TracingTest.groovy
@@ -205,7 +205,6 @@ class S3TracingTest extends AgentInstrumentationSpecification {
             "http.method" "POST"
             "http.status_code" 200
             "http.url" String
-            "$SemanticAttributes.USER_AGENT_ORIGINAL" String
             "net.peer.name" String
             "$SemanticAttributes.NET_PROTOCOL_NAME" "http"
             "$SemanticAttributes.NET_PROTOCOL_VERSION" "1.1"
@@ -572,7 +571,6 @@ class S3TracingTest extends AgentInstrumentationSpecification {
             "http.method" "POST"
             "http.status_code" 200
             "http.url" String
-            "$SemanticAttributes.USER_AGENT_ORIGINAL" String
             "net.peer.name" String
             "$SemanticAttributes.NET_PROTOCOL_NAME" "http"
             "$SemanticAttributes.NET_PROTOCOL_VERSION" "1.1"

diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/SnsTracingTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/SnsTracingTest.groovy
@@ -189,7 +189,6 @@ class SnsTracingTest extends AgentInstrumentationSpecification {
             "http.method" "POST"
             "http.status_code" 200
             "http.url" String
-            "$SemanticAttributes.USER_AGENT_ORIGINAL" String
             "net.peer.name" String
             "$SemanticAttributes.NET_PROTOCOL_NAME" "http"
             "$SemanticAttributes.NET_PROTOCOL_VERSION" "1.1"

diff --git a/...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy b/...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy
@@ -123,7 +123,6 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
             "http.method" "POST"
             "http.status_code" 200
             "http.url" "http://localhost:$sqsPort"
-            "$SemanticAttributes.USER_AGENT_ORIGINAL" String
             "net.peer.name" "localhost"
             "net.peer.port" sqsPort
             "$SemanticAttributes.MESSAGING_SYSTEM" "AmazonSQS"
@@ -213,7 +212,6 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
             "http.method" "POST"
             "http.status_code" 200
             "http.url" "http://localhost:$sqsPort"
-            "$SemanticAttributes.USER_AGENT_ORIGINAL" String
             "net.peer.name" "localhost"
             "net.peer.port" sqsPort
             "$SemanticAttributes.MESSAGING_SYSTEM" "AmazonSQS"

diff --git a/...c/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/...c/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java
@@ -23,7 +23,6 @@
 import java.io.InputStreamReader;
 import java.nio.charset.Charset;
 import java.time.Instant;
-import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
@@ -322,7 +321,6 @@ public void afterExecution(
       executionAttributes.putAttribute(SDK_HTTP_REQUEST_ATTRIBUTE, context.httpRequest());
 
       Span span = Span.fromContext(otelContext);
-      onUserAgentHeaderAvailable(span, executionAttributes);
       onSdkResponse(span, context.response(), executionAttributes);
 
       SdkHttpResponse httpResponse = context.httpResponse();
@@ -336,15 +334,6 @@ public void afterExecution(
     clearAttributes(executionAttributes);
   }
 
-  // Certain headers in the request like User-Agent are only available after execution.
-  private static void onUserAgentHeaderAvailable(Span span, ExecutionAttributes request) {
-    List<String> userAgent =
-        AwsSdkInstrumenterFactory.httpAttributesGetter.getHttpRequestHeader(request, "User-Agent");
-    if (!userAgent.isEmpty()) {
-      span.setAttribute(SemanticAttributes.USER_AGENT_ORIGINAL, userAgent.get(0));
-    }
-  }
-
   private void onSdkResponse(
       Span span, SdkResponse response, ExecutionAttributes executionAttributes) {
     if (captureExperimentalSpanAttributes) {