Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change user_agent.original from recommended to opt-in on HTTP client spans #9776

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Change user_agent.original from recommended to opt-in on HTTP client spans #9776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
862e4fd
Change user_agent.original from recommended to opt-in on HTTP client …
Oct 30, 2023
e874cd9
fix tests
Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions ...io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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")));
Expand Down
6 changes: 6 additions & 0 deletions ...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"));
}
}
1 change: 0 additions & 1 deletion ...pentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
1 change: 0 additions & 1 deletion ...y/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorBothSemconvTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
1 change: 0 additions & 1 deletion ...instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
20 changes: 0 additions & 20 deletions ...ntelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -132,11 +126,6 @@ public void sendRequestWithCallback(
request,
responseCallback(httpClientResult));
}

@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
configureTest(optionsBuilder);
}
}

@Nested
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 1 addition & 9 deletions ...lemetry/javaagent/instrumentation/apachehttpclient/v4_0/AbstractApacheHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
laurit marked this conversation as resolved.
Show resolved Hide resolved
headers.forEach(request::setHeader);
return request;
}
Expand Down
35 changes: 0 additions & 35 deletions .../io/opentelemetry/instrumentation/apachehttpclient/v4_3/AbstractApacheHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -90,11 +89,6 @@ public void sendRequestWithCallback(
httpClientResult.complete(t);
}
}

@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
configureTest(optionsBuilder);
}
}

@Nested
Expand Down Expand Up @@ -136,11 +130,6 @@ public void sendRequestWithCallback(
httpClientResult.complete(t);
}
}

@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
configureTest(optionsBuilder);
}
}

@Nested
Expand Down Expand Up @@ -177,11 +166,6 @@ public void sendRequestWithCallback(
httpClientResult.complete(t);
}
}

@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
configureTest(optionsBuilder);
}
}

@Nested
Expand Down Expand Up @@ -223,11 +207,6 @@ public void sendRequestWithCallback(
httpClientResult.complete(t);
}
}

@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
configureTest(optionsBuilder);
}
}

@Nested
Expand Down Expand Up @@ -259,11 +238,6 @@ public void sendRequestWithCallback(
httpClientResult.complete(t);
}
}

@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
configureTest(optionsBuilder);
}
}

@Nested
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions ...lemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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;
}
Expand Down
39 changes: 0 additions & 39 deletions ...nt/instrumentation/asynchttpclient/v2_0/AsyncHttpClientAdditionalAttributesExtractor.java
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...entelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientSingletons.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
1 change: 0 additions & 1 deletion .../io/opentelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,5 @@ public void onThrowable(Throwable throwable) {
@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
optionsBuilder.disableTestRedirects();
optionsBuilder.setUserAgent("AHC");
}
}
2 changes: 0 additions & 2 deletions instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/S3TracingTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/groovy/SnsTracingTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions ...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
11 changes: 0 additions & 11 deletions ...c/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
Loading
Loading