From 608284f8ac68c4f13579670c116a7d16e7a431cf Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 16:34:19 -0700 Subject: [PATCH 01/23] Remove url from HttpServerAttributesExtractor --- .../http/HttpServerAttributesExtractor.java | 7 ---- .../HttpServerAttributesExtractorTest.java | 7 ---- .../ArmeriaHttpServerAttributesExtractor.java | 5 --- .../v1_3/AbstractArmeriaHttpServerTest.groovy | 3 -- .../RatpackHttpAttributesExtractor.java | 41 +++++++++---------- .../server/RatpackAsyncHttpServerTest.groovy | 3 +- .../server/RatpackForkedHttpServerTest.groovy | 3 +- .../server/RatpackHttpServerTest.groovy | 3 +- .../server/AbstractRatpackRoutesTest.groovy | 14 ++----- .../v1_0/RestletHttpAttributesExtractor.java | 10 ++--- .../v1_0/AbstractRestletServerTest.groovy | 4 +- .../ServletHttpAttributesExtractor.java | 27 ++---------- .../common/TomcatHttpAttributesExtractor.java | 26 ++++-------- .../test/base/HttpServerTest.groovy | 28 ++++--------- 14 files changed, 49 insertions(+), 132 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java index 57014bd748f7..83d3ae78ff4b 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java @@ -31,9 +31,6 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) { set(attributes, SemanticAttributes.HTTP_HOST, host(request)); set(attributes, SemanticAttributes.HTTP_TARGET, target(request)); set(attributes, SemanticAttributes.HTTP_ROUTE, route(request)); - - // TODO: this is specific to clients, should we remove this? - set(attributes, SemanticAttributes.HTTP_URL, url(request)); } @Override @@ -49,10 +46,6 @@ protected final void onEnd( // Attributes that always exist in a request - // TODO: this is specific to clients, should we remove this? - @Nullable - protected abstract String url(REQUEST request); - @Nullable protected abstract String target(REQUEST request); diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java index f718a0d4d1b2..bd9838735414 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java @@ -25,11 +25,6 @@ protected String method(Map request) { return request.get("method"); } - @Override - protected String url(Map request) { - return request.get("url"); - } - @Override protected String target(Map request) { return request.get("target"); @@ -120,7 +115,6 @@ void normal() { assertThat(attributes.build()) .containsOnly( entry(SemanticAttributes.HTTP_METHOD, "POST"), - entry(SemanticAttributes.HTTP_URL, "http://github.com"), entry(SemanticAttributes.HTTP_TARGET, "github.com"), entry(SemanticAttributes.HTTP_HOST, "github.com:80"), entry(SemanticAttributes.HTTP_SCHEME, "https"), @@ -131,7 +125,6 @@ void normal() { assertThat(attributes.build()) .containsOnly( entry(SemanticAttributes.HTTP_METHOD, "POST"), - entry(SemanticAttributes.HTTP_URL, "http://github.com"), entry(SemanticAttributes.HTTP_TARGET, "github.com"), entry(SemanticAttributes.HTTP_HOST, "github.com:80"), entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"), diff --git a/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java b/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java index 0572e91a99f1..5c58d903d5f1 100644 --- a/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java +++ b/instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpServerAttributesExtractor.java @@ -24,11 +24,6 @@ protected String method(RequestContext ctx) { return ctx.method().name(); } - @Override - protected String url(RequestContext ctx) { - return request(ctx).uri().toString(); - } - @Override protected String target(RequestContext ctx) { return request(ctx).path(); diff --git a/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy b/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy index 3749bb898041..ce921dbbd8ea 100644 --- a/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy +++ b/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy @@ -38,13 +38,10 @@ abstract class AbstractArmeriaHttpServerTest extends HttpServerTest { @Override List> extraAttributes() { [ - SemanticAttributes.HTTP_HOST, SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH, SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH, SemanticAttributes.HTTP_ROUTE, - SemanticAttributes.HTTP_SCHEME, SemanticAttributes.HTTP_SERVER_NAME, - SemanticAttributes.HTTP_TARGET, SemanticAttributes.NET_PEER_NAME, SemanticAttributes.NET_TRANSPORT ] diff --git a/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java b/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java index 54c4c811936f..a07b0bd97b73 100644 --- a/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java +++ b/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java @@ -7,6 +7,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; +import java.net.URI; import org.checkerframework.checker.nullness.qual.Nullable; import ratpack.handling.Context; import ratpack.http.Request; @@ -20,11 +21,15 @@ protected String method(Request request) { return request.getMethod().getName(); } + @Override + protected String target(Request request) { + // Uri is the path + query string, not a full URL + return request.getUri(); + } + @Override @Nullable - protected String url(Request request) { - // TODO(anuraaga): We should probably just not fill this - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/3700 + protected String host(Request request) { Context ratpackContext = request.get(Context.class); if (ratpackContext == null) { return null; @@ -33,24 +38,8 @@ protected String url(Request request) { if (publicAddress == null) { return null; } - return publicAddress - .builder() - .path(request.getPath()) - .params(request.getQueryParams()) - .build() - .toString(); - } - - @Override - protected String target(Request request) { - // Uri is the path + query string, not a full URL - return request.getUri(); - } - - @Override - @Nullable - protected String host(Request request) { - return null; + URI uri = publicAddress.builder().build(); + return uri.getHost() + ":" + uri.getPort(); } @Override @@ -63,7 +52,15 @@ protected String route(Request request) { @Override @Nullable protected String scheme(Request request) { - return null; + Context ratpackContext = request.get(Context.class); + if (ratpackContext == null) { + return null; + } + PublicAddress publicAddress = ratpackContext.get(PublicAddress.class); + if (publicAddress == null) { + return null; + } + return publicAddress.builder().build().getScheme(); } @Override diff --git a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackAsyncHttpServerTest.groovy b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackAsyncHttpServerTest.groovy index 15e71eb022ef..69e97514c13e 100644 --- a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackAsyncHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackAsyncHttpServerTest.groovy @@ -29,8 +29,7 @@ class RatpackAsyncHttpServerTest extends AbstractRatpackAsyncHttpServerTest impl List> extraAttributes() { return [ SemanticAttributes.HTTP_ROUTE, - SemanticAttributes.HTTP_TARGET, - SemanticAttributes.NET_TRANSPORT, + SemanticAttributes.NET_TRANSPORT ] } } diff --git a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackForkedHttpServerTest.groovy b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackForkedHttpServerTest.groovy index a5f3445247df..038eff44ef56 100644 --- a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackForkedHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackForkedHttpServerTest.groovy @@ -29,8 +29,7 @@ class RatpackForkedHttpServerTest extends AbstractRatpackForkedHttpServerTest im List> extraAttributes() { return [ SemanticAttributes.HTTP_ROUTE, - SemanticAttributes.HTTP_TARGET, - SemanticAttributes.NET_TRANSPORT, + SemanticAttributes.NET_TRANSPORT ] } } diff --git a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackHttpServerTest.groovy b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackHttpServerTest.groovy index 19190f6fd784..713cabf9c503 100644 --- a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackHttpServerTest.groovy @@ -29,8 +29,7 @@ class RatpackHttpServerTest extends AbstractRatpackHttpServerTest implements Lib List> extraAttributes() { return [ SemanticAttributes.HTTP_ROUTE, - SemanticAttributes.HTTP_TARGET, - SemanticAttributes.NET_TRANSPORT, + SemanticAttributes.NET_TRANSPORT ] } } diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy index f6a575da2f1b..0d42b6b2e7f1 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy @@ -104,15 +104,15 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:${app.bindPort}/${path}" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" "${SemanticAttributes.HTTP_USER_AGENT.key}" String - if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { - "${SemanticAttributes.HTTP_HOST}" "localhost:${app.bindPort}" - } + "${SemanticAttributes.HTTP_HOST}" "localhost:${app.bindPort}" + "${SemanticAttributes.HTTP_SCHEME}" "http" + "${SemanticAttributes.HTTP_TARGET}" "/$path" + if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long } @@ -124,15 +124,9 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification { // currently reports '/*' which is a fallback route. "${SemanticAttributes.HTTP_ROUTE}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { - "${SemanticAttributes.HTTP_SCHEME}" "http" - } if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) { "${SemanticAttributes.HTTP_SERVER_NAME}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { - "${SemanticAttributes.HTTP_TARGET}" "/$path" - } if (extraAttributes.contains(SemanticAttributes.NET_PEER_NAME)) { "${SemanticAttributes.NET_PEER_NAME}" "localhost" } diff --git a/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java b/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java index de6f386005cd..c0dc62b77578 100644 --- a/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java +++ b/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java @@ -20,11 +20,6 @@ protected String method(Request request) { return request.getMethod().toString(); } - @Override - protected String url(Request request) { - return request.getOriginalRef().toString(); - } - @Override protected @Nullable String target(Request request) { Reference ref = request.getOriginalRef(); @@ -33,8 +28,9 @@ protected String url(Request request) { } @Override - protected @Nullable String host(Request request) { - return null; + protected String host(Request request) { + Reference originalRef = request.getOriginalRef(); + return originalRef.getHostDomain() + ":" + originalRef.getHostPort(); } @Override diff --git a/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy b/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy index 677e62d349c1..9c0433b078cf 100644 --- a/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy +++ b/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy @@ -136,9 +136,7 @@ abstract class AbstractRestletServerTest extends HttpServerTest { @Override List> extraAttributes() { [ - SemanticAttributes.HTTP_TARGET, - SemanticAttributes.HTTP_SCHEME, - SemanticAttributes.NET_TRANSPORT, + SemanticAttributes.NET_TRANSPORT ] } diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java index b004e8e73f95..fd2bff7727d5 100644 --- a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHttpAttributesExtractor.java @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.servlet; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor; -import io.opentelemetry.instrumentation.api.internal.UriBuilder; import io.opentelemetry.instrumentation.servlet.ServletAccessor; import org.checkerframework.checker.nullness.qual.Nullable; @@ -24,44 +23,26 @@ public ServletHttpAttributesExtractor(ServletAccessor accesso return accessor.getRequestMethod(requestContext.request()); } - @Override - protected @Nullable String url(ServletRequestContext requestContext) { - REQUEST request = requestContext.request(); - - return UriBuilder.uri( - accessor.getRequestScheme(request), - accessor.getRequestServerName(request), - accessor.getRequestServerPort(request), - accessor.getRequestUri(request), - accessor.getRequestQueryString(request)); - } - @Override protected @Nullable String target(ServletRequestContext requestContext) { - /* - String target = httpServletRequest.getRequestURI(); - String queryString = httpServletRequest.getQueryString(); + REQUEST request = requestContext.request(); + String target = accessor.getRequestUri(request); + String queryString = accessor.getRequestQueryString(request); if (queryString != null) { target += "?" + queryString; } return target; - */ - return null; } @Override protected @Nullable String host(ServletRequestContext requestContext) { - /* REQUEST request = requestContext.request(); return accessor.getRequestServerName(request) + ":" + accessor.getRequestServerPort(request); - */ - return null; } @Override protected @Nullable String scheme(ServletRequestContext requestContext) { - // return accessor.getRequestScheme(requestContext.request()); - return null; + return accessor.getRequestScheme(requestContext.request()); } @Override diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java index 7b17639a9dbe..da583bf25b23 100644 --- a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java +++ b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesExtractor.java @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.tomcat.common; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor; -import io.opentelemetry.instrumentation.api.internal.UriBuilder; import org.apache.coyote.Request; import org.apache.coyote.Response; import org.apache.tomcat.util.buf.MessageBytes; @@ -20,36 +19,25 @@ protected String method(Request request) { return request.method().toString(); } - @Override - protected String url(Request request) { - MessageBytes schemeMessageBytes = request.scheme(); - String scheme = schemeMessageBytes.isNull() ? "http" : schemeMessageBytes.toString(); - String host = request.serverName().toString(); - int serverPort = request.getServerPort(); - String path = request.requestURI().toString(); - String query = request.queryString().toString(); - - return UriBuilder.uri(scheme, host, serverPort, path, query); - } - @Override protected @Nullable String target(Request request) { - return null; + String target = request.requestURI().toString(); + String queryString = request.queryString().toString(); + if (queryString != null) { + target += "?" + queryString; + } + return target; } @Override protected @Nullable String host(Request request) { - // return request.serverName().toString() + ":" + request.getServerPort(); - return null; + return request.serverName().toString() + ":" + request.getServerPort(); } @Override protected @Nullable String scheme(Request request) { - /* MessageBytes schemeMessageBytes = request.scheme(); return schemeMessageBytes.isNull() ? "http" : schemeMessageBytes.toString(); - */ - return null; } @Override diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy index 5e74effbce4d..199560c07813 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy @@ -569,15 +569,15 @@ abstract class HttpServerTest extends InstrumentationSpecification imple "${SemanticAttributes.NET_PEER_PORT.key}" { it == null || it instanceof Long } "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } // Optional "${SemanticAttributes.HTTP_CLIENT_IP.key}" { it == null || it == TEST_CLIENT_IP } - "${SemanticAttributes.HTTP_URL.key}" { it == "${endpoint.resolve(address)}" || it == "${endpoint.resolveWithoutFragment(address)}" } "${SemanticAttributes.HTTP_METHOD.key}" method "${SemanticAttributes.HTTP_STATUS_CODE.key}" endpoint.status "${SemanticAttributes.HTTP_FLAVOR.key}" { it == "1.1" || it == "2.0" } "${SemanticAttributes.HTTP_USER_AGENT.key}" TEST_USER_AGENT - if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { - "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" - } + "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" + "${SemanticAttributes.HTTP_SCHEME}" "http" + "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "${endpoint == QUERY_PARAM ? "?${endpoint.body}" : ""}" + if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long } @@ -589,15 +589,9 @@ abstract class HttpServerTest extends InstrumentationSpecification imple // currently reports '/*' which is a fallback route. "${SemanticAttributes.HTTP_ROUTE}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { - "${SemanticAttributes.HTTP_SCHEME}" "http" - } if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) { "${SemanticAttributes.HTTP_SERVER_NAME}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { - "${SemanticAttributes.HTTP_TARGET}" endpoint.path + "${endpoint == QUERY_PARAM ? "?${endpoint.body}" : ""}" - } if (extraAttributes.contains(SemanticAttributes.NET_PEER_NAME)) { // "localhost" on linux, "127.0.0.1" on windows "${SemanticAttributes.NET_PEER_NAME.key}" { it == "localhost" || it == "127.0.0.1" } @@ -620,15 +614,15 @@ abstract class HttpServerTest extends InstrumentationSpecification imple "${SemanticAttributes.NET_PEER_PORT.key}" { it == null || it instanceof Long } "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } // Optional "${SemanticAttributes.HTTP_CLIENT_IP.key}" { it == null || it == TEST_CLIENT_IP } - "${SemanticAttributes.HTTP_URL.key}" endpoint.resolve(address).toString() + "?id=$requestId" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" "${SemanticAttributes.HTTP_USER_AGENT.key}" TEST_USER_AGENT - if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { - "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" - } + "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" + "${SemanticAttributes.HTTP_SCHEME}" "http" + "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "?id=$requestId" + if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long } @@ -640,15 +634,9 @@ abstract class HttpServerTest extends InstrumentationSpecification imple // currently reports '/*' which is a fallback route. "${SemanticAttributes.HTTP_ROUTE}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { - "${SemanticAttributes.HTTP_SCHEME}" "http" - } if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) { "${SemanticAttributes.HTTP_SERVER_NAME}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { - "${SemanticAttributes.HTTP_TARGET}" endpoint.path + "?id=$requestId" - } if (extraAttributes.contains(SemanticAttributes.NET_PEER_NAME)) { "${SemanticAttributes.NET_PEER_NAME}" "localhost" } From 3b15a79b539d032e7614acfcb8b3202cf4ceea62 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 17:10:12 -0700 Subject: [PATCH 02/23] Remove UriBuilder --- .../api/internal/UriBuilder.java | 53 ---------------- .../api/internal/UriBuilderTest.java | 63 ------------------- 2 files changed, 116 deletions(-) delete mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/UriBuilder.java delete mode 100644 instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/UriBuilderTest.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/UriBuilder.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/UriBuilder.java deleted file mode 100644 index 4c5ac0302567..000000000000 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/UriBuilder.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.internal; - -import org.checkerframework.checker.nullness.qual.Nullable; - -// internal until decisions made on -// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/3700 -public class UriBuilder { - - // TODO (trask) investigate and document implications of URI encoding, see - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/4008#discussion_r698027851 - // - // note: currently path must be empty or start with "/" but that can be relaxed if needed - public static String uri( - String scheme, String host, int serverPort, String path, @Nullable String query) { - - boolean isDefaultPort = - (scheme.equals("http") && serverPort == 80) - || (scheme.equals("https") && serverPort == 443); - - // +3 is space for "://" - int length = scheme.length() + 3 + host.length() + path.length(); - if (!isDefaultPort && serverPort != -1) { - // +6 is space for ":" and max port number (65535) - length += 6; - } - if (query != null) { - // the +1 is space for "?" - length += 1 + query.length(); - } - - StringBuilder url = new StringBuilder(length); - url.append(scheme); - url.append("://"); - url.append(host); - if (!isDefaultPort && serverPort != -1) { - url.append(':'); - url.append(serverPort); - } - url.append(path); - if (query != null) { - url.append('?'); - url.append(query); - } - return url.toString(); - } - - private UriBuilder() {} -} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/UriBuilderTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/UriBuilderTest.java deleted file mode 100644 index 791e54bc2e83..000000000000 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/UriBuilderTest.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.internal; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.net.URI; -import java.net.URISyntaxException; -import java.util.stream.Stream; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; - -public class UriBuilderTest { - - @ParameterizedTest - @ArgumentsSource(Parameters.class) - public void test(String scheme, String host, int port, String path, String query) - throws URISyntaxException { - - assertThat(UriBuilder.uri(scheme, host, port, path, query)) - .isEqualTo(new URI(scheme, null, host, port, path, query, null).toString()); - } - - // can't use parameterized test above because URI.toString() encodes the port when it is supplied, - // even it's the default port - @Test - public void testHttpDefaultPort() { - assertThat(UriBuilder.uri("http", "myhost", 80, "/mypath", "myquery")) - .isEqualTo("http://myhost/mypath?myquery"); - } - - // can't use parameterized test above because URI.toString() encodes the port when it is supplied, - // even it's the default port - @Test - public void testHttpsDefaultPort() { - assertThat(UriBuilder.uri("https", "myhost", 443, "/mypath", "myquery")) - .isEqualTo("https://myhost/mypath?myquery"); - } - - private static class Parameters implements ArgumentsProvider { - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of( - Arguments.of("http", "myhost", -1, "/mypath", "myquery"), // test default http port - Arguments.of("http", "myhost", 8080, "/mypath", "myquery"), // test non-default http port - Arguments.of("https", "myhost", -1, "/mypath", "myquery"), // test default https port - Arguments.of( - "https", "myhost", 8443, "/mypath", "myquery"), // test non-default https port - Arguments.of("http", "myhost", -1, "/", "myquery"), // test root path - Arguments.of("http", "myhost", -1, "", "myquery"), // test empty path - Arguments.of("http", "myhost", -1, "/mypath", ""), // test empty query string - Arguments.of("http", "myhost", -1, "/mypath", null) // test null query string - ); - } - } -} From 08d982653babea79c331aaa6aa6203cd95e97705 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 17:42:26 -0700 Subject: [PATCH 03/23] Tracers too --- .../api/tracer/HttpServerTracer.java | 39 ++++++++++--------- .../akkahttp/server/AkkaHttpServerTracer.java | 22 ++++++++++- .../src/test/groovy/DropwizardTest.groovy | 4 +- .../grizzly/GrizzlyHttpServerTracer.java | 33 ++++++++-------- .../dispatcher/LibertyDispatcherTracer.java | 32 +++++++-------- .../v3_8/server/NettyHttpServerTracer.java | 19 +++++++++ .../src/test/groovy/Netty38ServerTest.groovy | 10 +++++ .../v4_0/server/NettyHttpServerTracer.java | 19 +++++++++ .../src/test/groovy/Netty40ServerTest.groovy | 9 +++++ .../v4_1/server/NettyHttpServerTracer.java | 19 +++++++++ .../src/test/groovy/Netty41ServerTest.groovy | 9 +++++ .../servlet/ServletHttpServerTracer.java | 34 ++++++++-------- .../webmvc/SpringWebMvcServerTracer.java | 19 ++++++++- .../undertow/UndertowHttpServerTracer.java | 22 ++++++++--- .../test/base/HttpServerTest.groovy | 22 ++++++++--- 15 files changed, 227 insertions(+), 85 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java index 659739f65fe1..23d21db29bce 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java @@ -169,24 +169,15 @@ protected void onRequest(SpanBuilder spanBuilder, REQUEST request) { spanBuilder.setAttribute( SemanticAttributes.HTTP_USER_AGENT, requestHeader(request, USER_AGENT)); - setUrl(spanBuilder, request); - - // TODO set resource name from URL. - } - - /* - https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md - - HTTP semantic convention recommends setting http.scheme, http.host, http.target attributes - instead of http.url because it "is usually not readily available on the server side but would have - to be assembled in a cumbersome and sometimes lossy process from other information". - - But in Java world there is no standard way to access "The full request target as passed in a HTTP request line or equivalent" - which is the recommended value for http.target attribute. Therefore we cannot use any of the - recommended combinations of attributes and are forced to use http.url. - */ - private void setUrl(SpanBuilder spanBuilder, REQUEST request) { - spanBuilder.setAttribute(SemanticAttributes.HTTP_URL, url(request)); + String url = url(request); + if (url != null) { + // netty instrumentation uses this + spanBuilder.setAttribute(SemanticAttributes.HTTP_URL, url); + } else { + spanBuilder.setAttribute(SemanticAttributes.HTTP_SCHEME, scheme(request)); + spanBuilder.setAttribute(SemanticAttributes.HTTP_HOST, host(request)); + spanBuilder.setAttribute(SemanticAttributes.HTTP_TARGET, target(request)); + } } protected void onConnectionAndRequest( @@ -297,7 +288,17 @@ private static void setStatus(Span span, int status) { protected abstract TextMapGetter getGetter(); - protected abstract String url(REQUEST request); + // netty still uses this, otherwise should prefer scheme/host/target + @Nullable + protected String url(REQUEST request) { + return null; + } + + protected abstract String scheme(REQUEST request); + + protected abstract String host(REQUEST request); + + protected abstract String target(REQUEST request); protected abstract String method(REQUEST request); diff --git a/instrumentation/akka-http-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/server/AkkaHttpServerTracer.java b/instrumentation/akka-http-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/server/AkkaHttpServerTracer.java index 067a478f14d2..468fda5b0926 100644 --- a/instrumentation/akka-http-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/server/AkkaHttpServerTracer.java +++ b/instrumentation/akka-http-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/server/AkkaHttpServerTracer.java @@ -8,9 +8,11 @@ import akka.http.javadsl.model.HttpHeader; import akka.http.scaladsl.model.HttpRequest; import akka.http.scaladsl.model.HttpResponse; +import akka.http.scaladsl.model.Uri; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; +import scala.Option; public class AkkaHttpServerTracer extends HttpServerTracer { @@ -44,8 +46,24 @@ public Context getServerContext(Void none) { } @Override - protected String url(HttpRequest httpRequest) { - return httpRequest.uri().toString(); + protected String scheme(HttpRequest httpRequest) { + return httpRequest.uri().scheme(); + } + + @Override + protected String host(HttpRequest httpRequest) { + Uri.Authority authority = httpRequest.uri().authority(); + return authority.host().address() + ":" + authority.port(); + } + + @Override + protected String target(HttpRequest httpRequest) { + String target = httpRequest.uri().path().toString(); + Option queryString = httpRequest.uri().rawQueryString(); + if (queryString.isDefined()) { + target += "?" + queryString.get(); + } + return target; } @Override diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy index f1178460f071..3d3306ccb753 100644 --- a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy +++ b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy @@ -113,7 +113,9 @@ class DropwizardTest extends HttpServerTest implements Ag // dropwizard reports peer ip as the client ip "${SemanticAttributes.NET_PEER_IP.key}" TEST_CLIENT_IP "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" { it == "${endpoint.resolve(address)}" || it == "${endpoint.resolveWithoutFragment(address)}" } + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" + "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "${endpoint == QUERY_PARAM ? "?${endpoint.body}" : ""}" "${SemanticAttributes.HTTP_METHOD.key}" method "${SemanticAttributes.HTTP_STATUS_CODE.key}" endpoint.status "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" diff --git a/instrumentation/grizzly-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyHttpServerTracer.java b/instrumentation/grizzly-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyHttpServerTracer.java index 0fe374b44d57..dbe27c605a10 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyHttpServerTracer.java +++ b/instrumentation/grizzly-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyHttpServerTracer.java @@ -8,8 +8,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; -import java.net.URI; -import java.net.URISyntaxException; import org.glassfish.grizzly.filterchain.FilterChainContext; import org.glassfish.grizzly.http.HttpRequestPacket; import org.glassfish.grizzly.http.HttpResponsePacket; @@ -55,22 +53,23 @@ public Context getServerContext(FilterChainContext filterChainContext) { } @Override - protected String url(HttpRequestPacket httpRequest) { - try { - return new URI( - (httpRequest.isSecure() ? "https://" : "http://") - + httpRequest.serverName() - + ":" - + httpRequest.getServerPort() - + httpRequest.getRequestURI() - + (httpRequest.getQueryString() != null - ? "?" + httpRequest.getQueryString() - : "")) - .toString(); - } catch (URISyntaxException e) { - logger.warn("Failed to construct request URI", e); - return null; + protected String scheme(HttpRequestPacket httpRequest) { + return httpRequest.isSecure() ? "https" : "http"; + } + + @Override + protected String host(HttpRequestPacket httpRequest) { + return httpRequest.serverName() + ":" + httpRequest.getServerPort(); + } + + @Override + protected String target(HttpRequestPacket httpRequest) { + String target = httpRequest.getRequestURI(); + String queryString = httpRequest.getQueryString(); + if (queryString != null) { + target += "?" + queryString; } + return target; } @Override diff --git a/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherTracer.java b/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherTracer.java index 14094f5fbede..9ffe94141d14 100644 --- a/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherTracer.java +++ b/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherTracer.java @@ -8,8 +8,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; -import java.net.URI; -import java.net.URISyntaxException; import org.checkerframework.checker.nullness.qual.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -68,21 +66,23 @@ protected TextMapGetter getGetter() { } @Override - protected String url(LibertyRequestWrapper libertyRequestWrapper) { - try { - return new URI( - libertyRequestWrapper.getScheme(), - null, - libertyRequestWrapper.getServerName(), - libertyRequestWrapper.getServerPort(), - libertyRequestWrapper.getRequestUri(), - libertyRequestWrapper.getQueryString(), - null) - .toString(); - } catch (URISyntaxException e) { - logger.debug("Failed to construct request URI", e); - return null; + protected String scheme(LibertyRequestWrapper libertyRequestWrapper) { + return libertyRequestWrapper.getScheme(); + } + + @Override + protected String host(LibertyRequestWrapper libertyRequestWrapper) { + return libertyRequestWrapper.getServerName() + ":" + libertyRequestWrapper.getServerPort(); + } + + @Override + protected String target(LibertyRequestWrapper libertyRequestWrapper) { + String target = libertyRequestWrapper.getRequestUri(); + String queryString = libertyRequestWrapper.getQueryString(); + if (queryString != null) { + target += "?" + queryString; } + return target; } @Override diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpServerTracer.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpServerTracer.java index 5c4e14b9ad60..597ba1fd411d 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpServerTracer.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpServerTracer.java @@ -13,6 +13,7 @@ import io.opentelemetry.javaagent.instrumentation.netty.v3_8.ChannelTraceContext; import java.net.InetSocketAddress; import java.net.SocketAddress; +import org.checkerframework.checker.nullness.qual.Nullable; import org.jboss.netty.channel.Channel; import org.jboss.netty.handler.codec.http.HttpRequest; import org.jboss.netty.handler.codec.http.HttpResponse; @@ -60,6 +61,24 @@ protected String url(HttpRequest request) { } } + @Override + @Nullable + protected String scheme(HttpRequest request) { + return null; + } + + @Override + @Nullable + protected String host(HttpRequest request) { + return null; + } + + @Override + @Nullable + protected String target(HttpRequest request) { + return null; + } + @Override protected String peerHostIp(Channel channel) { SocketAddress socketAddress = channel.getRemoteAddress(); diff --git a/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy b/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy index 5d3d908bbfd9..83c4cf43b685 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy +++ b/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy @@ -3,8 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ + +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import org.jboss.netty.bootstrap.ServerBootstrap import org.jboss.netty.buffer.ChannelBuffer import org.jboss.netty.buffer.ChannelBuffers @@ -160,4 +163,11 @@ class Netty38ServerTest extends HttpServerTest implements Agent boolean testConcurrency() { return true } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java index 7399f707d9ca..d89a8a661d5c 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java @@ -17,6 +17,7 @@ import io.opentelemetry.javaagent.instrumentation.netty.v4_0.AttributeKeys; import java.net.InetSocketAddress; import java.net.SocketAddress; +import org.checkerframework.checker.nullness.qual.Nullable; public class NettyHttpServerTracer extends HttpServerTracer { @@ -71,6 +72,24 @@ protected String url(HttpRequest request) { } } + @Override + @Nullable + protected String scheme(HttpRequest request) { + return null; + } + + @Override + @Nullable + protected String host(HttpRequest request) { + return null; + } + + @Override + @Nullable + protected String target(HttpRequest request) { + return null; + } + @Override protected String peerHostIp(Channel channel) { SocketAddress socketAddress = channel.remoteAddress(); diff --git a/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy b/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy index a6e1200f1655..16a2ae95bb33 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy +++ b/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy @@ -24,8 +24,10 @@ import io.netty.handler.codec.http.QueryStringDecoder import io.netty.handler.logging.LogLevel import io.netty.handler.logging.LoggingHandler import io.netty.util.CharsetUtil +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_LENGTH import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE @@ -130,4 +132,11 @@ class Netty40ServerTest extends HttpServerTest implements AgentT boolean testConcurrency() { return true } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java index d70453df0cab..bb80aa507d35 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java @@ -17,6 +17,7 @@ import io.opentelemetry.javaagent.instrumentation.netty.common.server.NettyRequestExtractAdapter; import java.net.InetSocketAddress; import java.net.SocketAddress; +import org.checkerframework.checker.nullness.qual.Nullable; public class NettyHttpServerTracer extends HttpServerTracer { @@ -71,6 +72,24 @@ protected String url(HttpRequest request) { } } + @Override + @Nullable + protected String scheme(HttpRequest request) { + return null; + } + + @Override + @Nullable + protected String host(HttpRequest request) { + return null; + } + + @Override + @Nullable + protected String target(HttpRequest request) { + return null; + } + @Override protected String peerHostIp(Channel channel) { SocketAddress socketAddress = channel.remoteAddress(); diff --git a/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ServerTest.groovy b/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ServerTest.groovy index 83f311443dda..d88ad47e8c31 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ServerTest.groovy +++ b/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ServerTest.groovy @@ -23,8 +23,10 @@ import io.netty.handler.codec.http.QueryStringDecoder import io.netty.handler.logging.LogLevel import io.netty.handler.logging.LoggingHandler import io.netty.util.CharsetUtil +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE @@ -129,4 +131,11 @@ class Netty41ServerTest extends HttpServerTest implements AgentT boolean testConcurrency() { return true } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } diff --git a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java index f71dbb609655..a5b34ac42071 100644 --- a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java +++ b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java @@ -20,8 +20,6 @@ import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; -import java.net.URI; -import java.net.URISyntaxException; import java.security.Principal; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; @@ -92,21 +90,25 @@ public void endExceptionally( } @Override - protected String url(REQUEST httpServletRequest) { - try { - return new URI( - accessor.getRequestScheme(httpServletRequest), - null, - accessor.getRequestServerName(httpServletRequest), - accessor.getRequestServerPort(httpServletRequest), - accessor.getRequestUri(httpServletRequest), - accessor.getRequestQueryString(httpServletRequest), - null) - .toString(); - } catch (URISyntaxException e) { - logger.debug("Failed to construct request URI", e); - return null; + protected String scheme(REQUEST httpServletRequest) { + return accessor.getRequestScheme(httpServletRequest); + } + + @Override + protected String host(REQUEST httpServletRequest) { + return accessor.getRequestServerName(httpServletRequest) + + ":" + + accessor.getRequestServerPort(httpServletRequest); + } + + @Override + protected String target(REQUEST httpServletRequest) { + String target = accessor.getRequestUri(httpServletRequest); + String queryString = accessor.getRequestQueryString(httpServletRequest); + if (queryString != null) { + target += "?" + queryString; } + return target; } @Override diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcServerTracer.java b/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcServerTracer.java index 58cc54a6bd07..0f008c8f4bac 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcServerTracer.java +++ b/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcServerTracer.java @@ -37,8 +37,23 @@ protected TextMapGetter getGetter() { } @Override - protected String url(HttpServletRequest request) { - return request.getRequestURI(); + protected String scheme(HttpServletRequest request) { + return request.getScheme(); + } + + @Override + protected String host(HttpServletRequest request) { + return request.getServerName() + ":" + request.getServerPort(); + } + + @Override + protected String target(HttpServletRequest request) { + String target = request.getPathInfo(); + String queryString = request.getQueryString(); + if (queryString != null) { + target += "?" + queryString; + } + return target; } @Override diff --git a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java index fbb7621e68eb..2a873a50ea02 100644 --- a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java +++ b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java @@ -124,13 +124,23 @@ protected TextMapGetter getGetter() { } @Override - protected String url(HttpServerExchange exchange) { - String result = exchange.getRequestURL(); - if (exchange.getQueryString() == null || exchange.getQueryString().isEmpty()) { - return result; - } else { - return result + "?" + exchange.getQueryString(); + protected String scheme(HttpServerExchange exchange) { + return exchange.getRequestScheme(); + } + + @Override + protected String host(HttpServerExchange exchange) { + return exchange.getHostAndPort(); + } + + @Override + protected String target(HttpServerExchange exchange) { + String target = exchange.getRequestPath(); + String queryString = exchange.getQueryString(); + if (queryString != null) { + target += "?" + queryString; } + return target; } @Override diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy index 199560c07813..3ef55831fa55 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy @@ -574,9 +574,14 @@ abstract class HttpServerTest extends InstrumentationSpecification imple "${SemanticAttributes.HTTP_FLAVOR.key}" { it == "1.1" || it == "2.0" } "${SemanticAttributes.HTTP_USER_AGENT.key}" TEST_USER_AGENT - "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" - "${SemanticAttributes.HTTP_SCHEME}" "http" - "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "${endpoint == QUERY_PARAM ? "?${endpoint.body}" : ""}" + if (extraAttributes.contains(SemanticAttributes.HTTP_URL)) { + // netty instrumentation uses this + "${SemanticAttributes.HTTP_URL.key}" { it == "${endpoint.resolve(address)}" || it == "${endpoint.resolveWithoutFragment(address)}" } + } else { + "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" + "${SemanticAttributes.HTTP_SCHEME}" "http" + "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "${endpoint == QUERY_PARAM ? "?${endpoint.body}" : ""}" + } if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long @@ -619,9 +624,14 @@ abstract class HttpServerTest extends InstrumentationSpecification imple "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" "${SemanticAttributes.HTTP_USER_AGENT.key}" TEST_USER_AGENT - "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" - "${SemanticAttributes.HTTP_SCHEME}" "http" - "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "?id=$requestId" + if (extraAttributes.contains(SemanticAttributes.HTTP_URL)) { + // netty instrumentation uses this + "${SemanticAttributes.HTTP_URL.key}" endpoint.resolve(address).toString() + "?id=$requestId" + } else { + "${SemanticAttributes.HTTP_HOST}" "localhost:${port}" + "${SemanticAttributes.HTTP_SCHEME}" "http" + "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "?id=$requestId" + } if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long From 113a26f3ec2413b78c957126831337a933e6d1f7 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 20:01:36 -0700 Subject: [PATCH 04/23] apache-camel --- .../instrumentation/apachecamel/RestCamelTest.groovy | 4 +++- .../apachecamel/TwoServicesWithDirectClientCamelTest.groovy | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy b/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy index 3d89f3fb6b54..6ba1da5f2c82 100644 --- a/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy +++ b/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy @@ -87,7 +87,9 @@ class RestCamelTest extends AgentInstrumentationSpecification implements RetryOn kind SERVER parentSpanId(span(1).spanId) attributes { - "$SemanticAttributes.HTTP_URL.key" "http://localhost:$port/api/firstModule/unit/unitOne" + "$SemanticAttributes.HTTP_SCHEME.key" "http" + "$SemanticAttributes.HTTP_HOST.key" "localhost:$port" + "$SemanticAttributes.HTTP_TARGET.key" "/api/firstModule/unit/unitOne" "$SemanticAttributes.HTTP_STATUS_CODE.key" 200 "$SemanticAttributes.HTTP_USER_AGENT.key" String "$SemanticAttributes.HTTP_FLAVOR.key" "1.1" diff --git a/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy b/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy index a4955b32cc90..79d251e2a8f8 100644 --- a/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy +++ b/instrumentation/apache-camel-2.20/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy @@ -124,7 +124,9 @@ class TwoServicesWithDirectClientCamelTest extends AgentInstrumentationSpecifica attributes { "$SemanticAttributes.HTTP_METHOD.key" "POST" "$SemanticAttributes.HTTP_STATUS_CODE.key" 200 - "$SemanticAttributes.HTTP_URL.key" "http://127.0.0.1:$portTwo/serviceTwo" + "$SemanticAttributes.HTTP_SCHEME.key" "http" + "$SemanticAttributes.HTTP_HOST.key" "127.0.0.1:$portTwo" + "$SemanticAttributes.HTTP_TARGET.key" "/serviceTwo" "$SemanticAttributes.NET_PEER_PORT.key" Number "$SemanticAttributes.NET_PEER_IP.key" "127.0.0.1" "$SemanticAttributes.HTTP_USER_AGENT.key" "Jakarta Commons-HttpClient/3.1" From eecc0f037cff1a998fd4189ed2068765fd4a490b Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 20:30:07 -0700 Subject: [PATCH 05/23] Finatra --- .../latestDepTest/groovy/FinatraServerLatestTest.groovy | 9 +++++++++ .../javaagent/src/test/groovy/FinatraServerTest.groovy | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/instrumentation/finatra-2.9/javaagent/src/latestDepTest/groovy/FinatraServerLatestTest.groovy b/instrumentation/finatra-2.9/javaagent/src/latestDepTest/groovy/FinatraServerLatestTest.groovy index f7af2fe059e1..9898f0ea2684 100644 --- a/instrumentation/finatra-2.9/javaagent/src/latestDepTest/groovy/FinatraServerLatestTest.groovy +++ b/instrumentation/finatra-2.9/javaagent/src/latestDepTest/groovy/FinatraServerLatestTest.groovy @@ -10,10 +10,12 @@ import com.twitter.util.Await import com.twitter.util.Closable import com.twitter.util.Duration import com.twitter.util.Promise +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import io.opentelemetry.sdk.trace.data.SpanData +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import static io.opentelemetry.api.trace.SpanKind.INTERNAL import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND @@ -94,4 +96,11 @@ class FinatraServerLatestTest extends HttpServerTest implements Agen } } } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } diff --git a/instrumentation/finatra-2.9/javaagent/src/test/groovy/FinatraServerTest.groovy b/instrumentation/finatra-2.9/javaagent/src/test/groovy/FinatraServerTest.groovy index 625478821c05..8f1344afe0f3 100644 --- a/instrumentation/finatra-2.9/javaagent/src/test/groovy/FinatraServerTest.groovy +++ b/instrumentation/finatra-2.9/javaagent/src/test/groovy/FinatraServerTest.groovy @@ -7,10 +7,12 @@ import com.twitter.finatra.http.HttpServer import com.twitter.util.Await import com.twitter.util.Closable import com.twitter.util.Duration +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import io.opentelemetry.sdk.trace.data.SpanData +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import java.util.concurrent.TimeUnit @@ -78,4 +80,11 @@ class FinatraServerTest extends HttpServerTest implements AgentTestT } } } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } From c7339c738bd523a598918b11becca03d39554516 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 20:53:06 -0700 Subject: [PATCH 06/23] jsp --- .../JspInstrumentationBasicTests.groovy | 32 ++++++++++++++----- .../JspInstrumentationForwardTests.groovy | 24 ++++++++++---- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy index f62d272f5bde..46097dc64a42 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy @@ -89,7 +89,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/$jspFileName" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/$jspFileName" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -139,7 +141,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/getQuery.jsp?$queryString" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/getQuery.jsp?$queryString" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -185,7 +189,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/post.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/post.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "POST" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -240,7 +246,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/$jspFileName" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/$jspFileName" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 500 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -300,7 +308,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/includes/includeHtml.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/includes/includeHtml.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -341,7 +351,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/includes/includeMulti.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/includes/includeMulti.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -414,7 +426,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/$jspFileName" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/$jspFileName" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 500 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -456,7 +470,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/$staticFile" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/$staticFile" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy index ce1f244b36b1..77607c3ef8b1 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy @@ -86,7 +86,9 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/$forwardFromFileName" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/$forwardFromFileName" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -148,7 +150,9 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/forwards/forwardToHtml.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/forwards/forwardToHtml.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -189,7 +193,9 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/forwards/forwardToIncludeMulti.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/forwards/forwardToIncludeMulti.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -278,7 +284,9 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/forwards/forwardToJspForward.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/forwards/forwardToJspForward.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -353,7 +361,9 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/forwards/forwardToCompileError.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/forwards/forwardToCompileError.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 500 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -407,7 +417,9 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/$jspWebappContext/forwards/forwardToNonExistent.jsp" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/$jspWebappContext/forwards/forwardToNonExistent.jsp" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 404 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" From 9c0fa30303448490ac123fe96d1dec8b8e7fc475 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 21:21:09 -0700 Subject: [PATCH 07/23] Ratpack --- .../server/AbstractRatpackHttpServerTest.groovy | 9 +++++++++ .../server/AbstractRatpackRoutesTest.groovy | 14 ++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy index 36e9f1647ab0..e74da911f2b5 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy @@ -5,10 +5,12 @@ package io.opentelemetry.instrumentation.ratpack.server +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.api.trace.StatusCode import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import io.opentelemetry.sdk.trace.data.SpanData +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import ratpack.error.ServerErrorHandler import ratpack.handling.Context import ratpack.server.RatpackServer @@ -143,4 +145,11 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy index 0d42b6b2e7f1..f6a575da2f1b 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy @@ -104,15 +104,15 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } "${SemanticAttributes.NET_PEER_PORT.key}" Long + "${SemanticAttributes.HTTP_URL.key}" "http://localhost:${app.bindPort}/${path}" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" "${SemanticAttributes.HTTP_USER_AGENT.key}" String - "${SemanticAttributes.HTTP_HOST}" "localhost:${app.bindPort}" - "${SemanticAttributes.HTTP_SCHEME}" "http" - "${SemanticAttributes.HTTP_TARGET}" "/$path" - + if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { + "${SemanticAttributes.HTTP_HOST}" "localhost:${app.bindPort}" + } if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long } @@ -124,9 +124,15 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification { // currently reports '/*' which is a fallback route. "${SemanticAttributes.HTTP_ROUTE}" String } + if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { + "${SemanticAttributes.HTTP_SCHEME}" "http" + } if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) { "${SemanticAttributes.HTTP_SERVER_NAME}" String } + if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { + "${SemanticAttributes.HTTP_TARGET}" "/$path" + } if (extraAttributes.contains(SemanticAttributes.NET_PEER_NAME)) { "${SemanticAttributes.NET_PEER_NAME}" "localhost" } From a0c294a572c578196cdc5d48871a015857cc958d Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 21:38:47 -0700 Subject: [PATCH 08/23] Ratpack library --- .../instrumentation/ratpack/server/RatpackRoutesTest.groovy | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy index 14a31dffa0c1..9d877321231e 100644 --- a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy +++ b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy @@ -29,6 +29,8 @@ class RatpackRoutesTest extends AbstractRatpackRoutesTest implements LibraryTest List> extraAttributes() { return [ SemanticAttributes.HTTP_ROUTE, + SemanticAttributes.HTTP_SCHEME, + SemanticAttributes.HTTP_HOST, SemanticAttributes.HTTP_TARGET, SemanticAttributes.NET_TRANSPORT, ] From 1ef1fbd19577a69c718b068e73abf699a6fd4df5 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 27 Sep 2021 21:41:16 -0700 Subject: [PATCH 09/23] Ratpack --- .../test/groovy/server/RatpackRoutesTest.groovy | 9 +++++++++ .../ratpack/server/RatpackRoutesTest.groovy | 5 +---- .../server/AbstractRatpackRoutesTest.groovy | 14 ++++++-------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy index e52bd4f93c1e..e05de4827085 100644 --- a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy +++ b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/server/RatpackRoutesTest.groovy @@ -5,8 +5,10 @@ package server +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.ratpack.server.AbstractRatpackRoutesTest import io.opentelemetry.instrumentation.test.AgentTestTrait +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import ratpack.server.RatpackServerSpec class RatpackRoutesTest extends AbstractRatpackRoutesTest implements AgentTestTrait { @@ -18,4 +20,11 @@ class RatpackRoutesTest extends AbstractRatpackRoutesTest implements AgentTestTr boolean hasHandlerSpan() { return true } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } diff --git a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy index 9d877321231e..fceb8cd1c8da 100644 --- a/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy +++ b/instrumentation/ratpack-1.4/library/src/test/groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackRoutesTest.groovy @@ -29,10 +29,7 @@ class RatpackRoutesTest extends AbstractRatpackRoutesTest implements LibraryTest List> extraAttributes() { return [ SemanticAttributes.HTTP_ROUTE, - SemanticAttributes.HTTP_SCHEME, - SemanticAttributes.HTTP_HOST, - SemanticAttributes.HTTP_TARGET, - SemanticAttributes.NET_TRANSPORT, + SemanticAttributes.NET_TRANSPORT ] } } diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy index f6a575da2f1b..9a21d9700952 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy @@ -104,15 +104,19 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:${app.bindPort}/${path}" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" "${SemanticAttributes.HTTP_USER_AGENT.key}" String - if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { + if (extraAttributes.contains(SemanticAttributes.HTTP_URL)) { + "${SemanticAttributes.HTTP_URL.key}" "http://localhost:${app.bindPort}/${path}" + } else { + "${SemanticAttributes.HTTP_SCHEME}" "http" "${SemanticAttributes.HTTP_HOST}" "localhost:${app.bindPort}" + "${SemanticAttributes.HTTP_TARGET}" "/$path" } + if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long } @@ -124,15 +128,9 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification { // currently reports '/*' which is a fallback route. "${SemanticAttributes.HTTP_ROUTE}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { - "${SemanticAttributes.HTTP_SCHEME}" "http" - } if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) { "${SemanticAttributes.HTTP_SERVER_NAME}" String } - if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { - "${SemanticAttributes.HTTP_TARGET}" "/$path" - } if (extraAttributes.contains(SemanticAttributes.NET_PEER_NAME)) { "${SemanticAttributes.NET_PEER_NAME}" "localhost" } From e756927535d62d4b1876f3daa46bd4a4e9b958e6 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 09:38:42 -0700 Subject: [PATCH 10/23] Spark --- .../javaagent/src/test/groovy/SparkJavaBasedTest.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy b/instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy index 631c2682108b..93c24f4ed72a 100644 --- a/instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy +++ b/instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy @@ -48,7 +48,9 @@ class SparkJavaBasedTest extends AgentInstrumentationSpecification { attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" "http://localhost:$port/param/asdf1234" + "${SemanticAttributes.HTTP_SCHEME.key}" "http" + "${SemanticAttributes.HTTP_HOST.key}" "localhost:$port" + "${SemanticAttributes.HTTP_TARGET.key}" "/param/asdf1234" "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" From 13da1a2cb12114158c3284e05624402a293f2087 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 09:40:23 -0700 Subject: [PATCH 11/23] Feedback --- .../ratpack/RatpackHttpAttributesExtractor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java b/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java index a07b0bd97b73..4d8d22b66169 100644 --- a/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java +++ b/instrumentation/ratpack-1.4/library/src/main/java/io/opentelemetry/instrumentation/ratpack/RatpackHttpAttributesExtractor.java @@ -38,7 +38,7 @@ protected String host(Request request) { if (publicAddress == null) { return null; } - URI uri = publicAddress.builder().build(); + URI uri = publicAddress.get(); return uri.getHost() + ":" + uri.getPort(); } @@ -60,7 +60,7 @@ protected String scheme(Request request) { if (publicAddress == null) { return null; } - return publicAddress.builder().build().getScheme(); + return publicAddress.get().getScheme(); } @Override From 6f907e3ce6797b0df2a10cdbca42dead2f622f39 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 10:57:03 -0700 Subject: [PATCH 12/23] Fix Undertow --- .../undertow/UndertowHttpServerTracer.java | 2 +- .../javaagent/src/test/groovy/UndertowServerTest.groovy | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java index 2a873a50ea02..bff1988f13d4 100644 --- a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java +++ b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java @@ -137,7 +137,7 @@ protected String host(HttpServerExchange exchange) { protected String target(HttpServerExchange exchange) { String target = exchange.getRequestPath(); String queryString = exchange.getQueryString(); - if (queryString != null) { + if (queryString != null && !queryString.isEmpty()) { target += "?" + queryString; } return target; diff --git a/instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowServerTest.groovy b/instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowServerTest.groovy index a9b881858d31..bbf873bceacf 100644 --- a/instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowServerTest.groovy +++ b/instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowServerTest.groovy @@ -121,7 +121,9 @@ class UndertowServerTest extends HttpServerTest implements AgentTestTr "${SemanticAttributes.NET_PEER_PORT.key}" { it instanceof Long } "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.HTTP_CLIENT_IP.key}" TEST_CLIENT_IP - "${SemanticAttributes.HTTP_URL.key}" uri.toString() + "${SemanticAttributes.HTTP_SCHEME.key}" uri.getScheme() + "${SemanticAttributes.HTTP_HOST.key}" uri.getHost() + ":" + uri.getPort() + "${SemanticAttributes.HTTP_TARGET.key}" uri.getPath() "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" @@ -167,7 +169,9 @@ class UndertowServerTest extends HttpServerTest implements AgentTestTr "${SemanticAttributes.NET_PEER_PORT.key}" { it instanceof Long } "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.HTTP_CLIENT_IP.key}" TEST_CLIENT_IP - "${SemanticAttributes.HTTP_URL.key}" uri.toString() + "${SemanticAttributes.HTTP_SCHEME.key}" uri.getScheme() + "${SemanticAttributes.HTTP_HOST.key}" uri.getHost() + ":" + uri.getPort() + "${SemanticAttributes.HTTP_TARGET.key}" uri.getPath() "${SemanticAttributes.HTTP_METHOD.key}" "GET" "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200 "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" From dd3325d7383112ba916b3d4174c0b63b6ebc9b77 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 14:26:55 -0700 Subject: [PATCH 13/23] Vertx --- .../groovy/server/VertxRxHttpServerTest.groovy | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy index e53acf3baa10..bdc21d4d515a 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy @@ -5,8 +5,10 @@ package server +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import io.vertx.core.DeploymentOptions import io.vertx.core.Future import io.vertx.core.Vertx @@ -78,6 +80,13 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr return true } + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } + protected Class verticle() { return VertxReactiveWebServer } From b898716ce1225a745140714f943e9d11946978f0 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 16:42:51 -0700 Subject: [PATCH 14/23] vertx-web --- .../groovy/server/AbstractVertxHttpServerTest.groovy | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy b/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy index 9ba0d1af3997..03866f80c9d9 100644 --- a/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy +++ b/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy @@ -5,8 +5,10 @@ package server +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import io.vertx.core.AbstractVerticle import io.vertx.core.DeploymentOptions import io.vertx.core.Vertx @@ -58,6 +60,13 @@ abstract class AbstractVertxHttpServerTest extends HttpServerTest impleme return true } + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } + @Override String expectedServerSpanName(ServerEndpoint endpoint) { switch (endpoint) { From 9bb1c3e2da63013dcbf7ff4657c527ac322202a2 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 17:51:15 -0700 Subject: [PATCH 15/23] play-2.4 --- .../src/test/groovy/server/PlayServerTest.groovy | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy b/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy index 48471b8da3bd..3de337809c9c 100644 --- a/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy +++ b/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy @@ -5,11 +5,13 @@ package server +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.api.trace.StatusCode import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import io.opentelemetry.sdk.trace.data.SpanData +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import play.mvc.Results import play.routing.RoutingDsl import play.server.Server @@ -97,4 +99,11 @@ class PlayServerTest extends HttpServerTest implements AgentTestTrait { String expectedServerSpanName(ServerEndpoint endpoint) { return "HTTP GET" } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } From 52cae3344007c5f57b9f7e047e326f93a196a9c1 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 19:11:01 -0700 Subject: [PATCH 16/23] webflux --- .../groovy/server/base/SpringWebFluxServerTest.groovy | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy index 8f84d198a6f0..7a7aa79d6a11 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy @@ -5,8 +5,10 @@ package server.base +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import org.springframework.boot.SpringApplication import org.springframework.context.ConfigurableApplicationContext import util.SpringWebfluxTestUtil @@ -69,4 +71,11 @@ abstract class SpringWebFluxServerTest extends HttpServerTest expectedExceptionClass() { return IllegalStateException } + + @Override + List> extraAttributes() { + return [ + SemanticAttributes.HTTP_URL + ] + } } From 838298007183903b09408e9a1673a3dbc5f706a9 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 20:00:39 -0700 Subject: [PATCH 17/23] jaxrs --- .../src/main/groovy/JaxRsHttpServerTest.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy index 64b4d187ec3a..316bf2f5ced5 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy @@ -273,7 +273,9 @@ abstract class JaxRsHttpServerTest extends HttpServerTest implements Agent attributes { "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" } // Optional "${SemanticAttributes.NET_PEER_PORT.key}" Long - "${SemanticAttributes.HTTP_URL.key}" fullUrl.toString() + "${SemanticAttributes.HTTP_SCHEME.key}" fullUrl.getScheme() + "${SemanticAttributes.HTTP_HOST.key}" fullUrl.getHost() + ":" + fullUrl.getPort() + "${SemanticAttributes.HTTP_TARGET.key}" fullUrl.getPath() + (fullUrl.getQuery() != null ? "?" + fullUrl.getQuery() : "") "${SemanticAttributes.HTTP_METHOD.key}" method "${SemanticAttributes.HTTP_STATUS_CODE.key}" statusCode "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1" From e0f3c36544fdd37965a147c5b9864f524dc3101f Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 20:25:33 -0700 Subject: [PATCH 18/23] Spotless --- .../netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy b/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy index 83c4cf43b685..3c6844ca5226 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy +++ b/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ - import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest From 9b1f148e2bc15b3dd9db6e73ba6b2a98b29815b0 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 20:32:22 -0700 Subject: [PATCH 19/23] Update semantic-conventions.md --- docs/semantic-conventions.md | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/docs/semantic-conventions.md b/docs/semantic-conventions.md index ceb237a7b7a2..88234e1a163e 100644 --- a/docs/semantic-conventions.md +++ b/docs/semantic-conventions.md @@ -8,12 +8,12 @@ are implemented by Java autoinstrumentation and which ones are not. | Attribute | Required | Implemented? | |---|:---:|:---:| | `http.method` | Y | + | -| `http.url` | N | + | -| `http.target` | N | - [1] | -| `http.host` | N | - [1] | -| `http.scheme` | N | - [1] | +| `http.url` | N | - [1] | +| `http.target` | N | + [1] | +| `http.host` | N | + [1] | +| `http.scheme` | N | + [1] | | `http.status_code` | Y | + | -| `http.flavor` | N | + [3] | +| `http.flavor` | N | + [2] | | `http.user_agent` | N | + | | `http.request_content_length` | N | - | | `http.request_content_length_uncompressed` | N | - | @@ -23,12 +23,17 @@ are implemented by Java autoinstrumentation and which ones are not. | `http.route` | N | - | | `http.client_ip` | N | + | -**[1]:** As the majority of Java frameworks don't provide a standard way to obtain "The full request +**[1]:** Most server instrumentation captures `http.scheme`, `http.host`, and `http.target` +(and does not capture `http.url`). +Netty instrumentation is currently the only exception to this rule. Netty instrumentation +captures `http.url` (and does not capture `http.scheme`, `http.host`, or `http.target`). + +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. -**[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), +**[2]:** 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. @@ -42,19 +47,16 @@ not values defined by spec. | `http.host` | N | - [1] | | `http.scheme` | N | - [1] | | `http.status_code` | Y | + | -| `http.flavor` | N | + [3] | +| `http.flavor` | N | + [2] | | `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. +**[1]:** `http.scheme`, `http.host` and `http.target` are unnecessary since `http.url` is captured. -**[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), +**[2]:** 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. ## RPC From cd98c8a58a39073c2714a048576c42484c4b9944 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 28 Sep 2021 21:50:14 -0700 Subject: [PATCH 20/23] Update smoke tests --- .../smoketest/AppServerTest.groovy | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/AppServerTest.groovy b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/AppServerTest.groovy index 3b44bc421282..c01323bc3a4b 100644 --- a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/AppServerTest.groovy +++ b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/AppServerTest.groovy @@ -99,10 +99,13 @@ abstract class AppServerTest extends SmokeTest { traces.countSpansByName(getSpanName('/app/headers')) == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/app/greeting") == 1 + traces.countFilteredAttributes("http.target", "/app/greeting") == 1 - and: "Client and server spans for the remote call" - traces.countFilteredAttributes("http.url", "http://localhost:8080/app/headers") == 2 + and: "Client span for the remote call" + traces.countFilteredAttributes("http.url", "http://localhost:8080/app/headers") == 1 + + and: "Server span for the remote call" + traces.countFilteredAttributes("http.target", "/app/headers") == 1 and: "Number of spans with http protocol version" traces.countFilteredAttributes("http.flavor", "1.1") == 3 @@ -140,7 +143,7 @@ abstract class AppServerTest extends SmokeTest { traces.countSpansByName(getSpanName('/app/hello.txt')) == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/app/hello.txt") == 1 + traces.countFilteredAttributes("http.target", "/app/hello.txt") == 1 and: "Number of spans tagged with current otel library version" traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == 1 @@ -174,7 +177,7 @@ abstract class AppServerTest extends SmokeTest { traces.countSpansByName(getSpanName('/app/file-that-does-not-exist')) == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/app/file-that-does-not-exist") == 1 + traces.countFilteredAttributes("http.target", "/app/file-that-does-not-exist") == 1 and: "Number of spans tagged with current otel library version" traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == traces.countSpans() @@ -210,7 +213,7 @@ abstract class AppServerTest extends SmokeTest { traces.countSpansByName(getSpanName('/app/WEB-INF/web.xml')) == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/app/WEB-INF/web.xml") == 1 + traces.countFilteredAttributes("http.target", "/app/WEB-INF/web.xml") == 1 and: "Number of spans with http protocol version" traces.countFilteredAttributes("http.flavor", "1.1") == 1 @@ -252,7 +255,7 @@ abstract class AppServerTest extends SmokeTest { traces.countFilteredEventAttributes('exception.message', 'This is expected') == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/app/exception") == 1 + traces.countFilteredAttributes("http.target", "/app/exception") == 1 and: "Number of spans tagged with current otel library version" traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == 1 @@ -286,7 +289,7 @@ abstract class AppServerTest extends SmokeTest { traces.countSpansByName(getSpanName('/this-is-definitely-not-there-but-there-should-be-a-trace-nevertheless')) == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/this-is-definitely-not-there-but-there-should-be-a-trace-nevertheless") == 1 + traces.countFilteredAttributes("http.target", "/this-is-definitely-not-there-but-there-should-be-a-trace-nevertheless") == 1 and: "Number of spans with http protocol version" traces.countFilteredAttributes("http.flavor", "1.1") == 1 @@ -327,10 +330,13 @@ abstract class AppServerTest extends SmokeTest { traces.countSpansByName(getSpanName('/app/headers')) == 1 and: "The span for the initial web request" - traces.countFilteredAttributes("http.url", "http://localhost:${containerManager.getTargetMappedPort(8080)}/app/asyncgreeting") == 1 + traces.countFilteredAttributes("http.target", "/app/asyncgreeting") == 1 + + and: "Client span for the remote call" + traces.countFilteredAttributes("http.url", "http://localhost:8080/app/headers") == 1 - and: "Client and server spans for the remote call" - traces.countFilteredAttributes("http.url", "http://localhost:8080/app/headers") == 2 + and: "Server span for the remote call" + traces.countFilteredAttributes("http.target", "/app/headers") == 1 and: "Number of spans with http protocol version" traces.countFilteredAttributes("http.flavor", "1.1") == 3 From 6d1900a0107bebedb5ae46da90b92ec1f4e5179b Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 29 Sep 2021 09:35:47 -0700 Subject: [PATCH 21/23] More realistic target --- .../http/HttpServerAttributesExtractorTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java index bd9838735414..062b702eda7e 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java @@ -94,7 +94,7 @@ void normal() { Map request = new HashMap<>(); request.put("method", "POST"); request.put("url", "http://github.com"); - request.put("target", "github.com"); + request.put("target", "/repositories/1"); request.put("host", "github.com:80"); request.put("scheme", "https"); request.put("userAgent", "okhttp 3.x"); @@ -115,7 +115,7 @@ void normal() { assertThat(attributes.build()) .containsOnly( entry(SemanticAttributes.HTTP_METHOD, "POST"), - entry(SemanticAttributes.HTTP_TARGET, "github.com"), + entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"), entry(SemanticAttributes.HTTP_HOST, "github.com:80"), entry(SemanticAttributes.HTTP_SCHEME, "https"), entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"), @@ -125,7 +125,7 @@ void normal() { assertThat(attributes.build()) .containsOnly( entry(SemanticAttributes.HTTP_METHOD, "POST"), - entry(SemanticAttributes.HTTP_TARGET, "github.com"), + entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"), entry(SemanticAttributes.HTTP_HOST, "github.com:80"), entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"), entry(SemanticAttributes.HTTP_SCHEME, "https"), From c0164dc2991fe4475959788b7af53ccd1cc78aa3 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 29 Sep 2021 09:38:18 -0700 Subject: [PATCH 22/23] Remove outdated doc --- docs/semantic-conventions.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/semantic-conventions.md b/docs/semantic-conventions.md index 88234e1a163e..b452c1377183 100644 --- a/docs/semantic-conventions.md +++ b/docs/semantic-conventions.md @@ -28,11 +28,6 @@ are implemented by Java autoinstrumentation and which ones are not. Netty instrumentation is currently the only exception to this rule. Netty instrumentation captures `http.url` (and does not capture `http.scheme`, `http.host`, or `http.target`). -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]:** 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. From eafaafd4f6a0195e0bb2473eb927c6e9e94b295a Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 29 Sep 2021 09:39:33 -0700 Subject: [PATCH 23/23] Wording --- docs/semantic-conventions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/semantic-conventions.md b/docs/semantic-conventions.md index b452c1377183..081b5eb81653 100644 --- a/docs/semantic-conventions.md +++ b/docs/semantic-conventions.md @@ -23,8 +23,8 @@ are implemented by Java autoinstrumentation and which ones are not. | `http.route` | N | - | | `http.client_ip` | N | + | -**[1]:** Most server instrumentation captures `http.scheme`, `http.host`, and `http.target` -(and does not capture `http.url`). +**[1]:** Most server instrumentations capture `http.scheme`, `http.host`, and `http.target` +(and do not capture `http.url`). Netty instrumentation is currently the only exception to this rule. Netty instrumentation captures `http.url` (and does not capture `http.scheme`, `http.host`, or `http.target`).