From beb1901f5f33e136cd7e9ef1505a69a0be624e5d Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Fri, 18 Sep 2020 17:52:08 +0300 Subject: [PATCH] Review semantic convention for Http Client spans (#1214) * Review semantic convention for Http Client spans * Polish * Update instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy Co-authored-by: Trask Stalnaker Co-authored-by: Trask Stalnaker --- docs/semantic-conventions.md | 29 +++++++++ .../instrumentation/api/config/Config.java | 42 ------------- .../api/tracer/HttpClientTracer.java | 61 +++++++++++-------- .../api/tracer/HttpClientTracerTest.groovy | 36 +++++------ .../auto/akkahttp/AkkaHttpClientTracer.java | 5 ++ .../ApacheHttpAsyncClientTracer.java | 6 ++ .../v4_0/ApacheHttpClientTracer.java | 6 ++ .../v1_0/client/ArmeriaClientTracer.java | 5 ++ .../armeria/v1_0/AbstractArmeriaTest.groovy | 1 + .../src/test/groovy/AWS1ClientTest.groovy | 12 ++-- .../groovy/AWS0ClientTest.groovy | 12 ++-- .../awssdk/v2_2/AbstractAws2ClientTest.groovy | 22 ++++--- .../Elasticsearch5RestClientTest.groovy | 1 + .../Elasticsearch6RestClientTest.groovy | 1 + .../Elasticsearch6RestClientTest.groovy | 1 + .../AbstractGoogleHttpClientTest.groovy | 1 + .../src/test/groovy/UrlConnectionTest.groovy | 3 +- .../auto/httpclient/JdkHttpClientTracer.java | 22 ++++++- .../src/test/groovy/JdkHttpClientTest.groovy | 51 +++++++++++++++- .../src/test/groovy/JaxRsClientTest.groovy | 1 + .../v3_8/client/NettyHttpClientTracer.java | 6 ++ .../v4_0/client/NettyHttpClientTracer.java | 6 ++ .../v4_1/client/NettyHttpClientTracer.java | 6 ++ .../src/test/groovy/ReactorNettyTest.groovy | 6 +- .../test/groovy/test/TwilioClientTest.groovy | 14 +++++ .../auto/test/InMemoryExporter.java | 6 +- .../auto/test/base/HttpClientTest.groovy | 43 +++++-------- 27 files changed, 257 insertions(+), 148 deletions(-) diff --git a/docs/semantic-conventions.md b/docs/semantic-conventions.md index 36537c029720..9f10ce84adf5 100644 --- a/docs/semantic-conventions.md +++ b/docs/semantic-conventions.md @@ -34,3 +34,32 @@ attribute. As either it or `http.url` is required, we set the latter. This, in t **[3]:** In case of Armeria, return values are [SessionProtocol](https://github.com/line/armeria/blob/master/core/src/main/java/com/linecorp/armeria/common/SessionProtocol.java), not values defined by spec. + +## Http Client + +| Attribute | Required | Implemented? | +|---|:---:|:---:| +| `http.method` | Y | + | +| `http.url` | N | + | +| `http.target` | N | - [1] | +| `http.host` | N | - [1] | +| `http.scheme` | N | - [1] | +| `http.status_code` | Y | + | +| `http.status_text` | N | - [2] | +| `http.flavor` | N | + [3] | +| `http.user_agent` | N | + | +| `http.request_content_length` | N | - | +| `http.request_content_length_uncompressed` | N | - | +| `http.response_content_length` | N | - | +| `http.response_content_length_uncompressed` | N | - | + +**[1]:** As the majority of Java frameworks don't provide a standard way to obtain "The full request +target as passed in a HTTP request line or equivalent.", we don't set `http.target` semantic +attribute. As either it or `http.url` is required, we set the latter. This, in turn, makes setting +`http.schema` and `http.host` unnecessary duplication. Therefore, we do not set them as well. + +**[2]: TODO** After [this PR](https://github.com/open-telemetry/opentelemetry-specification/issues/950) + is merged, remove this line. If it rejected, then implement this attribute. + +**[3]:** In case of Armeria, return values are [SessionProtocol](https://github.com/line/armeria/blob/master/core/src/main/java/com/linecorp/armeria/common/SessionProtocol.java), +not values defined by spec. diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java index d0e1a86aacfb..29fb72ec7673 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java @@ -27,7 +27,6 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.SortedSet; @@ -76,8 +75,6 @@ public class Config { public static final String TRACE_CLASSES_EXCLUDE = "trace.classes.exclude"; public static final String HTTP_SERVER_ERROR_STATUSES = "http.server.error.statuses"; public static final String HTTP_CLIENT_ERROR_STATUSES = "http.client.error.statuses"; - public static final String HTTP_SERVER_TAG_QUERY_STRING = "http.server.tag.query-string"; - public static final String HTTP_CLIENT_TAG_QUERY_STRING = "http.client.tag.query-string"; public static final String SCOPE_DEPTH_LIMIT = "trace.scope.depth.limit"; public static final String RUNTIME_CONTEXT_FIELD_INJECTION = "trace.runtime.context.field.injection"; @@ -93,13 +90,8 @@ public class Config { private static final boolean DEFAULT_RUNTIME_CONTEXT_FIELD_INJECTION = true; - private static final boolean DEFAULT_HTTP_SERVER_TAG_QUERY_STRING = false; - private static final boolean DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING = false; private static final int DEFAULT_SCOPE_DEPTH_LIMIT = 100; - public static final boolean DEFAULT_LOG_INJECTION_ENABLED = false; - public static final String DEFAULT_EXPERIMENTAL_LOG_CAPTURE_THRESHOLD = null; - public static final boolean DEFAULT_KAFKA_CLIENT_PROPAGATION_ENABLED = true; public static final boolean DEFAULT_HYSTRIX_TAGS_ENABLED = false; @@ -119,8 +111,6 @@ public class Config { private final boolean traceEnabled; private final boolean integrationsEnabled; private final List excludedClasses; - private final boolean httpServerTagQueryString; - private final boolean httpClientTagQueryString; private final Integer scopeDepthLimit; private final boolean runtimeContextFieldInjection; @@ -157,14 +147,6 @@ public class Config { excludedClasses = getListSettingFromEnvironment(TRACE_CLASSES_EXCLUDE, null); - httpServerTagQueryString = - getBooleanSettingFromEnvironment( - HTTP_SERVER_TAG_QUERY_STRING, DEFAULT_HTTP_SERVER_TAG_QUERY_STRING); - - httpClientTagQueryString = - getBooleanSettingFromEnvironment( - HTTP_CLIENT_TAG_QUERY_STRING, DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING); - scopeDepthLimit = getIntegerSettingFromEnvironment(SCOPE_DEPTH_LIMIT, DEFAULT_SCOPE_DEPTH_LIMIT); @@ -213,14 +195,6 @@ private Config(Properties properties, Config parent) { excludedClasses = getPropertyListValue(properties, TRACE_CLASSES_EXCLUDE, parent.excludedClasses); - httpServerTagQueryString = - getPropertyBooleanValue( - properties, HTTP_SERVER_TAG_QUERY_STRING, parent.httpServerTagQueryString); - - httpClientTagQueryString = - getPropertyBooleanValue( - properties, HTTP_CLIENT_TAG_QUERY_STRING, parent.httpClientTagQueryString); - scopeDepthLimit = getPropertyIntegerValue(properties, SCOPE_DEPTH_LIMIT, parent.scopeDepthLimit); @@ -492,10 +466,6 @@ private static Properties loadConfigurationFile() { return properties; } - private static String toUpper(String str) { - return str == null ? null : str.toUpperCase(Locale.ENGLISH); - } - // This has to be placed after all other static fields to give them a chance to initialize private static final Config INSTANCE = new Config(); @@ -535,14 +505,6 @@ public List getExcludedClasses() { return excludedClasses; } - public boolean isHttpServerTagQueryString() { - return httpServerTagQueryString; - } - - public boolean isHttpClientTagQueryString() { - return httpClientTagQueryString; - } - public Integer getScopeDepthLimit() { return scopeDepthLimit; } @@ -604,10 +566,6 @@ public String toString() { + integrationsEnabled + ", excludedClasses=" + excludedClasses - + ", httpServerTagQueryString=" - + httpServerTagQueryString - + ", httpClientTagQueryString=" - + httpClientTagQueryString + ", scopeDepthLimit=" + scopeDepthLimit + ", runtimeContextFieldInjection=" diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java index e6481ef2ea86..832cab155ec2 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java @@ -24,10 +24,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapPropagator.Setter; -import io.opentelemetry.instrumentation.api.MoreAttributes; -import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.decorator.HttpStatusConverter; -import io.opentelemetry.instrumentation.api.tracer.utils.HttpUrlUtils; import io.opentelemetry.instrumentation.api.tracer.utils.NetPeerUtils; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span; @@ -54,6 +51,12 @@ public abstract class HttpClientTracer extends BaseT @Nullable protected abstract URI url(REQUEST request) throws URISyntaxException; + @Nullable + protected String flavor(REQUEST request) { + // This is de facto standard nowadays, so let us use it, unless overridden + return "1.1"; + } + protected abstract Integer status(RESPONSE response); @Nullable @@ -138,32 +141,40 @@ private Span startSpan(REQUEST request, String name, long startTimeNanos) { protected Span onRequest(Span span, REQUEST request) { assert span != null; if (request != null) { - span.setAttribute(SemanticAttributes.HTTP_METHOD.key(), method(request)); - span.setAttribute(SemanticAttributes.NET_TRANSPORT.key(), "IP.TCP"); + SemanticAttributes.NET_TRANSPORT.set(span, "IP.TCP"); + SemanticAttributes.HTTP_METHOD.set(span, method(request)); + SemanticAttributes.HTTP_USER_AGENT.set(span, requestHeader(request, USER_AGENT)); - String userAgent = requestHeader(request, USER_AGENT); - if (userAgent != null) { - SemanticAttributes.HTTP_USER_AGENT.set(span, userAgent); - } + setFlavor(span, request); + setUrl(span, request); + } + return span; + } - try { - URI url = url(request); - if (url != null && url.getHost() != null) { - NetPeerUtils.setNetPeer(span, url.getHost(), null); - if (url.getPort() > 0) { - span.setAttribute(SemanticAttributes.NET_PEER_PORT.key(), url.getPort()); - } - } - HttpUrlUtils.setHttpUrl(span, url); - if (Config.get().isHttpClientTagQueryString()) { - span.setAttribute(MoreAttributes.HTTP_QUERY, url.getQuery()); - span.setAttribute(MoreAttributes.HTTP_FRAGMENT, url.getFragment()); - } - } catch (Exception e) { - log.debug("Error tagging url", e); + private void setFlavor(Span span, REQUEST request) { + String flavor = flavor(request); + if (flavor == null) { + return; + } + + String httpProtocolPrefix = "HTTP/"; + if (flavor.startsWith(httpProtocolPrefix)) { + flavor = flavor.substring(httpProtocolPrefix.length()); + } + + SemanticAttributes.HTTP_FLAVOR.set(span, flavor); + } + + private void setUrl(Span span, REQUEST request) { + try { + URI url = url(request); + if (url != null) { + NetPeerUtils.setNetPeer(span, url.getHost(), null, url.getPort()); + SemanticAttributes.HTTP_URL.set(span, url.toString()); } + } catch (Exception e) { + log.debug("Error tagging url", e); } - return span; } protected Span onResponse(Span span, RESPONSE response) { diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy index 40c8a21b56da..ec27fcfe263a 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy @@ -17,13 +17,13 @@ package io.opentelemetry.instrumentation.api.tracer import static io.opentelemetry.auto.test.utils.ConfigUtils.withConfigOverride + import io.opentelemetry.context.propagation.TextMapPropagator import io.opentelemetry.instrumentation.api.decorator.HttpStatusConverter import io.opentelemetry.trace.Span import io.opentelemetry.trace.attributes.SemanticAttributes import spock.lang.Shared - class HttpClientTracerTest extends BaseTracerTest { @Shared @@ -47,6 +47,7 @@ class HttpClientTracerTest extends BaseTracerTest { 1 * span.setAttribute(SemanticAttributes.NET_PEER_NAME.key(), req.url.host) 1 * span.setAttribute(SemanticAttributes.NET_PEER_PORT.key(), req.url.port) 1 * span.setAttribute(SemanticAttributes.HTTP_USER_AGENT.key(), req["User-Agent"]) + 1 * span.setAttribute(SemanticAttributes.HTTP_FLAVOR.key(), "1.1") } 0 * _ @@ -78,6 +79,7 @@ class HttpClientTracerTest extends BaseTracerTest { 1 * span.setAttribute(SemanticAttributes.NET_PEER_PORT.key(), req.url.port) 1 * span.setAttribute("peer.service", "reservation-service") 1 * span.setAttribute(SemanticAttributes.HTTP_USER_AGENT.key(), req["User-Agent"]) + 1 * span.setAttribute(SemanticAttributes.HTTP_FLAVOR.key(), "1.1") } 0 * _ } @@ -87,20 +89,16 @@ class HttpClientTracerTest extends BaseTracerTest { def tracer = newTracer() when: - withConfigOverride(io.opentelemetry.instrumentation.api.config.Config.HTTP_CLIENT_TAG_QUERY_STRING, "$tagQueryString") { - tracer.onRequest(span, req) - } + tracer.onRequest(span, req) then: 1 * span.setAttribute(SemanticAttributes.NET_TRANSPORT.key(), "IP.TCP") - if (expectedUrl) { + if (expectedUrl != null) { 1 * span.setAttribute(SemanticAttributes.HTTP_URL.key(), expectedUrl) } - if (expectedUrl && tagQueryString) { - 1 * span.setAttribute(io.opentelemetry.instrumentation.api.MoreAttributes.HTTP_QUERY, expectedQuery) - 1 * span.setAttribute(io.opentelemetry.instrumentation.api.MoreAttributes.HTTP_FRAGMENT, expectedFragment) - } 1 * span.setAttribute(SemanticAttributes.HTTP_METHOD.key(), null) + 1 * span.setAttribute(SemanticAttributes.HTTP_FLAVOR.key(), "1.1") + 1 * span.setAttribute(SemanticAttributes.HTTP_USER_AGENT.key(), null) if (hostname) { 1 * span.setAttribute(SemanticAttributes.NET_PEER_NAME.key(), hostname) } @@ -110,19 +108,13 @@ class HttpClientTracerTest extends BaseTracerTest { 0 * _ where: - tagQueryString | url | expectedUrl | expectedQuery | expectedFragment | hostname | port - false | null | null | null | null | null | null - false | "" | "/" | "" | null | null | null - false | "/path?query" | "/path?query" | "" | null | null | null - false | "https://host:0" | "https://host/" | "" | null | "host" | null - false | "https://host/path" | "https://host/path" | "" | null | "host" | null - false | "http://host:99/path?query#fragment" | "http://host:99/path?query#fragment" | "" | null | "host" | 99 - true | null | null | null | null | null | null - true | "" | "/" | null | null | null | null - true | "/path?encoded+%28query%29%3F" | "/path?encoded+(query)?" | "encoded+(query)?" | null | null | null - true | "https://host:0" | "https://host/" | null | null | "host" | null - true | "https://host/path" | "https://host/path" | null | null | "host" | null - true | "http://host:99/path?query#encoded+%28fragment%29%3F" | "http://host:99/path?query#encoded+(fragment)?" | "query" | "encoded+(fragment)?" | "host" | 99 + tagQueryString | url | expectedUrl | expectedQuery | expectedFragment | hostname | port + false | null | null | null | null | null | null + false | "" | "" | "" | null | null | null + false | "/path?query" | "/path?query" | "" | null | null | null + false | "https://host:0" | "https://host:0" | "" | null | "host" | null + false | "https://host/path" | "https://host/path" | "" | null | "host" | null + false | "http://host:99/path?query#fragment" | "http://host:99/path?query#fragment" | "" | null | "host" | 99 req = [url: url == null ? null : new URI(url)] } diff --git a/instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/instrumentation/auto/akkahttp/AkkaHttpClientTracer.java b/instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/instrumentation/auto/akkahttp/AkkaHttpClientTracer.java index 95880ea2528c..1890d797bee2 100644 --- a/instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/instrumentation/auto/akkahttp/AkkaHttpClientTracer.java +++ b/instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/instrumentation/auto/akkahttp/AkkaHttpClientTracer.java @@ -48,6 +48,11 @@ protected URI url(HttpRequest httpRequest) throws URISyntaxException { return new URI(httpRequest.uri().toString()); } + @Override + protected String flavor(HttpRequest httpRequest) { + return httpRequest.protocol().value(); + } + @Override protected Integer status(HttpResponse httpResponse) { return httpResponse.status().intValue(); diff --git a/instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpasyncclient/ApacheHttpAsyncClientTracer.java b/instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpasyncclient/ApacheHttpAsyncClientTracer.java index 69e5131b8af5..75e279ff9506 100644 --- a/instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpasyncclient/ApacheHttpAsyncClientTracer.java +++ b/instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpasyncclient/ApacheHttpAsyncClientTracer.java @@ -30,6 +30,7 @@ import org.apache.http.RequestLine; import org.apache.http.StatusLine; import org.apache.http.client.methods.HttpUriRequest; +import org.checkerframework.checker.nullness.qual.Nullable; public class ApacheHttpAsyncClientTracer extends HttpClientTracer { @@ -46,6 +47,11 @@ protected String method(HttpRequest request) { } } + @Override + protected @Nullable String flavor(HttpRequest httpRequest) { + return httpRequest.getProtocolVersion().toString(); + } + @Override protected URI url(HttpRequest request) throws URISyntaxException { /* diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpclient/v4_0/ApacheHttpClientTracer.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpclient/v4_0/ApacheHttpClientTracer.java index dfbb29b99f51..327e83988f50 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpclient/v4_0/ApacheHttpClientTracer.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/instrumentation/auto/apachehttpclient/v4_0/ApacheHttpClientTracer.java @@ -26,6 +26,7 @@ import org.apache.http.HttpMessage; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpUriRequest; +import org.checkerframework.checker.nullness.qual.Nullable; class ApacheHttpClientTracer extends HttpClientTracer { @@ -37,6 +38,11 @@ protected String method(HttpUriRequest httpRequest) { return httpRequest.getMethod(); } + @Override + protected @Nullable String flavor(HttpUriRequest httpUriRequest) { + return httpUriRequest.getProtocolVersion().toString(); + } + @Override protected URI url(HttpUriRequest request) { return request.getURI(); diff --git a/instrumentation/armeria-1.0/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_0/client/ArmeriaClientTracer.java b/instrumentation/armeria-1.0/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_0/client/ArmeriaClientTracer.java index fff926316a1c..4b98f083cb37 100644 --- a/instrumentation/armeria-1.0/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_0/client/ArmeriaClientTracer.java +++ b/instrumentation/armeria-1.0/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_0/client/ArmeriaClientTracer.java @@ -40,6 +40,11 @@ protected String method(ClientRequestContext ctx) { return ctx.method().name(); } + @Override + protected @Nullable String flavor(ClientRequestContext clientRequestContext) { + return clientRequestContext.sessionProtocol().toString(); + } + @Override @Nullable protected URI url(ClientRequestContext ctx) throws URISyntaxException { diff --git a/instrumentation/armeria-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_0/AbstractArmeriaTest.groovy b/instrumentation/armeria-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_0/AbstractArmeriaTest.groovy index a4c7fe23e6ee..edabce4951c0 100644 --- a/instrumentation/armeria-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_0/AbstractArmeriaTest.groovy +++ b/instrumentation/armeria-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_0/AbstractArmeriaTest.groovy @@ -77,6 +77,7 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification { "${SemanticAttributes.HTTP_URL.key()}" "${server.httpUri()}${path}" "${SemanticAttributes.HTTP_METHOD.key()}" method.name() "${SemanticAttributes.HTTP_STATUS_CODE.key()}" code + "${SemanticAttributes.HTTP_FLAVOR.key()}" "http" } } span(1) { diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy index ff86a925a7cc..6fc03a5bff65 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy @@ -153,9 +153,10 @@ class AWS1ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "$server.address/" + "${SemanticAttributes.HTTP_URL.key()}" "$server.address" "${SemanticAttributes.HTTP_METHOD.key()}" "$method" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" "aws.service" { it.contains(service) } @@ -229,8 +230,9 @@ class AWS1ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:${UNUSABLE_PORT}/" + "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:${UNUSABLE_PORT}" "${SemanticAttributes.HTTP_METHOD.key()}" "$method" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" "${SemanticAttributes.NET_PEER_PORT.key()}" 61 "aws.service" { it.contains(service) } @@ -276,8 +278,9 @@ class AWS1ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "https://s3.amazonaws.com/" + "${SemanticAttributes.HTTP_URL.key()}" "https://s3.amazonaws.com" "${SemanticAttributes.HTTP_METHOD.key()}" "HEAD" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_NAME.key()}" "s3.amazonaws.com" "aws.service" "Amazon S3" "aws.endpoint" "https://s3.amazonaws.com" @@ -324,10 +327,11 @@ class AWS1ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "$server.address/" + "${SemanticAttributes.HTTP_URL.key()}" "$server.address" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "aws.service" "Amazon S3" "aws.endpoint" "$server.address" "aws.operation" "GetObjectRequest" diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy index 759ee8fd3204..8666ebf8efd5 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy @@ -116,9 +116,10 @@ class AWS0ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "$server.address/" + "${SemanticAttributes.HTTP_URL.key()}" "$server.address" "${SemanticAttributes.HTTP_METHOD.key()}" "$method" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" "aws.service" { it.contains(service) } @@ -174,8 +175,9 @@ class AWS0ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:${UNUSABLE_PORT}/" + "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:${UNUSABLE_PORT}" "${SemanticAttributes.HTTP_METHOD.key()}" "$method" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_PORT.key()}" 61 "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" "aws.service" { it.contains(service) } @@ -221,8 +223,9 @@ class AWS0ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "https://s3.amazonaws.com/" + "${SemanticAttributes.HTTP_URL.key()}" "https://s3.amazonaws.com" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_NAME.key()}" "s3.amazonaws.com" "aws.service" "Amazon S3" "aws.endpoint" "https://s3.amazonaws.com" @@ -266,8 +269,9 @@ class AWS0ClientTest extends AgentTestRunner { parent() attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" - "${SemanticAttributes.HTTP_URL.key()}" "$server.address/" + "${SemanticAttributes.HTTP_URL.key()}" "$server.address" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" "aws.service" "Amazon S3" diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/groovy/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/groovy/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.groovy index f4dac20f645e..e0369b66b75c 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/groovy/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/groovy/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.groovy @@ -146,6 +146,7 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { "${SemanticAttributes.HTTP_METHOD.key()}" "$method" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 "${SemanticAttributes.HTTP_USER_AGENT.key()}" { it.startsWith("aws-sdk-java/") } + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "aws.service" "$service" "aws.operation" "${operation}" "aws.agent" "java-aws-sdk" @@ -216,6 +217,7 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.HTTP_URL.key()}" { it.startsWith("${server.address}${path}") } "${SemanticAttributes.HTTP_METHOD.key()}" "$method" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 "${SemanticAttributes.HTTP_USER_AGENT.key()}" { it.startsWith("aws-sdk-java/") } "aws.service" "$service" @@ -241,14 +243,14 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { service | operation | method | path | requestId | builder | call | body "S3" | "CreateBucket" | "PUT" | "/somebucket" | "UNKNOWN" | S3Client.builder() | { c -> c.createBucket(CreateBucketRequest.builder().bucket("somebucket").build()) } | "" "S3" | "GetObject" | "GET" | "/somebucket/somekey" | "UNKNOWN" | S3Client.builder() | { c -> c.getObject(GetObjectRequest.builder().bucket("somebucket").key("somekey").build()) } | "" - "Kinesis" | "DeleteStream" | "POST" | "/" | "UNKNOWN" | KinesisClient.builder() | { c -> c.deleteStream(DeleteStreamRequest.builder().streamName("somestream").build()) } | "" - "Sqs" | "CreateQueue" | "POST" | "/" | "7a62c49f-347e-4fc4-9331-6e8e7a96aa73" | SqsClient.builder() | { c -> c.createQueue(CreateQueueRequest.builder().queueName("somequeue").build()) } | """ + "Kinesis" | "DeleteStream" | "POST" | "" | "UNKNOWN" | KinesisClient.builder() | { c -> c.deleteStream(DeleteStreamRequest.builder().streamName("somestream").build()) } | "" + "Sqs" | "CreateQueue" | "POST" | "" | "7a62c49f-347e-4fc4-9331-6e8e7a96aa73" | SqsClient.builder() | { c -> c.createQueue(CreateQueueRequest.builder().queueName("somequeue").build()) } | """ https://queue.amazonaws.com/123456789012/MyQueue 7a62c49f-347e-4fc4-9331-6e8e7a96aa73 """ - "Sqs" | "SendMessage" | "POST" | "/" | "27daac76-34dd-47df-bd01-1f6e873584a0" | SqsClient.builder() | { c -> c.sendMessage(SendMessageRequest.builder().queueUrl("someurl").messageBody("").build()) } | """ + "Sqs" | "SendMessage" | "POST" | "" | "27daac76-34dd-47df-bd01-1f6e873584a0" | SqsClient.builder() | { c -> c.sendMessage(SendMessageRequest.builder().queueUrl("someurl").messageBody("").build()) } | """ d41d8cd98f00b204e9800998ecf8427e @@ -258,14 +260,14 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { 27daac76-34dd-47df-bd01-1f6e873584a0 """ - "Ec2" | "AllocateAddress" | "POST" | "/" | "59dbff89-35bd-4eac-99ed-be587EXAMPLE" | Ec2Client.builder() | { c -> c.allocateAddress() } | """ + "Ec2" | "AllocateAddress" | "POST" | "" | "59dbff89-35bd-4eac-99ed-be587EXAMPLE" | Ec2Client.builder() | { c -> c.allocateAddress() } | """ 59dbff89-35bd-4eac-99ed-be587EXAMPLE 192.0.2.1 standard """ - "Rds" | "DeleteOptionGroup" | "POST" | "/" | "0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99" | RdsClient.builder() | { c -> c.deleteOptionGroup(DeleteOptionGroupRequest.builder().build()) } | """ + "Rds" | "DeleteOptionGroup" | "POST" | "" | "0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99" | RdsClient.builder() | { c -> c.deleteOptionGroup(DeleteOptionGroupRequest.builder().build()) } | """ 0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99 @@ -303,6 +305,7 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.HTTP_URL.key()}" { it.startsWith("${server.address}${path}") } "${SemanticAttributes.HTTP_METHOD.key()}" "$method" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 "${SemanticAttributes.HTTP_USER_AGENT.key()}" { it.startsWith("aws-sdk-java/") } "aws.service" "$service" @@ -330,13 +333,13 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { "S3" | "GetObject" | "GET" | "/somebucket/somekey" | "UNKNOWN" | S3AsyncClient.builder() | { c -> c.getObject(GetObjectRequest.builder().bucket("somebucket").key("somekey").build(), AsyncResponseTransformer.toBytes()) } | "1234567890" // Kinesis seems to expect an http2 response which is incompatible with our test server. // "Kinesis" | "DeleteStream" | "POST" | "/" | "UNKNOWN" | KinesisAsyncClient.builder() | { c -> c.deleteStream(DeleteStreamRequest.builder().streamName("somestream").build()) } | "" - "Sqs" | "CreateQueue" | "POST" | "/" | "7a62c49f-347e-4fc4-9331-6e8e7a96aa73" | SqsAsyncClient.builder() | { c -> c.createQueue(CreateQueueRequest.builder().queueName("somequeue").build()) } | """ + "Sqs" | "CreateQueue" | "POST" | "" | "7a62c49f-347e-4fc4-9331-6e8e7a96aa73" | SqsAsyncClient.builder() | { c -> c.createQueue(CreateQueueRequest.builder().queueName("somequeue").build()) } | """ https://queue.amazonaws.com/123456789012/MyQueue 7a62c49f-347e-4fc4-9331-6e8e7a96aa73 """ - "Sqs" | "SendMessage" | "POST" | "/" | "27daac76-34dd-47df-bd01-1f6e873584a0" | SqsAsyncClient.builder() | { c -> c.sendMessage(SendMessageRequest.builder().queueUrl("someurl").messageBody("").build()) } | """ + "Sqs" | "SendMessage" | "POST" | "" | "27daac76-34dd-47df-bd01-1f6e873584a0" | SqsAsyncClient.builder() | { c -> c.sendMessage(SendMessageRequest.builder().queueUrl("someurl").messageBody("").build()) } | """ d41d8cd98f00b204e9800998ecf8427e @@ -346,14 +349,14 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { 27daac76-34dd-47df-bd01-1f6e873584a0 """ - "Ec2" | "AllocateAddress" | "POST" | "/" | "59dbff89-35bd-4eac-99ed-be587EXAMPLE" | Ec2AsyncClient.builder() | { c -> c.allocateAddress() } | """ + "Ec2" | "AllocateAddress" | "POST" | "" | "59dbff89-35bd-4eac-99ed-be587EXAMPLE" | Ec2AsyncClient.builder() | { c -> c.allocateAddress() } | """ 59dbff89-35bd-4eac-99ed-be587EXAMPLE 192.0.2.1 standard """ - "Rds" | "DeleteOptionGroup" | "POST" | "/" | "0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99" | RdsAsyncClient.builder() | { c -> c.deleteOptionGroup(DeleteOptionGroupRequest.builder().build()) } | """ + "Rds" | "DeleteOptionGroup" | "POST" | "" | "0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99" | RdsAsyncClient.builder() | { c -> c.deleteOptionGroup(DeleteOptionGroupRequest.builder().build()) } | """ 0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99 @@ -402,6 +405,7 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification { "${SemanticAttributes.NET_PEER_PORT.key()}" server.address.port "${SemanticAttributes.HTTP_URL.key()}" "$server.address/somebucket/somekey" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" "aws.service" "S3" "aws.operation" "GetObject" "aws.agent" "java-aws-sdk" diff --git a/instrumentation/elasticsearch/elasticsearch-rest-5.0/src/test/groovy/Elasticsearch5RestClientTest.groovy b/instrumentation/elasticsearch/elasticsearch-rest-5.0/src/test/groovy/Elasticsearch5RestClientTest.groovy index 43543d92d6eb..0681a36217a9 100644 --- a/instrumentation/elasticsearch/elasticsearch-rest-5.0/src/test/groovy/Elasticsearch5RestClientTest.groovy +++ b/instrumentation/elasticsearch/elasticsearch-rest-5.0/src/test/groovy/Elasticsearch5RestClientTest.groovy @@ -119,6 +119,7 @@ class Elasticsearch5RestClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" "_cluster/health" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } diff --git a/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy b/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy index 9dfa35a473e3..47deb7020dd2 100644 --- a/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy +++ b/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy @@ -120,6 +120,7 @@ class Elasticsearch6RestClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" "_cluster/health" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } diff --git a/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/test/groovy/Elasticsearch6RestClientTest.groovy b/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/test/groovy/Elasticsearch6RestClientTest.groovy index 022b80e33b9c..af98716f6483 100644 --- a/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/test/groovy/Elasticsearch6RestClientTest.groovy +++ b/instrumentation/elasticsearch/elasticsearch-rest-6.4/src/test/groovy/Elasticsearch6RestClientTest.groovy @@ -115,6 +115,7 @@ class Elasticsearch6RestClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" "_cluster/health" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200 + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } diff --git a/instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy b/instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy index 8f81d0b998b9..d14c907d4603 100644 --- a/instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy +++ b/instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy @@ -84,6 +84,7 @@ abstract class AbstractGoogleHttpClientTest extends HttpClientTest { "${SemanticAttributes.HTTP_URL.key()}" "${uri}" "${SemanticAttributes.HTTP_METHOD.key()}" method "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500 + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } server.distributedRequestSpan(it, 1, span(0)) diff --git a/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy b/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy index 397c25d17c29..edd2b7fe22be 100644 --- a/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy +++ b/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy @@ -56,8 +56,9 @@ class UrlConnectionTest extends AgentTestRunner { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" "${SemanticAttributes.NET_PEER_NAME.key()}" "localhost" "${SemanticAttributes.NET_PEER_PORT.key()}" UNUSABLE_PORT - "${SemanticAttributes.HTTP_URL.key()}" "$url/" + "${SemanticAttributes.HTTP_URL.key()}" "$url" "${SemanticAttributes.HTTP_METHOD.key()}" "GET" + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } diff --git a/instrumentation/java-httpclient/src/main/java/io/opentelemetry/instrumentation/auto/httpclient/JdkHttpClientTracer.java b/instrumentation/java-httpclient/src/main/java/io/opentelemetry/instrumentation/auto/httpclient/JdkHttpClientTracer.java index 7c8757181c13..8858b41dd9a0 100644 --- a/instrumentation/java-httpclient/src/main/java/io/opentelemetry/instrumentation/auto/httpclient/JdkHttpClientTracer.java +++ b/instrumentation/java-httpclient/src/main/java/io/opentelemetry/instrumentation/auto/httpclient/JdkHttpClientTracer.java @@ -22,8 +22,11 @@ import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import io.opentelemetry.instrumentation.auto.api.CallDepthThreadLocalMap; import io.opentelemetry.instrumentation.auto.api.CallDepthThreadLocalMap.Depth; +import io.opentelemetry.trace.Span; +import io.opentelemetry.trace.attributes.SemanticAttributes; import java.net.URI; import java.net.http.HttpClient; +import java.net.http.HttpClient.Version; import java.net.http.HttpHeaders; import java.net.http.HttpRequest; import java.net.http.HttpResponse; @@ -33,7 +36,8 @@ import java.util.Map; import java.util.concurrent.CompletionException; -public class JdkHttpClientTracer extends HttpClientTracer { +public class JdkHttpClientTracer + extends HttpClientTracer> { public static final JdkHttpClientTracer TRACER = new JdkHttpClientTracer(); public Depth getCallDepth() { @@ -56,7 +60,7 @@ protected URI url(HttpRequest httpRequest) { } @Override - protected Integer status(HttpResponse httpResponse) { + protected Integer status(HttpResponse httpResponse) { return httpResponse.statusCode(); } @@ -66,10 +70,22 @@ protected String requestHeader(HttpRequest httpRequest, String name) { } @Override - protected String responseHeader(HttpResponse httpResponse, String name) { + protected String responseHeader(HttpResponse httpResponse, String name) { return httpResponse.headers().firstValue(name).orElse(null); } + @Override + protected Span onResponse(Span span, HttpResponse httpResponse) { + span = super.onResponse(span, httpResponse); + + if (httpResponse != null) { + SemanticAttributes.HTTP_FLAVOR.set( + span, httpResponse.version() == Version.HTTP_1_1 ? "1.1" : "2.0"); + } + + return span; + } + @Override protected Setter getSetter() { return HttpHeadersInjectAdapter.SETTER; diff --git a/instrumentation/java-httpclient/src/test/groovy/JdkHttpClientTest.groovy b/instrumentation/java-httpclient/src/test/groovy/JdkHttpClientTest.groovy index a275c842f539..3f639ebd99d6 100644 --- a/instrumentation/java-httpclient/src/test/groovy/JdkHttpClientTest.groovy +++ b/instrumentation/java-httpclient/src/test/groovy/JdkHttpClientTest.groovy @@ -14,15 +14,18 @@ * limitations under the License. */ -import io.opentelemetry.auto.test.base.HttpClientTest -import spock.lang.Shared -import spock.lang.Timeout +import static io.opentelemetry.trace.Span.Kind.CLIENT +import io.opentelemetry.auto.test.base.HttpClientTest +import io.opentelemetry.trace.attributes.SemanticAttributes import java.net.http.HttpClient import java.net.http.HttpRequest import java.net.http.HttpResponse import java.time.Duration import java.time.temporal.ChronoUnit +import spock.lang.Requires +import spock.lang.Shared +import spock.lang.Timeout @Timeout(5) abstract class JdkHttpClientTest extends HttpClientTest { @@ -52,4 +55,46 @@ abstract class JdkHttpClientTest extends HttpClientTest { boolean testCircularRedirects() { return false } + + //We override this test below because it produces somewhat different attributes + @Override + boolean testRemoteConnection() { + return false + } + + @Requires({ !System.getProperty("java.vm.name").contains("IBM J9 VM") }) + def "test https request"() { + given: + def uri = new URI("https://www.google.com/") + + when: + def status = doRequest(method, uri) + + then: + status == 200 + assertTraces(1) { + trace(0, 1 + extraClientSpans()) { + span(0) { + parent() + operationName expectedOperationName(method) + spanKind CLIENT + errored false + attributes { + "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" + "${SemanticAttributes.NET_PEER_NAME.key()}" uri.host + "${SemanticAttributes.NET_PEER_IP.key()}" { it == null || it == "127.0.0.1" } // Optional + "${SemanticAttributes.NET_PEER_PORT.key()}" uri.port > 0 ? uri.port : { it == null || it == 443 } + "${SemanticAttributes.HTTP_URL.key()}" { it == "${uri}" || it == "${removeFragment(uri)}" } + "${SemanticAttributes.HTTP_METHOD.key()}" method + "${SemanticAttributes.HTTP_FLAVOR.key()}" "2.0" + "${SemanticAttributes.HTTP_STATUS_CODE.key()}" status + } + } + } + } + + where: + method = "HEAD" + } + } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0/src/test/groovy/JaxRsClientTest.groovy b/instrumentation/jaxrs-client/jaxrs-client-2.0/src/test/groovy/JaxRsClientTest.groovy index 012e534608be..a23320f289bd 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0/src/test/groovy/JaxRsClientTest.groovy +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0/src/test/groovy/JaxRsClientTest.groovy @@ -79,6 +79,7 @@ abstract class JaxRsClientTest extends HttpClientTest { "${SemanticAttributes.HTTP_URL.key()}" "${uri}" "${SemanticAttributes.HTTP_METHOD.key()}" method "${SemanticAttributes.HTTP_STATUS_CODE.key()}" statusCode + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } serverSpan(it, 1, span(0)) diff --git a/instrumentation/netty/netty-3.8/src/main/java/io/opentelemetry/instrumentation/auto/netty/v3_8/client/NettyHttpClientTracer.java b/instrumentation/netty/netty-3.8/src/main/java/io/opentelemetry/instrumentation/auto/netty/v3_8/client/NettyHttpClientTracer.java index 2b9db7051dbd..6f74cecb5ef4 100644 --- a/instrumentation/netty/netty-3.8/src/main/java/io/opentelemetry/instrumentation/auto/netty/v3_8/client/NettyHttpClientTracer.java +++ b/instrumentation/netty/netty-3.8/src/main/java/io/opentelemetry/instrumentation/auto/netty/v3_8/client/NettyHttpClientTracer.java @@ -28,6 +28,7 @@ import io.opentelemetry.trace.Span; import java.net.URI; import java.net.URISyntaxException; +import org.checkerframework.checker.nullness.qual.Nullable; import org.jboss.netty.handler.codec.http.HttpHeaders; import org.jboss.netty.handler.codec.http.HttpRequest; import org.jboss.netty.handler.codec.http.HttpResponse; @@ -55,6 +56,11 @@ protected String method(HttpRequest httpRequest) { return httpRequest.getMethod().getName(); } + @Override + protected @Nullable String flavor(HttpRequest httpRequest) { + return httpRequest.getProtocolVersion().getText(); + } + @Override protected URI url(HttpRequest request) throws URISyntaxException { URI uri = new URI(request.getUri()); diff --git a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_0/client/NettyHttpClientTracer.java b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_0/client/NettyHttpClientTracer.java index 5e3252c179f7..2832774a3d5f 100644 --- a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_0/client/NettyHttpClientTracer.java +++ b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_0/client/NettyHttpClientTracer.java @@ -31,6 +31,7 @@ import io.opentelemetry.trace.Span; import java.net.URI; import java.net.URISyntaxException; +import org.checkerframework.checker.nullness.qual.Nullable; public class NettyHttpClientTracer extends HttpClientTracer { @@ -55,6 +56,11 @@ protected String method(HttpRequest httpRequest) { return httpRequest.getMethod().name(); } + @Override + protected @Nullable String flavor(HttpRequest httpRequest) { + return httpRequest.getProtocolVersion().text(); + } + @Override protected URI url(HttpRequest request) throws URISyntaxException { URI uri = new URI(request.getUri()); diff --git a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_1/client/NettyHttpClientTracer.java b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_1/client/NettyHttpClientTracer.java index 19b13cfa5569..4d18293d939b 100644 --- a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_1/client/NettyHttpClientTracer.java +++ b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/instrumentation/auto/netty/v4_1/client/NettyHttpClientTracer.java @@ -26,6 +26,7 @@ import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import java.net.URI; import java.net.URISyntaxException; +import org.checkerframework.checker.nullness.qual.Nullable; public class NettyHttpClientTracer extends HttpClientTracer { @@ -36,6 +37,11 @@ protected String method(HttpRequest httpRequest) { return httpRequest.method().name(); } + @Override + protected @Nullable String flavor(HttpRequest httpRequest) { + return httpRequest.protocolVersion().text(); + } + @Override protected URI url(HttpRequest request) throws URISyntaxException { URI uri = new URI(request.uri()); diff --git a/instrumentation/netty/netty-4.1/src/test/groovy/ReactorNettyTest.groovy b/instrumentation/netty/netty-4.1/src/test/groovy/ReactorNettyTest.groovy index ba7be0a19a93..e05eee24a5d1 100644 --- a/instrumentation/netty/netty-4.1/src/test/groovy/ReactorNettyTest.groovy +++ b/instrumentation/netty/netty-4.1/src/test/groovy/ReactorNettyTest.groovy @@ -86,14 +86,12 @@ class ReactorNettyTest extends AgentTestRunner { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" "${SemanticAttributes.NET_PEER_NAME.key()}" uri.host "${SemanticAttributes.NET_PEER_IP.key()}" { it == null || it == "127.0.0.1" } // Optional - // Optional "${SemanticAttributes.NET_PEER_PORT.key()}" uri.port > 0 ? uri.port : { it == null || it == 443 } "${SemanticAttributes.HTTP_URL.key()}" uri.toString() "${SemanticAttributes.HTTP_METHOD.key()}" method "${SemanticAttributes.HTTP_USER_AGENT.key()}" { it.startsWith(userAgent) } - if (status) { - "${SemanticAttributes.HTTP_STATUS_CODE.key()}" status - } + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" + "${SemanticAttributes.HTTP_STATUS_CODE.key()}" status } } } diff --git a/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy b/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy index 8a311f828be2..6bcf436dc397 100644 --- a/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy +++ b/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy @@ -266,6 +266,7 @@ class TwilioClientTest extends AgentTestRunner { span(1) { operationName "MessageCreator.create" spanKind CLIENT + childOf(span(0)) errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Message" @@ -277,6 +278,7 @@ class TwilioClientTest extends AgentTestRunner { span(2) { operationName expectedOperationName("POST") spanKind CLIENT + childOf(span(1)) errored false attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" @@ -284,6 +286,7 @@ class TwilioClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" String "${SemanticAttributes.HTTP_METHOD.key()}" String "${SemanticAttributes.HTTP_STATUS_CODE.key()}" Long + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } @@ -359,6 +362,7 @@ class TwilioClientTest extends AgentTestRunner { span(1) { operationName "MessageCreator.create" spanKind CLIENT + childOf(span(0)) errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Message" @@ -370,6 +374,7 @@ class TwilioClientTest extends AgentTestRunner { span(2) { operationName expectedOperationName("POST") spanKind CLIENT + childOf(span(1)) errored true attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" @@ -377,11 +382,13 @@ class TwilioClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" String "${SemanticAttributes.HTTP_METHOD.key()}" String "${SemanticAttributes.HTTP_STATUS_CODE.key()}" Long + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } span(3) { operationName expectedOperationName("POST") spanKind CLIENT + childOf(span(1)) errored false attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" @@ -389,6 +396,7 @@ class TwilioClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" String "${SemanticAttributes.HTTP_METHOD.key()}" String "${SemanticAttributes.HTTP_STATUS_CODE.key()}" Long + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } @@ -471,6 +479,7 @@ class TwilioClientTest extends AgentTestRunner { span(1) { operationName "MessageCreator.createAsync" spanKind CLIENT + childOf(span(0)) errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Message" @@ -482,6 +491,7 @@ class TwilioClientTest extends AgentTestRunner { span(2) { operationName "MessageCreator.create" spanKind CLIENT + childOf(span(1)) errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Message" @@ -493,6 +503,7 @@ class TwilioClientTest extends AgentTestRunner { span(3) { operationName expectedOperationName("POST") spanKind CLIENT + childOf(span(2)) errored true attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" @@ -500,11 +511,13 @@ class TwilioClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" String "${SemanticAttributes.HTTP_METHOD.key()}" String "${SemanticAttributes.HTTP_STATUS_CODE.key()}" Long + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } span(4) { operationName expectedOperationName("POST") spanKind CLIENT + childOf(span(2)) errored false attributes { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" @@ -512,6 +525,7 @@ class TwilioClientTest extends AgentTestRunner { "${SemanticAttributes.HTTP_URL.key()}" String "${SemanticAttributes.HTTP_METHOD.key()}" String "${SemanticAttributes.HTTP_STATUS_CODE.key()}" Long + "${SemanticAttributes.HTTP_FLAVOR.key()}" "1.1" } } } diff --git a/testing-common/src/main/groovy/io/opentelemetry/auto/test/InMemoryExporter.java b/testing-common/src/main/groovy/io/opentelemetry/auto/test/InMemoryExporter.java index 3f8b4dd3f62a..311cecd917ca 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/auto/test/InMemoryExporter.java +++ b/testing-common/src/main/groovy/io/opentelemetry/auto/test/InMemoryExporter.java @@ -68,7 +68,8 @@ public class InMemoryExporter implements SpanProcessor { public void onStart(ReadWriteSpan readWriteSpan) { SpanData sd = readWriteSpan.toSpanData(); log.debug( - ">>> SPAN START: {} id={} traceid={} parent={}, library={}", + ">>>{} SPAN START: {} id={} traceid={} parent={}, library={}", + sd.getStartEpochNanos(), sd.getName(), sd.getSpanId(), sd.getTraceId(), @@ -89,7 +90,8 @@ public boolean isStartRequired() { public void onEnd(ReadableSpan readableSpan) { SpanData sd = readableSpan.toSpanData(); log.debug( - "<<< SPAN END: {} id={} traceid={} parent={}, library={}, attributes={}", + "<<<{} SPAN END: {} id={} traceid={} parent={}, library={}, attributes={}", + sd.getEndEpochNanos(), sd.getName(), sd.getSpanId(), sd.getTraceId(), diff --git a/testing-common/src/main/groovy/io/opentelemetry/auto/test/base/HttpClientTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/auto/test/base/HttpClientTest.groovy index aca9626c83da..cf81b0259a7e 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/auto/test/base/HttpClientTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/auto/test/base/HttpClientTest.groovy @@ -17,7 +17,6 @@ package io.opentelemetry.auto.test.base import static io.opentelemetry.auto.test.server.http.TestHttpServer.httpServer -import static io.opentelemetry.auto.test.utils.ConfigUtils.withConfigOverride import static io.opentelemetry.auto.test.utils.PortUtils.UNUSABLE_PORT import static io.opentelemetry.auto.test.utils.TraceUtils.basicSpan import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace @@ -27,8 +26,6 @@ import static org.junit.Assume.assumeTrue import io.opentelemetry.auto.test.AgentTestRunner import io.opentelemetry.auto.test.asserts.TraceAssert -import io.opentelemetry.instrumentation.api.MoreAttributes -import io.opentelemetry.instrumentation.api.config.Config import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer import io.opentelemetry.sdk.trace.data.SpanData import io.opentelemetry.trace.attributes.SemanticAttributes @@ -106,29 +103,21 @@ abstract class HttpClientTest extends AgentTestRunner { return null } - def "basic #method request #url - tagQueryString=#tagQueryString"() { + def "basic #method request #url"() { when: - def status = withConfigOverride(Config.HTTP_CLIENT_TAG_QUERY_STRING, "$tagQueryString") { - doRequest(method, url) - } + def status = doRequest(method, url) then: status == 200 assertTraces(1) { trace(0, 2 + extraClientSpans()) { - clientSpan(it, 0, null, method, tagQueryString, url) + clientSpan(it, 0, null, method, url) serverSpan(it, 1 + extraClientSpans(), span(extraClientSpans())) } } where: - path | tagQueryString - "/success" | false - "/success" | true - "/success?with=params" | false - "/success?with=params" | true - "/success#with+fragment" | true - "/success?with=params#and=fragment" | true + path << ["/success", "/success?with=params"] method = "GET" url = server.address.resolve(path) @@ -240,7 +229,7 @@ abstract class HttpClientTest extends AgentTestRunner { status == 200 assertTraces(1) { trace(0, 3 + extraClientSpans()) { - clientSpan(it, 0, null, method, false, uri) + clientSpan(it, 0, null, method, uri) serverSpan(it, 1 + extraClientSpans(), span(extraClientSpans())) serverSpan(it, 2 + extraClientSpans(), span(extraClientSpans())) } @@ -262,7 +251,7 @@ abstract class HttpClientTest extends AgentTestRunner { status == 200 assertTraces(1) { trace(0, 4 + extraClientSpans()) { - clientSpan(it, 0, null, method, false, uri) + clientSpan(it, 0, null, method, uri) serverSpan(it, 1 + extraClientSpans(), span(extraClientSpans())) serverSpan(it, 2 + extraClientSpans(), span(extraClientSpans())) serverSpan(it, 3 + extraClientSpans(), span(extraClientSpans())) @@ -288,7 +277,7 @@ abstract class HttpClientTest extends AgentTestRunner { and: assertTraces(1) { trace(0, 3 + extraClientSpans()) { - clientSpan(it, 0, null, method, false, uri, statusOnRedirectError(), thrownException) + clientSpan(it, 0, null, method, uri, statusOnRedirectError(), thrownException) serverSpan(it, 1 + extraClientSpans(), span(extraClientSpans())) serverSpan(it, 2 + extraClientSpans(), span(extraClientSpans())) } @@ -311,7 +300,7 @@ abstract class HttpClientTest extends AgentTestRunner { status == 200 assertTraces(1) { trace(0, 3 + extraClientSpans()) { - clientSpan(it, 0, null, method, false, uri) + clientSpan(it, 0, null, method, uri) serverSpan(it, 1 + extraClientSpans(), span(extraClientSpans())) serverSpan(it, 2 + extraClientSpans(), span(extraClientSpans())) } @@ -339,7 +328,7 @@ abstract class HttpClientTest extends AgentTestRunner { assertTraces(1) { trace(0, 2 + extraClientSpans()) { basicSpan(it, 0, "parent", null, thrownException) - clientSpan(it, 1, span(0), method, false, uri, null, thrownException) + clientSpan(it, 1, span(0), method, uri, null, thrownException) } } @@ -364,7 +353,7 @@ abstract class HttpClientTest extends AgentTestRunner { assertTraces(1) { trace(0, 2 + extraClientSpans()) { basicSpan(it, 0, "parent", null, thrownException) - clientSpan(it, 1, span(0), method, false, uri, null, thrownException) + clientSpan(it, 1, span(0), method, uri, null, thrownException) } } @@ -388,7 +377,7 @@ abstract class HttpClientTest extends AgentTestRunner { assertTraces(1) { trace(0, 2 + extraClientSpans()) { basicSpan(it, 0, "parent", null, thrownException) - clientSpan(it, 1, span(0), method, false, uri, null, thrownException) + clientSpan(it, 1, span(0), method, uri, null, thrownException) } } @@ -410,7 +399,7 @@ abstract class HttpClientTest extends AgentTestRunner { status == 200 assertTraces(1) { trace(0, 1 + extraClientSpans()) { - clientSpan(it, 0, null, method, false, uri) + clientSpan(it, 0, null, method, uri) } } @@ -419,7 +408,7 @@ abstract class HttpClientTest extends AgentTestRunner { } // parent span must be cast otherwise it breaks debugging classloading (junit loads it early) - void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", boolean tagQueryString = false, URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null) { + void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null, String httpFlavor = "1.1") { def userAgent = userAgent() trace.span(index) { if (parentSpan == null) { @@ -437,20 +426,16 @@ abstract class HttpClientTest extends AgentTestRunner { "${SemanticAttributes.NET_TRANSPORT.key()}" "IP.TCP" "${SemanticAttributes.NET_PEER_NAME.key()}" uri.host "${SemanticAttributes.NET_PEER_IP.key()}" { it == null || it == "127.0.0.1" } // Optional - // Optional "${SemanticAttributes.NET_PEER_PORT.key()}" uri.port > 0 ? uri.port : { it == null || it == 443 } "${SemanticAttributes.HTTP_URL.key()}" { it == "${uri}" || it == "${removeFragment(uri)}" } "${SemanticAttributes.HTTP_METHOD.key()}" method + "${SemanticAttributes.HTTP_FLAVOR.key()}" httpFlavor if (userAgent) { "${SemanticAttributes.HTTP_USER_AGENT.key()}" { it.startsWith(userAgent) } } if (status) { "${SemanticAttributes.HTTP_STATUS_CODE.key()}" status } - if (tagQueryString) { - "$MoreAttributes.HTTP_QUERY" uri.query - "$MoreAttributes.HTTP_FRAGMENT" { it == null || it == uri.fragment } // Optional - } } } }