diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java index df159f8f2e6..126169e2001 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java @@ -23,6 +23,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIUtils; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; @@ -42,6 +43,7 @@ public abstract class HttpClientDecorator extends UriBasedCli private static final String DD_CLIENT_LIBRARY_LANGUAGE_HEADER_NAME = "DD-Client-Library-Language"; private static final BitSet CLIENT_ERROR_STATUSES = Config.get().getHttpClientErrorStatuses(); + private static final boolean OTEL_SEMANTICS = Config.get().isTraceOtelSemanticsEnabled(); private static final UTF8BytesString DEFAULT_RESOURCE_NAME = UTF8BytesString.create("/"); @@ -118,7 +120,14 @@ protected void doOnRequest(final AgentSpan span, final REQUEST request) { DSM_TRANSACTION_SOURCE_READER); String method = method(request); - span.setTag(Tags.HTTP_METHOD, method); + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setRequestMethod(span, method); + if (shouldSetResourceName()) { + OtelHttpSemantics.setMethodResourceName(span, method); + } + } else { + span.setTag(Tags.HTTP_METHOD, method); + } if (CLIENT_TAG_HEADERS) { for (Map.Entry headerTag : @@ -135,19 +144,28 @@ protected void doOnRequest(final AgentSpan span, final REQUEST request) { final URI url = url(request); if (url != null) { onURI(span, url); - span.setTag( - Tags.HTTP_URL, - URIUtils.lazyValidURL(url.getScheme(), url.getHost(), url.getPort(), url.getPath())); - if (Config.get().isHttpClientTagQueryString()) { - span.setTag(DDTags.HTTP_QUERY, url.getQuery()); - span.setTag(DDTags.HTTP_FRAGMENT, url.getFragment()); + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setClientUrl(span, url); + // url.full may trigger URLAsResourceNameRule; restore the OTel span name afterwards. + if (shouldSetResourceName()) { + OtelHttpSemantics.setMethodResourceName(span, method); + } + } else { + span.setTag( + Tags.HTTP_URL, + URIUtils.lazyValidURL( + url.getScheme(), url.getHost(), url.getPort(), url.getPath())); + if (Config.get().isHttpClientTagQueryString()) { + span.setTag(DDTags.HTTP_QUERY, url.getQuery()); + span.setTag(DDTags.HTTP_FRAGMENT, url.getFragment()); + } } - if (shouldSetResourceName()) { + if (!OTEL_SEMANTICS && shouldSetResourceName()) { HTTP_RESOURCE_DECORATOR.withClientPath(span, method, url.getPath()); } // SSRF exploit prevention check onHttpClientRequest(span, url.toString()); - } else if (shouldSetResourceName()) { + } else if (!OTEL_SEMANTICS && shouldSetResourceName()) { span.setResourceName(DEFAULT_RESOURCE_NAME); } } catch (final BlockingException e) { @@ -177,6 +195,9 @@ protected void doOnResponse(final AgentSpan span, final RESPONSE response) { span.setHttpStatusCode(status); if (CLIENT_ERROR_STATUSES.get(status)) { span.setError(true); + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setErrorType(span, status); + } } } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java index a962c759f92..a893faacd42 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java @@ -14,6 +14,7 @@ import datadog.context.propagation.Propagators; import datadog.trace.api.Config; import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTags; import datadog.trace.api.datastreams.DataStreamsTransactionExtractor; import datadog.trace.api.datastreams.DataStreamsTransactionTracker; import datadog.trace.api.function.TriConsumer; @@ -34,6 +35,7 @@ import datadog.trace.bootstrap.instrumentation.api.ClientIpAddressData; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -68,13 +70,15 @@ public abstract class HttpServerDecorator flow = callIGCallbackURI(span, url, method); if (flow.getAction() instanceof RequestBlockingAction) { @@ -422,6 +469,9 @@ protected void doOnRequest( } if (shouldTagIps) { span.setTag(Tags.NETWORK_CLIENT_IP, peerIp); + if (OTEL_SEMANTICS && span.getTag(KnownTags.HTTP_CLIENT_IP_OTEL_NAME) == null) { + span.setTag(KnownTags.HTTP_CLIENT_IP_OTEL_NAME, peerIp); + } } } if (shouldStashIps && (peerIp != null || inferredAddressStr != null)) { @@ -484,6 +534,9 @@ protected void doOnResponseStatus(final AgentSpan span, final int status) { if (!BlockingException.class.getName().equals(span.getTag("error.type"))) { span.setError(SERVER_ERROR_STATUSES.get(status), ErrorPriorities.HTTP_SERVER_DECORATOR); } + if (OTEL_SEMANTICS && SERVER_ERROR_STATUSES.get(status)) { + OtelHttpSemantics.setErrorType(span, status); + } } if (SHOULD_SET_404_RESOURCE_NAME && status == 404) { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UriBasedClientDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UriBasedClientDecorator.java index 0ab130eab19..68c1b42d876 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UriBasedClientDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UriBasedClientDecorator.java @@ -6,24 +6,36 @@ import datadog.context.Context; import datadog.context.propagation.CarrierSetter; import datadog.trace.api.Config; +import datadog.trace.api.KnownTags; import datadog.trace.api.datastreams.DataStreamsContext; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.net.URI; import javax.annotation.Nonnull; public abstract class UriBasedClientDecorator extends ClientDecorator { + private static final boolean OTEL_SEMANTICS = Config.get().isTraceOtelSemanticsEnabled(); + public void onURI(@Nonnull final AgentSpan span, @Nonnull final URI uri) { String host = uri.getHost(); - int port = uri.getPort(); if (null != host && !host.isEmpty()) { - span.setTag(Tags.PEER_HOSTNAME, host); + if (OTEL_SEMANTICS) { + span.setTag(KnownTags.SERVER_ADDRESS_NAME, host); + int serverPort = OtelHttpSemantics.serverPort(uri); + if (serverPort > 0) { + span.setTag(KnownTags.SERVER_PORT_NAME, serverPort); + } + } else { + span.setTag(Tags.PEER_HOSTNAME, host); + int port = uri.getPort(); + if (port > 0) { + setPeerPort(span, port); + } + } if (Config.get().isHttpClientSplitByDomain() && host.charAt(0) >= 'A') { span.setServiceName(host, component()); } - if (port > 0) { - setPeerPort(span, port); - } } } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java index a3c8be6dd5b..00cf9056ec0 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java @@ -1,10 +1,12 @@ package datadog.trace.bootstrap.instrumentation.decorator; +import datadog.trace.api.Config; import datadog.trace.api.cache.DDCache; import datadog.trace.api.cache.DDCaches; import datadog.trace.api.naming.SpanNaming; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import java.net.URI; @@ -16,6 +18,7 @@ import org.slf4j.LoggerFactory; public class UrlConnectionDecorator extends UriBasedClientDecorator { + private static final boolean OTEL_SEMANTICS = Config.get().isTraceOtelSemanticsEnabled(); private static final DDCache CACHE = DDCaches.newFixedSizeCache(16); private static final Function ADDER = @@ -51,7 +54,11 @@ protected String service() { @Override public void onURI(@Nonnull AgentSpan span, @Nonnull URI uri) { super.onURI(span, uri); - span.setTag(Tags.HTTP_URL, uri.toString()); + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setClientUrl(span, uri); + } else { + span.setTag(Tags.HTTP_URL, uri.toString()); + } } public void onURL(@Nonnull final AgentSpan span, @Nonnull final URL url) { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/HttpResourceDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/HttpResourceDecorator.java index 72ca66a520f..567646c7fb2 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/HttpResourceDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/http/HttpResourceDecorator.java @@ -3,6 +3,7 @@ import datadog.trace.api.Config; import datadog.trace.api.normalize.HttpResourceNames; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIUtils; @@ -12,6 +13,7 @@ public class HttpResourceDecorator { public static final HttpResourceDecorator HTTP_RESOURCE_DECORATOR = new HttpResourceDecorator(); private static final UTF8BytesString DEFAULT_RESOURCE_NAME = UTF8BytesString.create("/"); + private static final boolean OTEL_SEMANTICS = Config.get().isTraceOtelSemanticsEnabled(); private final boolean shouldSetUrlResourceName = Config.get().isRuleEnabled("URLAsResourceNameRule"); @@ -19,11 +21,19 @@ public class HttpResourceDecorator { private HttpResourceDecorator() {} public final void withClientPath(AgentSpan span, CharSequence method, CharSequence path) { - HttpResourceNames.setForClient(span, method, path, false); + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setMethodResourceName(span, method); + } else { + HttpResourceNames.setForClient(span, method, path, false); + } } public final void withServerPath( AgentSpan span, CharSequence method, CharSequence path, boolean encoded) { + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setMethodResourceName(span, method); + return; + } if (!shouldSetUrlResourceName) { span.setResourceName(DEFAULT_RESOURCE_NAME); return; @@ -44,7 +54,11 @@ public final void withRoute( routeTag = URIUtils.decode(route.toString()); } span.setTag(Tags.HTTP_ROUTE, routeTag); - if (Config.get().isHttpServerRouteBasedNaming()) { + if (OTEL_SEMANTICS) { + final CharSequence resourceName = + HttpResourceNames.join(OtelHttpSemantics.spanNameMethod(method), routeTag); + span.setResourceName(resourceName, ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE); + } else if (Config.get().isHttpServerRouteBasedNaming()) { final CharSequence resourceName = HttpResourceNames.join(method, route); span.setResourceName(resourceName, ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE); } diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/HttpDecoratorsOtelSemanticsForkedTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/HttpDecoratorsOtelSemanticsForkedTest.java new file mode 100644 index 00000000000..ec36e6d897e --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/HttpDecoratorsOtelSemanticsForkedTest.java @@ -0,0 +1,289 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import static datadog.context.Context.root; +import static datadog.trace.api.config.GeneralConfig.TRACE_OTEL_SEMANTICS_ENABLED; +import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_CLIENT_TAG_QUERY_STRING; +import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_TAG_QUERY_STRING; +import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.KnownTags; +import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.TagContext; +import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; +import datadog.trace.bootstrap.instrumentation.api.URIDefaultDataAdapter; +import datadog.trace.common.writer.ListWriter; +import datadog.trace.core.CoreTracer; +import datadog.trace.core.DDSpan; +import datadog.trace.test.junit.utils.config.WithConfig; +import java.net.URI; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.tabletest.junit.TableTest; + +@WithConfig(key = TRACE_OTEL_SEMANTICS_ENABLED, value = "true") +@WithConfig(key = HTTP_CLIENT_TAG_QUERY_STRING, value = "false") +@WithConfig(key = HTTP_SERVER_TAG_QUERY_STRING, value = "false") +class HttpDecoratorsOtelSemanticsForkedTest { + private CoreTracer tracer; + + @BeforeEach + void setUp() { + tracer = CoreTracer.builder().writer(new ListWriter()).build(); + } + + @AfterEach + void tearDown() { + tracer.close(); + } + + @Test + void clientDecoratorCapturesOtelHttpValues() throws Exception { + TestClientDecorator decorator = new TestClientDecorator(); + DDSpan span = (DDSpan) tracer.startSpan("test", "http.request"); + ClientRequest request = + new ClientRequest( + "BREW", new URI("https://user:password@example.com/orders?token=secret#fragment")); + + decorator.afterStart(span); + decorator.onRequest(span, request); + decorator.onResponse(span, 503); + + assertEquals("HTTP", span.getResourceName().toString()); + assertEquals("_OTHER", span.getTag(KnownTags.HTTP_METHOD_OTEL_NAME)); + assertEquals("BREW", span.getTag(KnownTags.HTTP_REQUEST_METHOD_ORIGINAL_NAME)); + assertEquals( + "https://REDACTED:REDACTED@example.com/orders#fragment", + span.getTag(KnownTags.HTTP_URL_OTEL_NAME).toString()); + assertEquals("token=secret", span.getTag(KnownTags.URL_QUERY_NAME)); + assertEquals("example.com", span.getTag(KnownTags.SERVER_ADDRESS_NAME)); + assertEquals(443, span.getTag(KnownTags.SERVER_PORT_NAME)); + assertEquals(503, span.getHttpStatusCode()); + assertEquals("503", span.getTag("error.type")); + assertTrue(span.isError()); + assertNull(span.getTag("peer.hostname")); + assertNull(span.getTag("peer.port")); + } + + @Test + void urlConnectionDecoratorCapturesQueryForObfuscation() throws Exception { + DDSpan span = (DDSpan) tracer.startSpan("test", "urlconnection.request"); + + UrlConnectionDecorator.DECORATE.onURI( + span, new URI("https://example.com/orders?token=secret#fragment")); + + assertEquals("https://example.com/orders#fragment", span.getTag(KnownTags.HTTP_URL_OTEL_NAME)); + assertEquals("token=secret", span.getTag(KnownTags.URL_QUERY_NAME)); + } + + @Test + void serverDecoratorCapturesOtelHttpValuesAndRouteName() throws Exception { + TestServerDecorator decorator = new TestServerDecorator(); + DDSpan span = (DDSpan) tracer.startSpan("test", "servlet.request"); + ServerRequest request = + new ServerRequest("GET", new URI("https://example.com:8443/users/123?view=full")); + + decorator.afterStart(span); + decorator.onRequest(span, new Connection("192.0.2.10", 32100), request, root()); + HTTP_RESOURCE_DECORATOR.withRoute(span, request.method, "/users/{id}"); + decorator.onResponse(span, 503); + + assertEquals("GET /users/{id}", span.getResourceName().toString()); + assertEquals("GET", span.getTag(KnownTags.HTTP_METHOD_OTEL_NAME)); + assertEquals("/users/123", span.getTag(KnownTags.URL_PATH_NAME)); + assertEquals("https", span.getTag(KnownTags.URL_SCHEME_NAME)); + assertEquals("view=full", span.getTag(KnownTags.URL_QUERY_NAME)); + assertEquals("example.com", span.getTag(KnownTags.SERVER_ADDRESS_NAME)); + assertEquals(8443, span.getTag(KnownTags.SERVER_PORT_NAME)); + assertEquals("192.0.2.10", span.getTag(KnownTags.HTTP_CLIENT_IP_OTEL_NAME)); + assertEquals("192.0.2.10", span.getTag(KnownTags.NETWORK_CLIENT_IP_OTEL_NAME)); + assertEquals(503, span.getHttpStatusCode()); + assertEquals("503", span.getTag("error.type")); + assertTrue(span.isError()); + assertFalse(span.getTags().containsKey("http.url")); + } + + @TableTest({ + "Scenario | Request URI | Expected Path", + "relative path | orders | /orders ", + "empty path | '' | / ", + "absolute path | /orders | /orders " + }) + void serverDecoratorNormalizesUrlPath(String requestUri, String expectedPath) { + TestServerDecorator decorator = new TestServerDecorator(); + DDSpan span = (DDSpan) tracer.startSpan("test", "servlet.request"); + ServerRequest request = new ServerRequest("GET", URI.create(requestUri)); + + decorator.onRequest(span, new Connection("192.0.2.10", 32100), request, root()); + + assertEquals(expectedPath, span.getTag(KnownTags.URL_PATH_NAME)); + } + + @Test + void serverDecoratorUsesForwardedOrigin() throws Exception { + TestServerDecorator decorator = new TestServerDecorator(); + DDSpan span = (DDSpan) tracer.startSpan("test", "servlet.request"); + ServerRequest request = + new ServerRequest("GET", new URI("http://internal.example.com:8080/users/123")); + TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); + headers.xForwardedProto = "https, http"; + headers.xForwardedHost = "public.example.com:8443, internal.example.com:8080"; + TagContext extracted = new TagContext(null, null, headers, null, 0, null, null, DDTraceId.ZERO); + + decorator.afterStart(span); + decorator.onRequest( + span, new Connection("192.0.2.10", 32100), request, AgentSpan.fromSpanContext(extracted)); + + assertEquals("https", span.getTag(KnownTags.URL_SCHEME_NAME)); + assertEquals("public.example.com", span.getTag(KnownTags.SERVER_ADDRESS_NAME)); + assertEquals(8443, span.getTag(KnownTags.SERVER_PORT_NAME)); + } + + @Test + void serverDecoratorUsesForwardedPortWithIpv6Host() throws Exception { + TestServerDecorator decorator = new TestServerDecorator(); + DDSpan span = (DDSpan) tracer.startSpan("test", "servlet.request"); + ServerRequest request = + new ServerRequest("GET", new URI("http://internal.example.com:8080/users/123")); + TagContext.HttpHeaders headers = new TagContext.HttpHeaders(); + headers.xForwardedProto = "https"; + headers.xForwardedHost = "[2001:db8::1]"; + headers.xForwardedPort = "9443"; + TagContext extracted = new TagContext(null, null, headers, null, 0, null, null, DDTraceId.ZERO); + + decorator.afterStart(span); + decorator.onRequest( + span, new Connection("192.0.2.10", 32100), request, AgentSpan.fromSpanContext(extracted)); + + assertEquals("https", span.getTag(KnownTags.URL_SCHEME_NAME)); + assertEquals("2001:db8::1", span.getTag(KnownTags.SERVER_ADDRESS_NAME)); + assertEquals(9443, span.getTag(KnownTags.SERVER_PORT_NAME)); + } + + private static final class ClientRequest { + private final String method; + private final URI uri; + + private ClientRequest(String method, URI uri) { + this.method = method; + this.uri = uri; + } + } + + private static final class TestClientDecorator + extends HttpClientDecorator { + @Override + protected String[] instrumentationNames() { + return new String[] {"test-http-client"}; + } + + @Override + protected CharSequence component() { + return "test-http-client"; + } + + @Override + protected String method(ClientRequest request) { + return request.method; + } + + @Override + protected URI url(ClientRequest request) { + return request.uri; + } + + @Override + protected int status(Integer response) { + return response; + } + + @Override + protected String getRequestHeader(ClientRequest request, String headerName) { + return null; + } + + @Override + protected String getResponseHeader(Integer response, String headerName) { + return null; + } + } + + private static final class ServerRequest { + private final String method; + private final URI uri; + + private ServerRequest(String method, URI uri) { + this.method = method; + this.uri = uri; + } + } + + private static final class Connection { + private final String host; + private final int port; + + private Connection(String host, int port) { + this.host = host; + this.port = port; + } + } + + private static final class TestServerDecorator + extends HttpServerDecorator { + @Override + protected String[] instrumentationNames() { + return new String[] {"test-http-server"}; + } + + @Override + protected CharSequence component() { + return "test-http-server"; + } + + @Override + protected AgentPropagation.ContextVisitor getter() { + return null; + } + + @Override + protected AgentPropagation.ContextVisitor responseGetter() { + return null; + } + + @Override + public CharSequence spanName() { + return "http.request"; + } + + @Override + protected String method(ServerRequest request) { + return request.method; + } + + @Override + protected URIDataAdapter url(ServerRequest request) { + return new URIDefaultDataAdapter(request.uri); + } + + @Override + protected String peerHostIP(Connection connection) { + return connection.host; + } + + @Override + protected int peerPort(Connection connection) { + return connection.port; + } + + @Override + protected int status(Integer response) { + return response; + } + } +} diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy index e787feeda1f..2f2394d78c3 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy @@ -5,8 +5,10 @@ import datadog.trace.agent.test.asserts.TagsAssert import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.naming.VersionedNamingTestBase import datadog.trace.agent.test.server.http.HttpProxy +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags +import datadog.trace.api.KnownTags import datadog.trace.api.appsec.HttpClientRequest import datadog.trace.api.appsec.HttpClientResponse import datadog.trace.api.config.TracerConfig @@ -16,6 +18,7 @@ import datadog.trace.api.gateway.Flow import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics import datadog.trace.bootstrap.instrumentation.api.TagContext import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.bootstrap.instrumentation.api.URIUtils @@ -990,11 +993,15 @@ abstract class HttpClientTest extends VersionedNamingTestBase { boolean ignorePeer = false, Map extraTags = null) { - def expectedQuery = tagQueryString ? uri.query : null + def otelSemantics = Config.get().isTraceOtelSemanticsEnabled() + def expectedQuery = otelSemantics || tagQueryString ? uri.query : null def expectedUrl = URIUtils.buildURL(uri.scheme, uri.host, uri.port, uri.path) if (expectedQuery != null && !expectedQuery.empty) { expectedUrl = "$expectedUrl?$expectedQuery" } + if (otelSemantics && emitsUrlFragment() && uri.rawFragment != null) { + expectedUrl = "$expectedUrl#$uri.rawFragment" + } trace.span { if (parentSpan == null) { parent() @@ -1005,28 +1012,40 @@ abstract class HttpClientTest extends VersionedNamingTestBase { serviceName uri.host } operationName operation() - resourceName "$method $uri.path" + resourceName otelSemantics ? OtelHttpSemantics.spanNameMethod(method) : "$method $uri.path" spanType DDSpanTypes.HTTP_CLIENT - errored error + errored otelSemantics && status != null && status >= 400 ? true : error measured true tags { "$Tags.COMPONENT" component "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT - "$Tags.PEER_HOSTNAME" { - it == uri.host || ignorePeer - } - "$Tags.PEER_HOST_IPV4" { - it == null || it == "127.0.0.1" || ignorePeer - } // Optional - "$Tags.PEER_PORT" { - it == null || it == uri.port || it == proxy.port || it == 443 || ignorePeer + if (otelSemantics) { + "$Tags.PEER_HOSTNAME" null + "$Tags.PEER_HOST_IPV4" null + "$Tags.PEER_PORT" null + "$KnownTags.SERVER_ADDRESS_NAME" uri.host + "$KnownTags.SERVER_PORT_NAME" OtelHttpSemantics.serverPort(uri) + } else { + "$Tags.PEER_HOSTNAME" { + it == uri.host || ignorePeer + } + "$Tags.PEER_HOST_IPV4" { + it == null || it == "127.0.0.1" || ignorePeer + } // Optional + "$Tags.PEER_PORT" { + it == null || it == uri.port || it == proxy.port || it == 443 || ignorePeer + } } "$Tags.HTTP_URL" expectedUrl "$Tags.HTTP_METHOD" method if (status) { "$Tags.HTTP_STATUS" status } - if (tagQueryString) { + if (otelSemantics) { + "$DDTags.HTTP_QUERY" null + "$KnownTags.URL_QUERY_NAME" null + "$DDTags.HTTP_FRAGMENT" null + } else if (tagQueryString) { "$DDTags.HTTP_QUERY" expectedQuery "$DDTags.HTTP_FRAGMENT" { it == null || it == uri.fragment @@ -1041,9 +1060,15 @@ abstract class HttpClientTest extends VersionedNamingTestBase { } if (exception) { this.assertErrorTags(it, exception) + } else if (otelSemantics && status != null && status >= 400) { + "$DDTags.ERROR_TYPE" status.toString() + } + if (otelSemantics) { + defaultTagsNoPeerService() + } else { + peerServiceFrom(Tags.PEER_HOSTNAME) + defaultTags() } - peerServiceFrom(Tags.PEER_HOSTNAME) - defaultTags() if (extraTags) { it.addTags(extraTags) } @@ -1095,6 +1120,10 @@ abstract class HttpClientTest extends VersionedNamingTestBase { true } + boolean emitsUrlFragment() { + true + } + boolean testAppSecClientRequest() { false } diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index ed867b49643..22ca8eade56 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -11,6 +11,7 @@ import datadog.trace.agent.test.utils.OkHttpUtils import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags +import datadog.trace.api.KnownTags import datadog.trace.api.ProductActivation import datadog.trace.api.config.GeneralConfig import datadog.trace.api.config.TracerConfig @@ -33,6 +34,7 @@ import datadog.trace.api.telemetry.EndpointCollector import datadog.trace.bootstrap.blocking.BlockingActionHelper import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics import datadog.trace.bootstrap.instrumentation.api.SpanAttributes import datadog.trace.bootstrap.instrumentation.api.SpanLink import datadog.trace.bootstrap.instrumentation.api.Tags @@ -199,7 +201,32 @@ abstract class HttpServerTest extends WithHttpServer { abstract String expectedOperationName() - String expectedResourceName(ServerEndpoint endpoint, String method, URI address) { + Serializable expectedResourceName(ServerEndpoint endpoint, String method, URI address) { + if (Config.get().isTraceOtelSemanticsEnabled()) { + def route = expectedServerSpanRoute(endpoint) + def nameMethod = OtelHttpSemantics.spanNameMethod(method) + if (route == null) { + return nameMethod + } + if (route instanceof CharSequence) { + return "$nameMethod $route" + } + return { + String resourceName -> + def prefix = "$nameMethod " + if (!resourceName.startsWith(prefix)) { + return false + } + def resourceRoute = resourceName.substring(prefix.length()) + if (route instanceof Class) { + return route.isInstance(resourceRoute) + } + if (route instanceof Closure) { + return route.call(resourceRoute) + } + return false + } + } if (endpoint.status == 404 && (changesAll404s() || endpoint.path == "/not-found")) { return "404" } else if (endpoint.hasPathParam) { @@ -241,7 +268,8 @@ abstract class HttpServerTest extends WithHttpServer { it == null || it == EXCEPTION.body }, "error.type" : { - it == null || it == Exception.name + it == null || it == Exception.name || + (Config.get().isTraceOtelSemanticsEnabled() && it == endpoint.status.toString()) }, "error.stack" : { it == null || it instanceof String @@ -877,12 +905,13 @@ abstract class HttpServerTest extends WithHttpServer { } } } - recordedBaggageTags == [ + def expectedBaggageTags = Config.get().isInjectBaggageAsTagsEnabled() ? [ "baggage.user.id" : "test-user", "baggage.session.id": "test-session", "baggage.account.id": "test-account" // "baggage.language" should NOT be present since it's not in default config - ] + ] : [:] + recordedBaggageTags == expectedBaggageTags and: if (isDataStreamsEnabled()) { @@ -2454,11 +2483,10 @@ abstract class HttpServerTest extends WithHttpServer { trace.span { operationName operation - if (handshake.getTag(Tags.HTTP_ROUTE) != null) { - resourceName "websocket ${handshake.getTag(Tags.HTTP_ROUTE) as String}" - } else { - resourceName "websocket ${URI.create(handshake.getTag(Tags.HTTP_URL) as String).path}" - } + def handshakeResource = handshake.resourceName.toString() + def separator = handshakeResource.indexOf(' ') + def websocketResource = separator < 0 ? handshakeResource : handshakeResource.substring(separator + 1) + resourceName "websocket $websocketResource" if (traceStarter && Config.get().isWebsocketMessagesSeparateTraces()) { parent() } else { @@ -2613,15 +2641,19 @@ abstract class HttpServerTest extends WithHttpServer { boolean hasForwardedIP = hasForwardedIP() def expectedExtraServerTags = expectedExtraServerTags(endpoint) def expectedStatus = expectedStatus(endpoint) + def expectedIsErrored = expectedErrored(endpoint) def expectedQueryTag = expectedQueryTag(endpoint) def expectedUrl = expectedUrl(endpoint, address) def expectedIntegrationName = expectedIntegrationName() + def otelSemantics = Config.get().isTraceOtelSemanticsEnabled() + def requestUrl = endpoint.resolve(address) + def expectedPath = Config.get().isHttpServerRawResource() && supportsRaw() ? requestUrl.rawPath : requestUrl.path trace.span { serviceName expectedServiceName() operationName operation() resourceName expectedResourceName(endpoint, method, address) spanType DDSpanTypes.HTTP_SERVER - errored expectedErrored(endpoint) + errored expectedIsErrored if (parentID != null) { traceId traceID parentSpanId parentID @@ -2649,8 +2681,17 @@ abstract class HttpServerTest extends WithHttpServer { "$Tags.NETWORK_CLIENT_IP" null "$Tags.HTTP_CLIENT_IP" clientIp } - "$Tags.HTTP_HOSTNAME" address.host - "$Tags.HTTP_URL" "$expectedUrl" + if (otelSemantics) { + "$Tags.HTTP_URL" null + "$Tags.HTTP_HOSTNAME" null + "$KnownTags.SERVER_ADDRESS_NAME" address.host + "$KnownTags.URL_PATH_NAME" expectedPath + "$KnownTags.URL_SCHEME_NAME" requestUrl.scheme + "$KnownTags.SERVER_PORT_NAME" requestUrl.port + } else { + "$Tags.HTTP_HOSTNAME" address.host + "$Tags.HTTP_URL" "$expectedUrl" + } "$Tags.HTTP_METHOD" method "$Tags.HTTP_STATUS" expectedStatus "$Tags.HTTP_USER_AGENT" String @@ -2666,8 +2707,17 @@ abstract class HttpServerTest extends WithHttpServer { if (null != expectedExtraErrorInformation) { addTags(expectedExtraErrorInformation) } + if (otelSemantics && expectedIsErrored && + (expectedExtraErrorInformation == null || !expectedExtraErrorInformation.containsKey(DDTags.ERROR_TYPE))) { + "$DDTags.ERROR_TYPE" endpoint.status.toString() + } if (endpoint.query) { - "$DDTags.HTTP_QUERY" expectedQueryTag + if (otelSemantics) { + "$KnownTags.URL_QUERY_NAME" expectedQueryTag + "$DDTags.HTTP_QUERY" null + } else { + "$DDTags.HTTP_QUERY" expectedQueryTag + } } if ({ isDataStreamsEnabled() diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/NamingConventions.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/NamingConventions.groovy index 06f40257318..aefb4e3d6fa 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/NamingConventions.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/NamingConventions.groovy @@ -1,5 +1,9 @@ package datadog.trace.agent.test.naming +/** Marks a forked HTTP test suite that runs with OpenTelemetry HTTP semantics enabled. */ +interface TestingOtelHttpSemantics { +} + class TestingGenericHttpNamingConventions { trait ClientV0 implements VersionedNamingTest { @Override diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/VersionedNamingTestBase.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/VersionedNamingTestBase.groovy index 5bc5f1a9eaf..3ed5da2290a 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/VersionedNamingTestBase.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/naming/VersionedNamingTestBase.groovy @@ -1,6 +1,7 @@ package datadog.trace.agent.test.naming import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.api.config.GeneralConfig import datadog.trace.api.config.TracerConfig abstract class VersionedNamingTestBase extends InstrumentationSpecification implements VersionedNamingTest { @@ -10,5 +11,8 @@ abstract class VersionedNamingTestBase extends InstrumentationSpecification impl protected void configurePreAgent() { super.configurePreAgent() injectSysConfig(TracerConfig.TRACE_SPAN_ATTRIBUTE_SCHEMA, "v" + version()) + if (this instanceof TestingOtelHttpSemantics) { + injectSysConfig(GeneralConfig.TRACE_OTEL_SEMANTICS_ENABLED, "true") + } } } diff --git a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/groovy/AkkaHttpServerInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/groovy/AkkaHttpServerInstrumentationTest.groovy index 2d807d5e215..c4619379863 100644 --- a/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/groovy/AkkaHttpServerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/baseTest/groovy/AkkaHttpServerInstrumentationTest.groovy @@ -1,6 +1,7 @@ import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.api.config.TraceInstrumentationConfig import datadog.trace.test.util.ThreadUtils import datadog.trace.instrumentation.akkahttp.AkkaHttpServerDecorator @@ -275,6 +276,9 @@ class AkkaHttpServerInstrumentationSyncV0ForkedTest extends AkkaHttpServerInstru class AkkaHttpServerInstrumentationSyncV1ForkedTest extends AkkaHttpServerInstrumentationSyncTest implements TestingGenericHttpNamingConventions.ServerV1 { } +class AkkaHttpServerInstrumentationSyncOtelSemanticsForkedTest extends AkkaHttpServerInstrumentationSyncV0ForkedTest implements TestingOtelHttpSemantics { +} + class AkkaHttpServerInstrumentationAsyncTest extends AkkaHttpServerInstrumentationTest { @Override HttpServer server() { diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/groovy/ApacheHttpAsyncClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/groovy/ApacheHttpAsyncClientTest.groovy index bc57def64cf..96fe31e3a6a 100644 --- a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/groovy/ApacheHttpAsyncClientTest.groovy +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/groovy/ApacheHttpAsyncClientTest.groovy @@ -1,6 +1,7 @@ import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator import org.apache.http.HttpHost import org.apache.http.HttpResponse @@ -100,6 +101,9 @@ class ApacheHttpAsyncClientV0Test extends ApacheHttpAsyncClientTest implements T class ApacheHttpAsyncClientV1ForkedTest extends ApacheHttpAsyncClientTest implements TestingGenericHttpNamingConventions.ClientV1 { } +class ApacheHttpAsyncClientOtelSemanticsForkedTest extends ApacheHttpAsyncClientTest implements TestingGenericHttpNamingConventions.ClientV0, TestingOtelHttpSemantics { +} + class ApacheHttpAsyncClientHostRequestTest extends ApacheHttpAsyncClientV0Test { def relativizeUri(URI uri) { diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/test/groovy/ApacheHttpClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/test/groovy/ApacheHttpClientTest.groovy index 1d34b88381e..684303bd16b 100644 --- a/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/test/groovy/ApacheHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpclient-4.0/src/test/groovy/ApacheHttpClientTest.groovy @@ -1,5 +1,6 @@ import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.instrumentation.apachehttpclient.ApacheHttpClientDecorator import org.apache.http.HttpHost import org.apache.http.HttpRequest @@ -195,3 +196,7 @@ class ApacheClientUriRequestResponseHandlerContextV0ForkedTest extends ApacheCli @Timeout(5) class ApacheClientUriRequestResponseHandlerContextV1ForkedTest extends ApacheClientUriRequestResponseHandlerContext implements TestingGenericHttpNamingConventions.ClientV1 { } + +@Timeout(5) +class ApacheClientOtelSemanticsForkedTest extends ApacheClientUriRequestResponseHandlerContext implements TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/axway-api-7.5/src/main/java/datadog/trace/instrumentation/axway/AxwayHTTPPluginDecorator.java b/dd-java-agent/instrumentation/axway-api-7.5/src/main/java/datadog/trace/instrumentation/axway/AxwayHTTPPluginDecorator.java index 10ce28bdd1b..756d4fec11e 100644 --- a/dd-java-agent/instrumentation/axway-api-7.5/src/main/java/datadog/trace/instrumentation/axway/AxwayHTTPPluginDecorator.java +++ b/dd-java-agent/instrumentation/axway-api-7.5/src/main/java/datadog/trace/instrumentation/axway/AxwayHTTPPluginDecorator.java @@ -2,8 +2,11 @@ import static java.lang.invoke.MethodType.methodType; +import datadog.trace.api.Config; +import datadog.trace.api.KnownTags; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.URIDefaultDataAdapter; @@ -22,6 +25,7 @@ // request = is com.vordel.circuit.net.State, connection = com.vordel.dwe.http.ServerTransaction public class AxwayHTTPPluginDecorator extends HttpServerDecorator { private static final Logger log = LoggerFactory.getLogger(AxwayHTTPPluginDecorator.class); + private static final boolean OTEL_SEMANTICS = Config.get().isTraceOtelSemanticsEnabled(); public static final CharSequence AXWAY_TRY_TRANSACTION = UTF8BytesString.create("axway.trytransaction"); @@ -198,13 +202,47 @@ protected int status(final Object serverTransaction) { */ public void onTransaction(AgentSpan span, Object stateInstance) { if (span != null) { - setStringTagFromStateField(span, Tags.PEER_HOSTNAME, stateInstance, hostField_mh); - setStringTagFromStateField(span, Tags.PEER_PORT, stateInstance, portField_mh); - setStringTagFromStateField(span, Tags.HTTP_METHOD, stateInstance, methodField_mh); - setURLTagFromUriStateField(span, stateInstance); + if (OTEL_SEMANTICS) { + setOtelTransactionTags(span, stateInstance); + } else { + setStringTagFromStateField(span, Tags.PEER_HOSTNAME, stateInstance, hostField_mh); + setStringTagFromStateField(span, Tags.PEER_PORT, stateInstance, portField_mh); + setStringTagFromStateField(span, Tags.HTTP_METHOD, stateInstance, methodField_mh); + setURLTagFromUriStateField(span, stateInstance); + } } } + private static void setOtelTransactionTags(AgentSpan span, Object stateInstance) { + String method = getStringFromStateField(stateInstance, methodField_mh); + OtelHttpSemantics.setRequestMethod(span, method); + OtelHttpSemantics.setMethodResourceName(span, method); + + URI uri = getUriFromStateField(stateInstance); + String scheme = null; + String host = getStringFromStateField(stateInstance, hostField_mh); + int port = OtelHttpSemantics.parsePort(getStringFromStateField(stateInstance, portField_mh)); + if (uri != null) { + scheme = uri.getScheme(); + if (uri.getPath() != null) { + span.setTag(KnownTags.URL_PATH_NAME, uri.getPath()); + } + if (uri.getRawQuery() != null && !uri.getRawQuery().isEmpty()) { + span.setTag(KnownTags.URL_QUERY_NAME, uri.getRawQuery()); + } + if (host == null || host.isEmpty()) { + host = uri.getHost(); + } + if (port <= 0) { + port = uri.getPort(); + } + } + if (scheme != null) { + span.setTag(KnownTags.URL_SCHEME_NAME, scheme); + } + OtelHttpSemantics.setServerAddressAndPort(span, null, null, scheme, host, port); + } + /** * class hierarchy in package com.vordel.dwe.http : * @@ -245,6 +283,16 @@ private static void setStringTagFromStateField( span.setTag(tag, v); } + private static String getStringFromStateField(Object stateInstance, MethodHandle mh) { + try { + Object value = mh.invoke(stateInstance); + return value == null ? null : value.toString(); + } catch (Throwable e) { + log.debug("Can't invoke '{}' on instance '{}'; value not read.", mh, stateInstance, e); + return null; + } + } + private static void setURLTagFromUriStateField(AgentSpan span, Object stateInstance) { try { span.setTag(Tags.HTTP_URL, uriField_mh.invoke(stateInstance).toString()); @@ -257,4 +305,14 @@ private static void setURLTagFromUriStateField(AgentSpan span, Object stateInsta e); } } + + private static URI getUriFromStateField(Object stateInstance) { + try { + Object value = uriField_mh.invoke(stateInstance); + return value == null ? null : URI.create(value.toString()); + } catch (Throwable e) { + log.debug("Can't invoke '{}' on instance '{}'; URL not read.", uriField_mh, stateInstance, e); + return null; + } + } } diff --git a/dd-java-agent/instrumentation/google-http-client-1.19/src/test/groovy/GoogleHttpClientTest.groovy b/dd-java-agent/instrumentation/google-http-client-1.19/src/test/groovy/GoogleHttpClientTest.groovy index 5efcd6d7acb..0b606c320f5 100644 --- a/dd-java-agent/instrumentation/google-http-client-1.19/src/test/groovy/GoogleHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/google-http-client-1.19/src/test/groovy/GoogleHttpClientTest.groovy @@ -1,6 +1,7 @@ import com.google.api.client.http.HttpRequest import com.google.api.client.http.HttpResponse import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import spock.lang.Timeout abstract class GoogleHttpClientTest extends AbstractGoogleHttpClientTest { @@ -18,3 +19,7 @@ class GoogleHttpClientV0ForkedTest extends GoogleHttpClientTest implements Testi @Timeout(5) class GoogleHttpClientV1ForkedTest extends GoogleHttpClientTest implements TestingGenericHttpNamingConventions.ClientV1 { } + +@Timeout(5) +class GoogleHttpClientOtelSemanticsForkedTest extends GoogleHttpClientTest implements TestingGenericHttpNamingConventions.ClientV0, TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/groovy/HttpUrlConnectionTest.groovy b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/groovy/HttpUrlConnectionTest.groovy index 52d45844a33..7222b55e90a 100644 --- a/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/groovy/HttpUrlConnectionTest.groovy +++ b/dd-java-agent/instrumentation/java/java-net/java-net-1.8/src/test/groovy/HttpUrlConnectionTest.groovy @@ -4,6 +4,7 @@ import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.bootstrap.instrumentation.httpurlconnection.HttpUrlConnectionDecorator @@ -380,3 +381,6 @@ class HttpUrlConnectionV0ForkedTest extends HttpUrlConnectionTest implements Tes class HttpUrlConnectionV1ForkedTest extends HttpUrlConnectionTest implements TestingGenericHttpNamingConventions.ClientV1 { } + +class HttpUrlConnectionOtelSemanticsForkedTest extends HttpUrlConnectionTest implements TestingGenericHttpNamingConventions.ClientV0, TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/jersey/jersey-2.0/build.gradle b/dd-java-agent/instrumentation/jersey/jersey-2.0/build.gradle index 0bb3e609294..9e7a5ce57fc 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-2.0/build.gradle +++ b/dd-java-agent/instrumentation/jersey/jersey-2.0/build.gradle @@ -19,6 +19,7 @@ testJvmConstraints { // there are tests with jersey2 and grizzly on the grizzly-http-2.3.20 module addTestSuiteForDir('jersey2JettyTest', 'jersey2JettyTest') +addForkedTestTask('jersey2JettyTest') addTestSuiteForDir('jersey3JettyTest', 'jersey3JettyTest') addTestSuiteForDir('latestDepTest', 'test') diff --git a/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy b/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy index b2d8dd20751..cdff4d99d26 100644 --- a/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy +++ b/dd-java-agent/instrumentation/jersey/jersey-2.0/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/Jersey2JettyTest.groovy @@ -2,6 +2,7 @@ package datadog.trace.instrumentation.jersey2 import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import javax.ws.rs.NotFoundException import javax.ws.rs.core.Response @@ -89,3 +90,6 @@ class Jersey2JettyTest extends HttpServerTest { } } } + +class Jersey2OtelSemanticsForkedTest extends Jersey2JettyTest implements TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy index fd80aa18479..0a7aeca9982 100644 --- a/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy +++ b/dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy @@ -1,6 +1,7 @@ import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.instrumentation.servlet5.HtmlRumServlet import datadog.trace.instrumentation.servlet5.TestServlet5 import datadog.trace.instrumentation.servlet5.XmlRumServlet @@ -162,6 +163,9 @@ class Jetty11V1ForkedTest extends Jetty11Test implements TestingGenericHttpNamin } } +class Jetty11OtelSemanticsForkedTest extends Jetty11Test implements TestingGenericHttpNamingConventions.ServerV0, TestingOtelHttpSemantics { +} + class JettyRumInjectionForkedTest extends Jetty11V0ForkedTest { @Override boolean testRumInjection() { diff --git a/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy index 1c4831d0ce5..3faf3c13257 100644 --- a/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy +++ b/dd-java-agent/instrumentation/netty/netty-4.1/src/test/groovy/Netty41ServerTest.groovy @@ -4,6 +4,7 @@ import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.base.WebsocketClient import datadog.trace.agent.test.base.WebsocketServer import datadog.trace.agent.test.naming.TestingNettyHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.bootstrap.instrumentation.api.URIUtils import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator import io.netty.bootstrap.ServerBootstrap @@ -335,6 +336,9 @@ class Netty41ServerV0Test extends Netty41ServerTest implements TestingNettyHttpN class Netty41ServerV1ForkedTest extends Netty41ServerTest implements TestingNettyHttpNamingConventions.ServerV1 { } +class Netty41ServerOtelSemanticsForkedTest extends Netty41ServerTest implements TestingNettyHttpNamingConventions.ServerV0, TestingOtelHttpSemantics { +} + class WsEndpoint { static volatile ChannelHandlerContext activeSession @@ -351,4 +355,3 @@ class WsEndpoint { } } - diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/groovy/OkHttp3Test.groovy b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/groovy/OkHttp3Test.groovy index 2aed1c0682a..3b414f4185c 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/groovy/OkHttp3Test.groovy +++ b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/groovy/OkHttp3Test.groovy @@ -1,5 +1,6 @@ import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator import okhttp3.* import okhttp3.internal.http.HttpMethod @@ -109,3 +110,22 @@ class OkHttp3V0ForkedTest extends OkHttp3Test { @Timeout(5) class OkHttp3V1ForkedTest extends OkHttp3Test implements TestingGenericHttpNamingConventions.ClientV1 { } + +@Timeout(5) +class OkHttp3OtelSemanticsForkedTest extends OkHttp3Test implements TestingOtelHttpSemantics { + + @Override + int version() { + return 0 + } + + @Override + String service() { + return null + } + + @Override + String operation() { + return "okhttp.request" + } +} diff --git a/dd-java-agent/instrumentation/play-ws/play-ws-2.1/src/test/groovy/PlayWSClientTest.groovy b/dd-java-agent/instrumentation/play-ws/play-ws-2.1/src/test/groovy/PlayWSClientTest.groovy index 72a5b41be3a..dae498722e2 100644 --- a/dd-java-agent/instrumentation/play-ws/play-ws-2.1/src/test/groovy/PlayWSClientTest.groovy +++ b/dd-java-agent/instrumentation/play-ws/play-ws-2.1/src/test/groovy/PlayWSClientTest.groovy @@ -1,3 +1,4 @@ +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import play.libs.ws.StandaloneWSClient import play.libs.ws.StandaloneWSRequest import play.libs.ws.StandaloneWSResponse @@ -8,6 +9,7 @@ import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.concurrent.duration.Duration import spock.lang.Shared +import spock.lang.Timeout import java.util.concurrent.TimeUnit @@ -129,3 +131,7 @@ class PlayScalaStreamedWSClientTest extends PlayWSClientTestBase { wsClient?.close() } } + +@Timeout(5) +class PlayWSClientOtelSemanticsForkedTest extends PlayJavaWSClientTest implements TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/play-ws/play-ws-common/src/test/groovy/PlayWSClientTestBase.groovy b/dd-java-agent/instrumentation/play-ws/play-ws-common/src/test/groovy/PlayWSClientTestBase.groovy index f5c0ffbb471..820a8886812 100644 --- a/dd-java-agent/instrumentation/play-ws/play-ws-common/src/test/groovy/PlayWSClientTestBase.groovy +++ b/dd-java-agent/instrumentation/play-ws/play-ws-common/src/test/groovy/PlayWSClientTestBase.groovy @@ -57,6 +57,11 @@ abstract class PlayWSClientTestBase extends HttpClientTest { return false } + @Override + boolean emitsUrlFragment() { + return false + } + @Override int version() { return 0 diff --git a/dd-java-agent/instrumentation/play/play-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy index 5f42d474c10..a12670a1285 100644 --- a/dd-java-agent/instrumentation/play/play-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-2.6/src/test/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy @@ -1,5 +1,7 @@ package datadog.trace.instrumentation.play26.server +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics + class PlayServerTest extends AbstractPlayServerTest { // Disabling involving Appsec / Instrumentation Gateway, they are tested in play-appsec-2.x modules @@ -48,3 +50,6 @@ class PlayServerTest extends AbstractPlayServerTest { false } } + +class PlayServerOtelSemanticsForkedTest extends PlayServerTest implements TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/play/play-2.6/src/testFixtures/groovy/datadog/trace/instrumentation/play26/server/AbstractPlayServerTest.groovy b/dd-java-agent/instrumentation/play/play-2.6/src/testFixtures/groovy/datadog/trace/instrumentation/play26/server/AbstractPlayServerTest.groovy index 8edf9005759..358e2028fac 100644 --- a/dd-java-agent/instrumentation/play/play-2.6/src/testFixtures/groovy/datadog/trace/instrumentation/play26/server/AbstractPlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play/play-2.6/src/testFixtures/groovy/datadog/trace/instrumentation/play26/server/AbstractPlayServerTest.groovy @@ -3,8 +3,10 @@ package datadog.trace.instrumentation.play26.server import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags +import datadog.trace.api.KnownTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.play26.PlayHttpServerDecorator import groovy.transform.CompileStatic @@ -125,6 +127,9 @@ class AbstractPlayServerTest extends HttpServerTest { @Override void handlerSpan(TraceAssert trace, ServerEndpoint endpoint = SUCCESS) { def expectedQueryTag = expectedQueryTag(endpoint) + def otelSemantics = Config.get().isTraceOtelSemanticsEnabled() + def requestUrl = endpoint.resolve(address) + def expectedPath = Config.get().isHttpServerRawResource() && supportsRaw() ? requestUrl.rawPath : requestUrl.path trace.span { serviceName expectedServiceName() operationName "play.request" @@ -137,8 +142,17 @@ class AbstractPlayServerTest extends HttpServerTest { "$Tags.PEER_HOST_IPV4" '127.0.0.1' "$Tags.HTTP_CLIENT_IP" (endpoint == FORWARDED ? endpoint.body : '127.0.0.1') "$Tags.NETWORK_CLIENT_IP" '127.0.0.1' - "$Tags.HTTP_URL" String - "$Tags.HTTP_HOSTNAME" address.host + if (otelSemantics) { + "$Tags.HTTP_URL" null + "$Tags.HTTP_HOSTNAME" null + "$KnownTags.SERVER_ADDRESS_NAME" address.host + "$KnownTags.SERVER_PORT_NAME" address.port + "$KnownTags.URL_PATH_NAME" expectedPath + "$KnownTags.URL_SCHEME_NAME" address.scheme + } else { + "$Tags.HTTP_URL" String + "$Tags.HTTP_HOSTNAME" address.host + } "$Tags.HTTP_METHOD" String // BUG // "$Tags.HTTP_ROUTE" String @@ -146,7 +160,12 @@ class AbstractPlayServerTest extends HttpServerTest { errorTags(endpoint == CUSTOM_EXCEPTION ? TestHttpErrorHandler.CustomRuntimeException : RuntimeException, endpoint.body) } if (endpoint.query) { - "$DDTags.HTTP_QUERY" expectedQueryTag + if (otelSemantics) { + "$KnownTags.URL_QUERY_NAME" expectedQueryTag + "$DDTags.HTTP_QUERY" null + } else { + "$DDTags.HTTP_QUERY" expectedQueryTag + } } defaultTags() } diff --git a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy index 0b665683b2b..68e3f6ecf3a 100644 --- a/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/test/groovy/server/RatpackHttpServerTest.groovy @@ -3,8 +3,11 @@ package server import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags +import datadog.trace.api.KnownTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator import datadog.trace.instrumentation.ratpack.RatpackServerDecorator @@ -33,15 +36,6 @@ class RatpackHttpServerTest extends HttpServerTest { "netty.request" } - @Override - String expectedResourceName(ServerEndpoint endpoint, String method, URI address) { - if (endpoint == PATH_PARAM) { - 'GET /path/:id/param' - } else { - super.expectedResourceName(endpoint, method, address) - } - } - @Override Map expectedIGPathParams() { [id: '123'] @@ -89,7 +83,7 @@ class RatpackHttpServerTest extends HttpServerTest { @Override String testPathParam() { - true + '/path/:id/param' } @Override @@ -110,7 +104,7 @@ class RatpackHttpServerTest extends HttpServerTest { @Override Serializable expectedServerSpanRoute(ServerEndpoint endpoint) { - return String + return endpoint == PATH_PARAM ? testPathParam() : endpoint.rawPath } @Override @@ -126,6 +120,9 @@ class RatpackHttpServerTest extends HttpServerTest { @Override void handlerSpan(TraceAssert trace, ServerEndpoint endpoint = SUCCESS) { + def otelSemantics = Config.get().isTraceOtelSemanticsEnabled() + def requestUrl = endpoint.resolve(address) + def expectedRoute = expectedServerSpanRoute(endpoint) trace.span { serviceName expectedServiceName() operationName "ratpack.handler" @@ -137,21 +134,35 @@ class RatpackHttpServerTest extends HttpServerTest { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" // This span ignores "x-forwards-from". "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" String - "$Tags.HTTP_HOSTNAME" "${address.host}" + if (otelSemantics) { + "$Tags.HTTP_URL" null + "$Tags.HTTP_HOSTNAME" null + "$KnownTags.SERVER_ADDRESS_NAME" address.host + "$KnownTags.SERVER_PORT_NAME" address.port + "$KnownTags.URL_PATH_NAME" requestUrl.rawPath + "$KnownTags.URL_SCHEME_NAME" address.scheme + } else { + "$Tags.HTTP_URL" String + "$Tags.HTTP_HOSTNAME" "${address.host}" + } "$Tags.HTTP_METHOD" String "$Tags.HTTP_STATUS" Integer - "$Tags.HTTP_ROUTE" String + "$Tags.HTTP_ROUTE" expectedRoute "$Tags.HTTP_CLIENT_IP" (endpoint == FORWARDED ? endpoint.body : '127.0.0.1') "$Tags.NETWORK_CLIENT_IP" '127.0.0.1' if (endpoint == EXCEPTION) { errorTags(Exception, EXCEPTION.body) + } else if (otelSemantics && endpoint == ERROR) { + "$DDTags.ERROR_TYPE" endpoint.status.toString() } if (endpoint.query) { - "$DDTags.HTTP_QUERY" endpoint.rawQuery + tag(otelSemantics ? KnownTags.URL_QUERY_NAME : DDTags.HTTP_QUERY, endpoint.rawQuery) } defaultTags() } } } } + +class RatpackHttpServerOtelSemanticsForkedTest extends RatpackHttpServerTest implements TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy index fe542a6baac..5d1e4c107c6 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy @@ -1,4 +1,6 @@ import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.api.Config +import datadog.trace.api.config.GeneralConfig import datadog.trace.api.config.TraceInstrumentationConfig import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.jaxrs2.JaxRsAnnotationsDecorator @@ -54,7 +56,7 @@ class JaxRsAnnotations2InstrumentationTest extends InstrumentationSpecification trace(2) { span { operationName "test" - resourceName name + resourceName(Config.get().isTraceOtelSemanticsEnabled() ? otelName : name) parent() tags { "$Tags.COMPONENT" "jax-rs" @@ -77,65 +79,65 @@ class JaxRsAnnotations2InstrumentationTest extends InstrumentationSpecification } where: - name | obj - "/a" | new Jax() { + name | otelName | obj + "/a" | "HTTP /a" | new Jax() { @Path("/a") void call() { } } - "GET /b" | new Jax() { + "GET /b" | "GET /b" | new Jax() { @GET @Path("/b") void call() { } } - "POST /interface/c" | new InterfaceWithPath() { + "POST /interface/c" | "POST /interface/c" | new InterfaceWithPath() { @POST @Path("/c") void call() { } } - "HEAD /interface" | new InterfaceWithPath() { + "HEAD /interface" | "HEAD /interface" | new InterfaceWithPath() { @HEAD void call() { } } - "POST /abstract/d" | new AbstractClassWithPath() { + "POST /abstract/d" | "POST /abstract/d" | new AbstractClassWithPath() { @POST @Path("/d") void call() { } } - "PATCH /interface" | new InterfaceWithPath() { + "PATCH /interface" | "PATCH /interface" | new InterfaceWithPath() { @PATCH void call() { } } - "CUSTOM /interface" | new InterfaceWithPath() { + "CUSTOM /interface" | "HTTP /interface" | new InterfaceWithPath() { @CustomMethod void call() { } } - "PUT /abstract" | new AbstractClassWithPath() { + "PUT /abstract" | "PUT /abstract" | new AbstractClassWithPath() { @PUT void call() { } } - "OPTIONS /child/e" | new ChildClassWithPath() { + "OPTIONS /child/e" | "OPTIONS /child/e" | new ChildClassWithPath() { @OPTIONS @Path("/e") void call() { } } - "DELETE /child/call" | new ChildClassWithPath() { + "DELETE /child/call" | "DELETE /child/call" | new ChildClassWithPath() { @DELETE void call() { } } - "POST /child/call" | new ChildClassWithPath() - "GET /child/call" | new JavaInterfaces.ChildClassOnInterface() + "POST /child/call" | "POST /child/call" | new ChildClassWithPath() + "GET /child/call" | "GET /child/call" | new JavaInterfaces.ChildClassOnInterface() // TODO: uncomment when we drop support for Java 7 - // "GET /child/invoke" | new JavaInterfaces.DefaultChildClassOnInterface() + // "GET /child/invoke" | "GET /child/invoke" | new JavaInterfaces.DefaultChildClassOnInterface() className = JaxRsAnnotationsDecorator.DECORATE.className(obj.class) } @@ -188,3 +190,11 @@ class JaxRsAnnotations2InstrumentationTest extends InstrumentationSpecification } } } + +class JaxRsAnnotations2OtelSemanticsForkedTest extends JaxRsAnnotations2InstrumentationTest { + @Override + protected void configurePreAgent() { + super.configurePreAgent() + injectSysConfig(GeneralConfig.TRACE_OTEL_SEMANTICS_ENABLED, "true") + } +} diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientAsyncTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientAsyncTest.groovy index 0af25e9b290..01c31059cf1 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientAsyncTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientAsyncTest.groovy @@ -1,6 +1,7 @@ import datadog.environment.JavaVirtualMachine import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl import org.glassfish.jersey.client.ClientConfig @@ -94,6 +95,10 @@ class JerseyClientAsyncTest extends JaxRsClientAsyncTest { } } +@Timeout(5) +class JerseyClientOtelSemanticsForkedTest extends JerseyClientAsyncTest implements TestingOtelHttpSemantics { +} + abstract class ResteasyClientAsyncTest extends JaxRsClientAsyncTest { @Override diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientTest.groovy index 78660157452..964231d9015 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-client/jax-rs-client-2.0/src/test/groovy/JaxRsClientTest.groovy @@ -1,6 +1,7 @@ import datadog.environment.JavaVirtualMachine import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator import datadog.trace.test.util.Flaky import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl @@ -101,6 +102,10 @@ class ResteasyClientV0ForkedTest extends ResteasyClientTest { class ResteasyClientV1ForkedTest extends ResteasyClientTest implements TestingGenericHttpNamingConventions.ClientV1 { } +@Timeout(5) +class JaxRsClientOtelSemanticsForkedTest extends ResteasyClientTest implements TestingOtelHttpSemantics { +} + @Timeout(5) @IgnoreIf({ // TODO Java 17: This version of apache-cxf doesn't work on Java 17 diff --git a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy index 1a4ef433990..34fb9c4bc46 100644 --- a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy @@ -1,6 +1,7 @@ import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.servlet3.AsyncDispatcherDecorator @@ -254,6 +255,9 @@ class JettyServlet3SyncRumInjectionForkedTest extends JettyServlet3TestSync { class JettyServlet3SyncV1ForkedTest extends JettyServlet3TestSync implements TestingGenericHttpNamingConventions.ServerV1 { } +class JettyServlet3SyncOtelSemanticsForkedTest extends JettyServlet3TestSync implements TestingOtelHttpSemantics { +} + class JettyServlet3TestAsync extends JettyServlet3Test { @Override diff --git a/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/build.gradle b/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/build.gradle index b64728243d7..f56484366de 100644 --- a/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/build.gradle +++ b/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/build.gradle @@ -9,6 +9,7 @@ testJvmConstraints { // test that webflux5 instrumentation works for webflux6 too addTestSuite('iastTest') addTestSuite('bootTest') +addForkedTestTask('bootTest') addTestSuiteForDir('latestDepTest', 'test') addTestSuiteExtendingForDir('latestDepForkedTest', 'latestDepTest', 'test') addTestSuiteExtendingForDir('latestDepBootTest', 'latestDepTest', 'bootTest') diff --git a/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/src/bootTest/groovy/SpringWebfluxTest.groovy b/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/src/bootTest/groovy/SpringWebfluxTest.groovy index 61b73aed5eb..66a2ccd79a4 100644 --- a/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/src/bootTest/groovy/SpringWebfluxTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webflux/spring-webflux-6.0/src/bootTest/groovy/SpringWebfluxTest.groovy @@ -10,8 +10,13 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_ANNOTATI import datadog.trace.agent.test.InstrumentationSpecification import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.asserts.TagsAssert +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics +import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags +import datadog.trace.api.KnownTags +import datadog.trace.api.config.GeneralConfig import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.bootstrap.instrumentation.api.URIUtils import datadog.trace.core.DDSpan @@ -55,6 +60,9 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { protected void configurePreAgent() { super.configurePreAgent() injectSysConfig(TRACE_ANNOTATION_ASYNC, "true") + if (this instanceof TestingOtelHttpSemantics) { + injectSysConfig(GeneralConfig.TRACE_OTEL_SEMANTICS_ENABLED, "true") + } } def "Basic GET test #testName"() { @@ -84,8 +92,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 "$Tags.HTTP_USER_AGENT" String @@ -135,6 +142,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "annotation API without parameters" | "/foo" | "/foo" | "getFooModel" | new FooModel(0L, "DEFAULT").toString() "annotation API with one parameter" | "/foo/1" | "/foo/{id}" | "getFooModel" | new FooModel(1L, "pass").toString() + "annotation API with query" | "/foo/1?view=full" | "/foo/{id}" | "getFooModel" | new FooModel(1L, "pass").toString() "annotation API with two parameters" | "/foo/2/world" | "/foo/{id}/{name}" | "getFooModel" | new FooModel(2L, "world").toString() "annotation API delayed response" | "/foo-delayed" | "/foo-delayed" | "getFooDelayed" | new FooModel(3L, "delayed").toString() } @@ -166,8 +174,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 "$Tags.HTTP_USER_AGENT" String @@ -300,7 +307,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { } trace(2) { span { - resourceName "404" + resourceName Config.get().isTraceOtelSemanticsEnabled() ? "GET" : "404" operationName "netty.request" spanType DDSpanTypes.HTTP_SERVER childOf(traceParent) @@ -309,8 +316,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 404 "$Tags.HTTP_USER_AGENT" String @@ -366,8 +372,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "POST" "$Tags.HTTP_STATUS" 202 "$Tags.HTTP_USER_AGENT" String @@ -433,10 +438,12 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 500 + if (Config.get().isTraceOtelSemanticsEnabled()) { + "$DDTags.ERROR_TYPE" "500" + } "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" "$Tags.NETWORK_CLIENT_IP" "127.0.0.1" @@ -524,8 +531,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 307 "$Tags.HTTP_USER_AGENT" String @@ -570,8 +576,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" finalUrl - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, finalUrl) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 "$Tags.HTTP_USER_AGENT" String @@ -630,8 +635,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 200 "$Tags.HTTP_USER_AGENT" String @@ -692,7 +696,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { def traceParent sortSpansByStart() trace(2) { - clientSpan(it, null, "http.request", "spring-webflux-client", "GET", URI.create(url), null, false, null, false, + clientSpan(it, null, "http.request", "spring-webflux-client", "GET", URI.create(url), null, false, false, ["message": "The subscription was cancelled", "event": "cancelled"]) traceParent = clientSpan(it, span(0), "netty.client.request", "netty-client", "GET", URI.create(url), null) } @@ -707,8 +711,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_USER_AGENT" String "$Tags.HTTP_CLIENT_IP" "127.0.0.1" @@ -762,8 +765,7 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" "127.0.0.1" "$Tags.PEER_PORT" Integer - "$Tags.HTTP_URL" url - "$Tags.HTTP_HOSTNAME" "localhost" + this.serverUrlTags(it, url) "$Tags.HTTP_METHOD" "GET" "$Tags.HTTP_STATUS" 101 "$Tags.HTTP_USER_AGENT" String @@ -802,6 +804,21 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { } + void serverUrlTags(TagsAssert tags, String url) { + def uri = URI.create(url) + if (Config.get().isTraceOtelSemanticsEnabled()) { + tags.tag(KnownTags.SERVER_ADDRESS_NAME, uri.host) + tags.tag(KnownTags.SERVER_PORT_NAME, uri.port) + tags.tag(KnownTags.URL_PATH_NAME, uri.path) + tags.tag(KnownTags.URL_SCHEME_NAME, uri.scheme) + tags.tag(KnownTags.URL_QUERY_NAME, uri.rawQuery) + } else { + tags.tag(Tags.HTTP_URL, url) + tags.tag(Tags.HTTP_HOSTNAME, uri.host) + tags.tag(DDTags.HTTP_QUERY, uri.query) + } + } + def clientSpan( TraceAssert trace, Object parentSpan, @@ -811,12 +828,13 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { URI uri, Integer status = 200, boolean error = false, - Throwable exception = null, - boolean tagQueryString = false, + boolean tagQueryString = Config.get().isHttpClientTagQueryString(), Map extraTags = null) { def ret - def expectedQuery = tagQueryString ? uri.query : null + def otelSemantics = Config.get().isTraceOtelSemanticsEnabled() + def statusError = otelSemantics && status != null && status >= 400 + def expectedQuery = otelSemantics || tagQueryString ? uri.query : null def expectedUrl = URIUtils.buildURL(uri.scheme, uri.host, uri.port, uri.path) if (expectedQuery != null && !expectedQuery.empty) { expectedUrl = "$expectedUrl?$expectedQuery" @@ -829,26 +847,36 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { childOf((DDSpan) parentSpan) } operationName operation + if (otelSemantics) { + resourceName method + } spanType DDSpanTypes.HTTP_CLIENT - errored error + errored error || statusError measured true tags { "$Tags.COMPONENT" component "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT - "$Tags.PEER_HOSTNAME" uri.host - "$Tags.PEER_HOST_IPV4" { it == null || it == "127.0.0.1" } // Optional - "$Tags.PEER_PORT" { it == null || it == uri.port } + if (!otelSemantics || component == "netty-client") { + // Netty also decorates the underlying socket connection. + "$Tags.PEER_HOSTNAME" uri.host + "$Tags.PEER_HOST_IPV4" { it == null || it == "127.0.0.1" } // Optional + "$Tags.PEER_PORT" { it == null || it == uri.port } + } + if (otelSemantics) { + "$KnownTags.SERVER_ADDRESS_NAME" uri.host + "$KnownTags.SERVER_PORT_NAME" uri.port + } "$Tags.HTTP_URL" expectedUrl "$Tags.HTTP_METHOD" method if (status) { "$Tags.HTTP_STATUS" status } - if (tagQueryString) { + if (!otelSemantics && tagQueryString) { "$DDTags.HTTP_QUERY" expectedQuery "$DDTags.HTTP_FRAGMENT" { it == null || it == uri.fragment } // Optional } - if (exception) { - errorTags(exception.class, exception.message) + if (statusError) { + "$DDTags.ERROR_TYPE" status.toString() } peerServiceFrom(Tags.PEER_HOSTNAME) defaultTags() @@ -861,6 +889,9 @@ class SpringWebfluxHttp11Test extends InstrumentationSpecification { } } +class SpringWebfluxOtelSemanticsForkedTest extends SpringWebfluxHttp11Test implements TestingOtelHttpSemantics { +} + class SpringWebfluxHttp2UpgradeTest extends SpringWebfluxHttp11Test { @Override protected HttpClient buildClient() { diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy index 885831440f5..f9077e78710 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy @@ -5,6 +5,8 @@ import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.base.WebsocketServer +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics +import datadog.trace.api.Config import datadog.trace.api.ConfigDefaults import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags @@ -244,6 +246,9 @@ class SpringBootBasedTest extends HttpServerTest @Override String expectedResourceName(ServerEndpoint endpoint, String method, URI address) { + if (Config.get().isTraceOtelSemanticsEnabled()) { + return super.expectedResourceName(endpoint, method, address) + } if (endpoint.status == 404 && endpoint.path == "/not-found") { return "404" } else if (endpoint.hasPathParam) { @@ -512,6 +517,9 @@ class SpringBootRumInjectionForkedTest extends SpringBootBasedTest { } } +class SpringBootOtelSemanticsForkedTest extends SpringBootBasedTest implements TestingOtelHttpSemantics { +} + /** * Runs the full suite with the opt-in legacy route-naming filter enabled, to verify that path still * produces the same traces (including APMS-8174). The default path (getHandler + ControllerAdvice) diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/test/groovy/TomcatServletTest.groovy b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/test/groovy/TomcatServletTest.groovy index 3e507f1ae71..48fd61bbf32 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/test/groovy/TomcatServletTest.groovy +++ b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/test/groovy/TomcatServletTest.groovy @@ -13,6 +13,7 @@ import com.google.common.io.Files import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.api.DDTags import datadog.trace.instrumentation.servlet3.TestServlet3 import org.apache.catalina.Context @@ -344,3 +345,6 @@ class TomcatServletV1ForkedTest extends TomcatServletTest implements TestingGene true } } + +class TomcatServletOtelSemanticsForkedTest extends TomcatServletTest implements TestingGenericHttpNamingConventions.ServerV0, TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowTest.groovy b/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowTest.groovy index c369cce9e43..1c76ffa68bc 100644 --- a/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowTest.groovy +++ b/dd-java-agent/instrumentation/undertow/undertow-2.2/src/test/groovy/UndertowTest.groovy @@ -2,6 +2,7 @@ import datadog.appsec.api.blocking.Blocking import datadog.appsec.api.blocking.BlockingException import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import io.undertow.Handlers import io.undertow.Undertow import io.undertow.UndertowOptions @@ -209,3 +210,6 @@ class UndertowTest extends HttpServerTest { } } } + +class UndertowOtelSemanticsForkedTest extends UndertowTest implements TestingOtelHttpSemantics { +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 62455354611..51d7a9e5c6f 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -12,7 +12,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.naming.TestingNettyHttpNamingConventions +import datadog.trace.agent.test.naming.TestingOtelHttpSemantics import datadog.trace.api.Config +import datadog.trace.api.DDTags import okhttp3.MediaType import okhttp3.MultipartBody import okhttp3.RequestBody @@ -217,6 +220,8 @@ class VertxHttpServerForkedTest extends HttpServerTest { "$Tags.HTTP_STATUS" Integer if (endpoint == EXCEPTION && this.testExceptionTag()) { errorTags(RuntimeException, EXCEPTION.body) + } else if (Config.get().isTraceOtelSemanticsEnabled() && endpoint == ERROR) { + "$DDTags.ERROR_TYPE" endpoint.status.toString() } defaultTags() } @@ -230,3 +235,6 @@ class VertxHttpServerWorkerForkedTest extends VertxHttpServerForkedTest { return new VertxServer(verticle(), routerBasePath(), true) } } + +class VertxHttpServerOtelSemanticsForkedTest extends VertxHttpServerForkedTest implements TestingNettyHttpNamingConventions.ServerV0, TestingOtelHttpSemantics { +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index bf5bab30d05..e5eab1da02b 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -16,6 +16,7 @@ public final class ConfigDefaults { static final BitSet DEFAULT_HTTP_SERVER_ERROR_STATUSES; static final BitSet DEFAULT_HTTP_CLIENT_ERROR_STATUSES; + static final BitSet DEFAULT_OTEL_HTTP_CLIENT_ERROR_STATUSES; static final BitSet DEFAULT_GRPC_SERVER_ERROR_STATUSES; static final BitSet DEFAULT_GRPC_CLIENT_ERROR_STATUSES; @@ -26,6 +27,8 @@ public final class ConfigDefaults { DEFAULT_HTTP_SERVER_ERROR_STATUSES.set(500, 600); DEFAULT_HTTP_CLIENT_ERROR_STATUSES = new BitSet(); DEFAULT_HTTP_CLIENT_ERROR_STATUSES.set(400, 500); + DEFAULT_OTEL_HTTP_CLIENT_ERROR_STATUSES = new BitSet(); + DEFAULT_OTEL_HTTP_CLIENT_ERROR_STATUSES.set(400, 600); DEFAULT_GRPC_SERVER_ERROR_STATUSES = new BitSet(); DEFAULT_GRPC_SERVER_ERROR_STATUSES.set(2, 17); DEFAULT_GRPC_CLIENT_ERROR_STATUSES = new BitSet(); @@ -126,6 +129,7 @@ public final class ConfigDefaults { public static final boolean DEFAULT_METRICS_OTEL_EXPERIMENTAL_ENABLED = true; + static final boolean DEFAULT_TRACE_OTEL_SEMANTICS_ENABLED = false; public static final int DEFAULT_OTLP_TRACES_TIMEOUT = 10_000; // ms static final String DEFAULT_OTLP_HTTP_LOGS_ENDPOINT = "v1/logs"; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessor.java b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessor.java index c2e0dd72761..56057e996e8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessor.java @@ -4,6 +4,7 @@ import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_ROUTE; import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_URL; +import datadog.trace.api.KnownTags; import datadog.trace.api.TagMap; import datadog.trace.api.endpoint.EndpointResolver; import datadog.trace.api.internal.VisibleForTesting; @@ -68,6 +69,9 @@ public void processTags( try { String httpRoute = unsafeTags.getString(HTTP_ROUTE); String httpUrl = unsafeTags.getString(HTTP_URL); + if (httpUrl == null) { + httpUrl = unsafeTags.getString(KnownTags.URL_PATH_NAME); + } endpointResolver.resolveEndpoint(unsafeTags, httpRoute, httpUrl); } catch (Throwable t) { log.debug("Error processing HTTP endpoint for span {}", spanContext.getSpanId(), t); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/QueryObfuscator.java b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/QueryObfuscator.java index 6ae813a772f..72ee76e0752 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/QueryObfuscator.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/QueryObfuscator.java @@ -3,7 +3,10 @@ import com.google.re2j.Matcher; import com.google.re2j.Pattern; import com.google.re2j.PatternSyntaxException; +import datadog.trace.api.Config; +import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTags; import datadog.trace.api.TagMap; import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks; @@ -58,12 +61,18 @@ public final class QueryObfuscator extends TagsPostProcessor { private final Pattern pattern; private final String replacement; private final boolean failClosed; + private final boolean otelSemanticsEnabled; /** * If regex is null - then used default regex pattern If regex is empty string - then disable * regex. If the regex does not compile, the query string is not reported. */ public QueryObfuscator(String regex) { + this(regex, Config.get().isTraceOtelSemanticsEnabled()); + } + + QueryObfuscator(String regex, boolean otelSemanticsEnabled) { + this.otelSemanticsEnabled = otelSemanticsEnabled; // empty string -> disabled query obfuscation if ("".equals(regex)) { this.pattern = null; @@ -116,10 +125,11 @@ private String obfuscate(String query) { @Override public void processTags( TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) { - Object query = unsafeTags.getObject(DDTags.HTTP_QUERY); + String queryTag = otelSemanticsEnabled ? KnownTags.URL_QUERY_NAME : DDTags.HTTP_QUERY; + Object query = unsafeTags.getObject(queryTag); if (query instanceof CharSequence) { if (failClosed) { - unsafeTags.remove(DDTags.HTTP_QUERY); + unsafeTags.remove(queryTag); return; } try { @@ -127,16 +137,38 @@ public void processTags( } catch (Exception e) { // never report a query that could not be obfuscated, and never log it log.debug("Could not obfuscate query string, dropping it", e); - unsafeTags.remove(DDTags.HTTP_QUERY); + unsafeTags.remove(queryTag); return; } - unsafeTags.put(DDTags.HTTP_QUERY, query); + unsafeTags.put(queryTag, query); Object url = unsafeTags.getObject(Tags.HTTP_URL); if (url instanceof CharSequence) { - unsafeTags.put(Tags.HTTP_URL, url + "?" + query); + unsafeTags.put(Tags.HTTP_URL, appendQuery(url.toString(), query.toString())); + } + + // HTTP clients carry the query separately only so this processor can scrub it before + // rebuilding url.full. url.query is a server attribute in the OTel HTTP conventions. + if (otelSemanticsEnabled + && spanContext != null + && spanContext.getSpanType() != null + && DDSpanTypes.HTTP_CLIENT.contentEquals(spanContext.getSpanType())) { + unsafeTags.remove(queryTag); } } } + + private static String appendQuery(final String url, final String query) { + final int fragment = url.indexOf('#'); + if (fragment < 0) { + return url + "?" + query; + } + return new StringBuilder(url.length() + query.length() + 1) + .append(url, 0, fragment) + .append('?') + .append(query) + .append(url, fragment, url.length()) + .toString(); + } } diff --git a/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollectorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollectorTest.java index 811e3cdf32b..be50fc65a91 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollectorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollectorTest.java @@ -95,6 +95,18 @@ void tagsAreEmittedUnderTheirOpenTelemetryNameWhenOtelSemanticsEnabled() throws assertFalse( attrKeys.contains("http.method"), "renamed tag must not also appear under its Datadog name; got " + attrKeys); + assertTrue( + attrKeys.contains("network.peer.address"), + "renamed tag must use its OpenTelemetry name; got " + attrKeys); + assertFalse( + attrKeys.contains("network.client.ip"), + "renamed tag must not also appear under its Datadog name; got " + attrKeys); + assertTrue( + attrKeys.contains("server.address"), + "canonical OpenTelemetry tag must pass through unchanged; got " + attrKeys); + assertTrue( + attrKeys.contains("server.port"), + "canonical OpenTelemetry tag must pass through unchanged; got " + attrKeys); assertTrue( attrKeys.contains("custom.unregistered"), "a tag the registry does not name passes through unchanged; got " + attrKeys); @@ -113,17 +125,33 @@ void tagsAreEmittedUnderTheirDatadogNameWhenOtelSemanticsDisabled() throws IOExc attrKeys.contains("http.request.method"), "tag must not appear under its OpenTelemetry name when OTel semantics are disabled; got " + attrKeys); + assertTrue( + attrKeys.contains("network.client.ip"), + "tag must use its Datadog name when OTel semantics are disabled; got " + attrKeys); + assertFalse( + attrKeys.contains("network.peer.address"), + "tag must not appear under its OpenTelemetry name when OTel semantics are disabled; got " + + attrKeys); + assertTrue( + attrKeys.contains("server.address"), + "canonical tag must pass through unchanged; got " + attrKeys); + assertTrue( + attrKeys.contains("server.port"), + "canonical tag must pass through unchanged; got " + attrKeys); assertTrue( attrKeys.contains("custom.unregistered"), "a tag the registry does not name passes through unchanged; got " + attrKeys); } + // Restore the commented out pairings when an OTel name can also be qualified by a direction. + // Currently, OTel tags server.address and server.port represent the "peer" on client spans + // and the "self" on server spans @TableTest({ "scenario | kind | tag | otelName ", - "client hostname | client | peer.hostname | server.address ", - "server hostname | server | http.hostname | server.address ", + // "client hostname | client | peer.hostname | server.address ", + // "server hostname | server | http.hostname | server.address ", "server client ip | server | network.client.ip | network.peer.address", - "client peer port | client | peer.port | server.port ", + // "client peer port | client | peer.port | server.port ", "server peer port | server | peer.port | client.port " }) void aDirectionScopedTagIsEmittedUnderTheNameForTheSpansKind( @@ -159,6 +187,9 @@ private static Set collectTagsAttributeKeys(boolean otelSemanticsEnabled AgentSpan agentSpan = TRACER.startSpan("test", "op.tagged"); agentSpan.setResourceName("GET /api"); agentSpan.setTag("http.method", "GET"); + agentSpan.setTag("network.client.ip", "192.0.2.10"); + agentSpan.setTag("server.address", "example.com"); + agentSpan.setTag("server.port", 443); agentSpan.setTag("custom.unregistered", "value"); agentSpan.setSamplingPriority(PrioritySampling.USER_KEEP, SamplingMechanism.DEFAULT); agentSpan.finish(); diff --git a/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessorTest.java index e12c1a0dfa6..e6369b97c60 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessorTest.java @@ -7,6 +7,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import datadog.trace.api.KnownTags; import datadog.trace.api.TagMap; import datadog.trace.api.endpoint.EndpointResolver; import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks; @@ -57,6 +58,21 @@ void shouldComputeAndTagHttpEndpointFromUrlWhenRouteIsInvalidWithoutTouchingReso assertEquals("/users/{param:int}/orders/{param:int}", tags.get(Tags.HTTP_ENDPOINT)); } + @Test + void shouldComputeAndTagHttpEndpointFromOtelUrlPath() { + EndpointResolver endpointResolver = new EndpointResolver(true, false); + HttpEndpointPostProcessor processor = new HttpEndpointPostProcessor(endpointResolver); + Map tagInput = new HashMap<>(); + tagInput.put(KnownTags.HTTP_METHOD_OTEL_NAME, "GET"); + tagInput.put(KnownTags.URL_PATH_NAME, "/users/123/orders/456"); + TagMap tags = TagMap.fromMap(tagInput); + + processor.processTags(tags, mockContext, mockSpanLinks); + + verify(mockContext, never()).setResourceName(any(CharSequence.class), anyByte()); + assertEquals("/users/{param:int}/orders/{param:int}", tags.get(Tags.HTTP_ENDPOINT)); + } + @Test void shouldSkipNonHttpSpans() { EndpointResolver endpointResolver = new EndpointResolver(true, false); diff --git a/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/QueryObfuscatorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/QueryObfuscatorTest.java index 4e2d837b56b..7508ef465ca 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/QueryObfuscatorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/tagprocessor/QueryObfuscatorTest.java @@ -3,11 +3,16 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import com.google.re2j.Pattern; import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTags; import datadog.trace.api.TagMap; import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.core.DDSpanContext; import datadog.trace.test.util.DDJavaSpecification; import java.util.LinkedHashMap; import java.util.Map; @@ -65,9 +70,13 @@ void tagsProcessingWithCustomRegexpForEmail(String query, String expectedQuery) } private static TagMap process(QueryObfuscator obfuscator, Object query) { + return process(obfuscator, query, DDTags.HTTP_QUERY); + } + + private static TagMap process(QueryObfuscator obfuscator, Object query, String queryTag) { Map tags = new LinkedHashMap<>(); tags.put(Tags.HTTP_URL, "http://site.com/index"); - tags.put(DDTags.HTTP_QUERY, query); + tags.put(queryTag, query); TagMap unsafeTags = TagMap.fromMap(tags); obfuscator.processTags(unsafeTags, null, link -> {}); @@ -130,16 +139,26 @@ void userCopyOfDefaultPatternAlsoRedactsDelimiter() { assertEquals("a=1&b=2", unsafeTags.getObject(DDTags.HTTP_QUERY)); } - @Test - void invalidRegexDropsQueryString() { - TagMap unsafeTags = process(new QueryObfuscator("(?<=a)b"), "pwd=secret"); + @TableTest({ + "Scenario | OTel semantics", + "Datadog | false ", + "OpenTelemetry | true " + }) + void invalidRegexDropsQueryString(boolean otelSemantics) { + String queryTag = otelSemantics ? KnownTags.URL_QUERY_NAME : DDTags.HTTP_QUERY; + TagMap unsafeTags = + process(new QueryObfuscator("(?<=a)b", otelSemantics), "pwd=secret", queryTag); - assertFalse(unsafeTags.containsKey(DDTags.HTTP_QUERY)); + assertFalse(unsafeTags.containsKey(queryTag)); assertEquals("http://site.com/index", unsafeTags.getObject(Tags.HTTP_URL)); } - @Test - void obfuscationFailureDropsQueryString() { + @TableTest({ + "Scenario | OTel semantics", + "Datadog | false ", + "OpenTelemetry | true " + }) + void obfuscationFailureDropsQueryString(boolean otelSemantics) { CharSequence failing = new CharSequence() { @Override @@ -163,9 +182,12 @@ public String toString() { } }; - TagMap unsafeTags = assertDoesNotThrow(() -> process(new QueryObfuscator(null), failing)); + String queryTag = otelSemantics ? KnownTags.URL_QUERY_NAME : DDTags.HTTP_QUERY; + TagMap unsafeTags = + assertDoesNotThrow( + () -> process(new QueryObfuscator(null, otelSemantics), failing, queryTag)); - assertFalse(unsafeTags.containsKey(DDTags.HTTP_QUERY)); + assertFalse(unsafeTags.containsKey(queryTag)); assertEquals("http://site.com/index", unsafeTags.getObject(Tags.HTTP_URL)); } @@ -178,4 +200,28 @@ void emptyRegexKeepsObfuscationDisabled() { assertEquals(query, unsafeTags.getObject(DDTags.HTTP_QUERY)); assertEquals("http://site.com/index?" + query, unsafeTags.getObject(Tags.HTTP_URL)); } + + @TableTest({ + "spanType | retainQuery", + "http | false ", + "web | true " + }) + void otelClientQueryIsOnlyUsedToBuildUrlFull(String spanType, boolean retainQuery) { + QueryObfuscator obfuscator = new QueryObfuscator(null, true); + DDSpanContext spanContext = mock(DDSpanContext.class); + when(spanContext.getSpanType()).thenReturn(spanType); + + TagMap tags = TagMap.create(); + tags.put(Tags.HTTP_URL, "http://site.com/index#fragment"); + tags.put(KnownTags.URL_QUERY_NAME, "token=secret"); + + obfuscator.processTags(tags, spanContext, link -> {}); + + assertEquals("http://site.com/index?#fragment", tags.get(Tags.HTTP_URL)); + if (retainQuery) { + assertEquals("", tags.get(KnownTags.URL_QUERY_NAME)); + return; + } + assertNull(tags.get(KnownTags.URL_QUERY_NAME)); + } } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 3471750c347..c2846542fef 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -129,6 +129,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_METRICS_OTEL_EXPERIMENTAL_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_METRICS_OTEL_INTERVAL; import static datadog.trace.api.ConfigDefaults.DEFAULT_METRICS_OTEL_TIMEOUT; +import static datadog.trace.api.ConfigDefaults.DEFAULT_OTEL_HTTP_CLIENT_ERROR_STATUSES; import static datadog.trace.api.ConfigDefaults.DEFAULT_OTLP_GRPC_PORT; import static datadog.trace.api.ConfigDefaults.DEFAULT_OTLP_HTTP_LOGS_ENDPOINT; import static datadog.trace.api.ConfigDefaults.DEFAULT_OTLP_HTTP_METRICS_ENDPOINT; @@ -188,6 +189,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_LONG_RUNNING_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL; +import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_OTEL_SEMANTICS_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_POST_PROCESSING_TIMEOUT; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_PROPAGATION_EXTRACT_FIRST; @@ -1560,11 +1562,33 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins integrationSynapseLegacyOperationName = configProvider.getBoolean(INTEGRATION_SYNAPSE_LEGACY_OPERATION_NAME, false); - traceOtelExporter = configProvider.getString(TRACE_OTEL_EXPORTER); + traceOtelSemanticsEnabled = + configProvider.getBoolean( + TRACE_OTEL_SEMANTICS_ENABLED, DEFAULT_TRACE_OTEL_SEMANTICS_ENABLED); + String configuredTraceOtelExporter = configProvider.getString(TRACE_OTEL_EXPORTER); + if (traceOtelSemanticsEnabled) { + if (configuredTraceOtelExporter != null + && !"otlp".equalsIgnoreCase(configuredTraceOtelExporter)) { + logOverriddenSettingWarning(TRACE_OTEL_EXPORTER, TRACE_OTEL_SEMANTICS_ENABLED, "otlp"); + } + traceOtelExporter = "otlp"; + reportOtelSemanticsOverride(TRACE_OTEL_EXPORTER, traceOtelExporter); + } else { + traceOtelExporter = configuredTraceOtelExporter; + } boolean otlpTracesExporter = isTraceOtlpExporterEnabled(); - writerType = + String configuredWriterType = configProvider.getString( WRITER_TYPE, otlpTracesExporter ? OTLP_WRITER_TYPE : DEFAULT_AGENT_WRITER_TYPE); + if (traceOtelSemanticsEnabled) { + if (!OTLP_WRITER_TYPE.equals(configuredWriterType)) { + logOverriddenSettingWarning(WRITER_TYPE, TRACE_OTEL_SEMANTICS_ENABLED, OTLP_WRITER_TYPE); + } + writerType = OTLP_WRITER_TYPE; + reportOtelSemanticsOverride(WRITER_TYPE, writerType); + } else { + writerType = configuredWriterType; + } boolean isDatadogTraceWriter = !otlpTracesExporter; injectBaggageAsTagsEnabled = configProvider.getBoolean(WRITER_BAGGAGE_INJECT, isDatadogTraceWriter); @@ -1777,14 +1801,32 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins sqsInjectDatadogAttributeEnabled = isInjectDatadogAttributeEnabled(DEFAULT_INJECT_DATADOG_ATTRIBUTE, "sqs"); - spanAttributeSchemaVersion = schemaVersionFromConfig(); + int configuredSpanAttributeSchemaVersion = schemaVersionFromConfig(); + if (traceOtelSemanticsEnabled) { + if (configuredSpanAttributeSchemaVersion != SpanNaming.SCHEMA_MIN_VERSION) { + logOtelSemanticsOverride(TRACE_SPAN_ATTRIBUTE_SCHEMA, "v" + SpanNaming.SCHEMA_MIN_VERSION); + } + spanAttributeSchemaVersion = SpanNaming.SCHEMA_MIN_VERSION; + reportOtelSemanticsOverride(TRACE_SPAN_ATTRIBUTE_SCHEMA, "v" + spanAttributeSchemaVersion); + } else { + spanAttributeSchemaVersion = configuredSpanAttributeSchemaVersion; + } peerHostNameEnabled = configProvider.getBoolean(TRACE_PEER_HOSTNAME_ENABLED, true); // following two only used in v0. // in v1+ defaults are always calculated regardless this feature flag - peerServiceDefaultsEnabled = + boolean configuredPeerServiceDefaultsEnabled = configProvider.getBoolean(TRACE_PEER_SERVICE_DEFAULTS_ENABLED, false); + if (traceOtelSemanticsEnabled) { + if (configuredPeerServiceDefaultsEnabled) { + logOtelSemanticsOverride(TRACE_PEER_SERVICE_DEFAULTS_ENABLED, false); + } + peerServiceDefaultsEnabled = false; + reportOtelSemanticsOverride(TRACE_PEER_SERVICE_DEFAULTS_ENABLED, false); + } else { + peerServiceDefaultsEnabled = configuredPeerServiceDefaultsEnabled; + } peerServiceComponentOverrides = configProvider.getMergedMap(TRACE_PEER_SERVICE_COMPONENT_OVERRIDES); // feature flag to remove fake services in v0 @@ -1816,7 +1858,9 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins httpClientErrorStatuses = configProvider.getIntegerRange( TRACE_HTTP_CLIENT_ERROR_STATUSES, - DEFAULT_HTTP_CLIENT_ERROR_STATUSES, + traceOtelSemanticsEnabled + ? DEFAULT_OTEL_HTTP_CLIENT_ERROR_STATUSES + : DEFAULT_HTTP_CLIENT_ERROR_STATUSES, HTTP_CLIENT_ERROR_STATUSES); httpServerTagQueryString = @@ -2213,7 +2257,6 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins OtlpConfig.Temporality.class, OtlpConfig.Temporality.DELTA); - traceOtelSemanticsEnabled = configProvider.getBoolean(TRACE_OTEL_SEMANTICS_ENABLED, false); // Tri-state default: when unset, SDK-computed OTLP span metrics are emitted iff OTLP trace // export and OTLP metrics export are both enabled. otelTracesSpanMetricsEnabled = @@ -6144,6 +6187,19 @@ private void logOverriddenSettingWarning(String setting, String overridingSettin value); } + private static void reportOtelSemanticsOverride(String setting, Object value) { + ConfigCollector.get().put(setting, value, ConfigOrigin.CALCULATED, Integer.MAX_VALUE); + } + + private static void logOtelSemanticsOverride(String setting, Object value) { + log.warn( + SEND_TELEMETRY, + "Enabling {} overrode {} to {}", + propertyNameToEnvironmentVariableName(TRACE_OTEL_SEMANTICS_ENABLED), + propertyNameToEnvironmentVariableName(setting), + value); + } + private void logOverriddenDeprecatedSettingWarning( String setting, String overridingSetting, Object value) { log.warn( diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java b/internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java index 3aa95465e68..6eed4b07b77 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java @@ -18,10 +18,12 @@ import datadog.context.ContextKey; import datadog.context.ImplicitContextKeyed; import datadog.trace.api.Config; +import datadog.trace.api.KnownTags; import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.OtelHttpSemantics; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -42,6 +44,7 @@ public class InferredProxySpan implements ImplicitContextKeyed { static final String PROXY_REGION = "x-dd-proxy-region"; static final Map SUPPORTED_PROXIES; static final String INSTRUMENTATION_NAME = "inferred_proxy"; + private static final boolean OTEL_SEMANTICS = Config.get().isTraceOtelSemanticsEnabled(); static { SUPPORTED_PROXIES = new HashMap<>(); @@ -95,7 +98,6 @@ public AgentSpanContext start(AgentSpanContext extracted) { String path = header(PROXY_PATH); String resourcePath = header(PROXY_RESOURCE_PATH); String domainName = header(PROXY_DOMAIN_NAME); - AgentSpan span = AgentTracer.get().startSpan(INSTRUMENTATION_NAME, proxy, extracted, startTime); // Service: value of x-dd-proxy-domain-name or global config if not found @@ -112,16 +114,24 @@ public AgentSpanContext start(AgentSpanContext extracted) { // SpanType: web span.setTag(SPAN_TYPE, "web"); - // Http.method - value of x-dd-proxy-httpmethod - span.setTag(HTTP_METHOD, httpMethod); - - // Http.url - https:// + x-dd-proxy-domain-name + x-dd-proxy-path - span.setTag( - HTTP_URL, - domainName != null && !domainName.isEmpty() ? "https://" + domainName + path : path); + if (OTEL_SEMANTICS) { + OtelHttpSemantics.setRequestMethod(span, httpMethod); + span.setTag(KnownTags.URL_PATH_NAME, path); + if (domainName != null && !domainName.isEmpty()) { + span.setTag(KnownTags.URL_SCHEME_NAME, "https"); + span.setTag(KnownTags.SERVER_ADDRESS_NAME, domainName); + } + } else { + span.setTag(HTTP_METHOD, httpMethod); + span.setTag( + HTTP_URL, + domainName != null && !domainName.isEmpty() ? "https://" + domainName + path : path); + } - // Http.route - value of x-dd-proxy-resource-path (or x-dd-proxy-path as fallback) - span.setTag(HTTP_ROUTE, resourcePath != null && !resourcePath.isEmpty() ? resourcePath : path); + // A concrete path is not a route under the OTel conventions. Keep the legacy fallback only + // while Datadog semantics are active. + String route = resourcePath != null && !resourcePath.isEmpty() ? resourcePath : null; + span.setTag(HTTP_ROUTE, OTEL_SEMANTICS ? route : route != null ? route : path); // "stage" - value of x-dd-proxy-stage span.setTag("stage", header(STAGE)); @@ -153,12 +163,19 @@ public AgentSpanContext start(AgentSpanContext extracted) { // _dd.inferred_span = 1 (indicates that this is an inferred span) span.setTag("_dd.inferred_span", 1); - // Resource Name: when route available, else - // Prefer x-dd-proxy-resource-path (route) over x-dd-proxy-path (path) + // Resource Name: OTel uses or just ; Datadog keeps the historical + // fallback to when no route is available. // Use MANUAL_INSTRUMENTATION priority to prevent TagInterceptor from overriding - String routeOrPath = resourcePath != null && !resourcePath.isEmpty() ? resourcePath : path; - String resourceName = - httpMethod != null && routeOrPath != null ? httpMethod + " " + routeOrPath : null; + String spanNameMethod = + OTEL_SEMANTICS ? OtelHttpSemantics.spanNameMethod(httpMethod) : httpMethod; + String resourceName; + if (OTEL_SEMANTICS) { + resourceName = route != null ? spanNameMethod + " " + route : spanNameMethod; + } else { + String routeOrPath = route != null ? route : path; + resourceName = + spanNameMethod != null && routeOrPath != null ? spanNameMethod + " " + routeOrPath : null; + } if (resourceName != null) { span.setResourceName(resourceName, MANUAL_INSTRUMENTATION); } @@ -290,6 +307,9 @@ private void copyTagsFromServiceEntry(AgentSpan serviceEntrySpan) { this.span.setHttpStatusCode(statusCode); boolean isError = Config.get().getHttpServerErrorStatuses().get(statusCode); this.span.setError(isError, HTTP_SERVER_DECORATOR); + if (OTEL_SEMANTICS && isError) { + OtelHttpSemantics.setErrorType(this.span, statusCode); + } } Object userAgent = serviceEntrySpan.getTag(HTTP_USER_AGENT); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/OtelHttpSemantics.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/OtelHttpSemantics.java new file mode 100644 index 00000000000..5b23167fd88 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/OtelHttpSemantics.java @@ -0,0 +1,177 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTags; +import datadog.trace.api.cache.RadixTreeCache; +import java.net.URI; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** Shared helpers for emitting OpenTelemetry HTTP semantic conventions. */ +public final class OtelHttpSemantics { + public static final String OTHER_METHOD = "_OTHER"; + + // RFC 9110, PATCH (RFC 5789), and QUERY (HTTPbis draft). Method names are case-sensitive. + private static final Set KNOWN_METHODS = + Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + "CONNECT", "DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT", "QUERY", + "TRACE"))); + + private OtelHttpSemantics() {} + + public static void setRequestMethod(final AgentSpan span, final String method) { + if (method != null && !KNOWN_METHODS.contains(method)) { + span.setTag(KnownTags.HTTP_METHOD_OTEL_NAME, OTHER_METHOD); + span.setTag(KnownTags.HTTP_REQUEST_METHOD_ORIGINAL_NAME, method); + } else { + span.setTag(KnownTags.HTTP_METHOD_OTEL_NAME, method); + } + } + + public static void setMethodResourceName(final AgentSpan span, final CharSequence method) { + span.setResourceName(spanNameMethod(method), ResourceNamePriorities.HTTP_PATH_NORMALIZER); + } + + public static void setClientUrl(final AgentSpan span, final URI url) { + span.setTag(KnownTags.HTTP_URL_OTEL_NAME, withoutQuery(redactedUrl(url))); + // QueryObfuscator scrubs this before rebuilding url.full and removes it from client spans. + final String query = url.getRawQuery(); + if (query != null) { + span.setTag(KnownTags.URL_QUERY_NAME, query); + } + } + + public static String spanNameMethod(final CharSequence method) { + final String methodName = method == null ? null : method.toString(); + return KNOWN_METHODS.contains(methodName) ? methodName : "HTTP"; + } + + public static String redactedUrl(final URI url) { + final String full = url.toString(); + // URI does not parse user-info for some hosts, including underscores and Unicode names. + final String authority = url.getRawAuthority(); + if (authority == null) { + return full; + } + final int userInfoEnd = authority.lastIndexOf('@'); + if (userInfoEnd <= 0) { + return full; + } + final int colon = authority.indexOf(':'); + final String redacted = colon >= 0 && colon < userInfoEnd ? "REDACTED:REDACTED" : "REDACTED"; + final int userInfoStart = full.indexOf("//") + 2; + return full.substring(0, userInfoStart) + + redacted + + full.substring(userInfoStart + userInfoEnd); + } + + public static String withoutQuery(final String url) { + final int fragment = url.indexOf('#'); + final int query = url.indexOf('?'); + if (query < 0 || (fragment >= 0 && fragment < query)) { + return url; + } + if (fragment < 0) { + return url.substring(0, query); + } + return new StringBuilder(url.length() - (fragment - query)) + .append(url, 0, query) + .append(url, fragment, url.length()) + .toString(); + } + + public static void setErrorType(final AgentSpan span, final int status) { + if (span.getTag(DDTags.ERROR_TYPE) == null) { + span.setTag(DDTags.ERROR_TYPE, RadixTreeCache.HTTP_STATUSES.get(status).toString()); + } + } + + public static int serverPort(final URI url) { + return serverPort(url.getScheme(), url.getPort()); + } + + public static int serverPort(final String scheme, final int port) { + if (port > 0) { + return port; + } + if ("https".equalsIgnoreCase(scheme)) { + return 443; + } + if ("http".equalsIgnoreCase(scheme)) { + return 80; + } + return -1; + } + + public static String forwardedValue(final String value) { + if (value == null) { + return null; + } + final int comma = value.indexOf(','); + final String first = (comma >= 0 ? value.substring(0, comma) : value).trim(); + return first.isEmpty() ? null : first; + } + + public static void setServerAddressAndPort( + final AgentSpan span, + final String forwardedHostHeader, + final String forwardedPortHeader, + final String scheme, + final String requestHost, + final int requestPort) { + final String forwardedHost = forwardedValue(forwardedHostHeader); + String serverAddress = requestHost; + int hostPort = -1; + + if (forwardedHost != null) { + serverAddress = forwardedHost; + if (forwardedHost.charAt(0) == '[') { + final int closingBracket = forwardedHost.indexOf(']'); + if (closingBracket > 0) { + serverAddress = forwardedHost.substring(1, closingBracket); + if (closingBracket + 1 < forwardedHost.length() + && forwardedHost.charAt(closingBracket + 1) == ':') { + hostPort = parsePort(forwardedHost.substring(closingBracket + 2)); + } + } + } else { + final int colon = forwardedHost.lastIndexOf(':'); + if (colon > 0 && colon == forwardedHost.indexOf(':')) { + serverAddress = forwardedHost.substring(0, colon); + hostPort = parsePort(forwardedHost.substring(colon + 1)); + } + } + } + + if (serverAddress != null && !serverAddress.isEmpty()) { + span.setTag(KnownTags.SERVER_ADDRESS_NAME, serverAddress); + } + + int serverPort = parsePort(forwardedValue(forwardedPortHeader)); + if (serverPort <= 0) { + serverPort = hostPort; + } + if (serverPort <= 0) { + serverPort = forwardedHost == null && requestPort > 0 ? requestPort : serverPort(scheme, -1); + } + if (serverPort > 0) { + span.setTag(KnownTags.SERVER_PORT_NAME, serverPort); + } + } + + public static int parsePort(final String value) { + if (value == null || value.isEmpty()) { + return -1; + } + try { + final int port = Integer.parseInt(value); + return port > 0 && port <= 65535 ? port : -1; + } catch (NumberFormatException ignored) { + return -1; + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigTest.java index bbea22a73fa..56f789b9166 100644 --- a/internal-api/src/test/java/datadog/trace/api/ConfigTest.java +++ b/internal-api/src/test/java/datadog/trace/api/ConfigTest.java @@ -156,6 +156,7 @@ import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_ENABLED; import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_FLUSH_INTERVAL; import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL; +import static datadog.trace.api.config.TracerConfig.TRACE_PEER_SERVICE_DEFAULTS_ENABLED; import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_BEHAVIOR_EXTRACT; import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_EXTRACT_FIRST; import static datadog.trace.api.config.TracerConfig.TRACE_RATE_LIMIT; @@ -164,6 +165,7 @@ import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLE_RATE; import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLING_OPERATION_RULES; import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLING_SERVICE_RULES; +import static datadog.trace.api.config.TracerConfig.TRACE_SPAN_ATTRIBUTE_SCHEMA; import static datadog.trace.api.config.TracerConfig.TRACE_X_DATADOG_TAGS_MAX_LENGTH; import static datadog.trace.api.config.TracerConfig.WRITER_TYPE; import static datadog.trace.api.env.FixedCapturedEnvironment.useFixedEnv; @@ -698,6 +700,33 @@ void otelDefaultValuesWhenNotSet() { assertEquals(10000, config.getTraceStatsInterval()); } + @Test + void otelSemanticsForcesItsRequiredConfiguration() { + ConfigCollector.get().collect(); + Properties prop = new Properties(); + prop.setProperty(TRACE_OTEL_SEMANTICS_ENABLED, "true"); + prop.setProperty(TRACE_OTEL_EXPORTER, "agent"); + prop.setProperty(WRITER_TYPE, "LoggingWriter"); + prop.setProperty(TRACE_SPAN_ATTRIBUTE_SCHEMA, "v1"); + prop.setProperty(TRACE_PEER_SERVICE_DEFAULTS_ENABLED, "true"); + + Config config = Config.get(prop); + Map calculated = + ConfigCollector.get().collect().get(ConfigOrigin.CALCULATED); + + assertTrue(config.isTraceOtelSemanticsEnabled()); + assertTrue(config.isTraceOtlpExporterEnabled()); + assertEquals("OtlpWriter", config.getWriterType()); + assertEquals(0, config.getSpanAttributeSchemaVersion()); + assertFalse(config.isPeerServiceDefaultsEnabled()); + assertEquals(toBitSet(400, 599), config.getHttpClientErrorStatuses()); + + assertEquals("otlp", calculated.get(TRACE_OTEL_EXPORTER).value); + assertEquals("OtlpWriter", calculated.get(WRITER_TYPE).value); + assertEquals("v0", calculated.get(TRACE_SPAN_ATTRIBUTE_SCHEMA).value); + assertEquals(false, calculated.get(TRACE_PEER_SERVICE_DEFAULTS_ENABLED).value); + } + @Test void otelCheckSyntaxForOtlpHeaders() { Properties prop = new Properties(); @@ -1001,7 +1030,7 @@ void specifyOverridesViaSystemProperties() throws Exception { assertEquals("new site", config.getSite()); assertEquals("something else", config.getServiceName()); assertFalse(config.isTraceEnabled()); - assertEquals("LoggingWriter", config.getWriterType()); + assertEquals("OtlpWriter", config.getWriterType()); assertEquals("somehost", config.getAgentHost()); assertEquals(123, config.getAgentPort()); assertEquals("somepath", config.getAgentUnixDomainSocket()); diff --git a/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java b/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java index 8562ef3c8bf..1854639c4e4 100644 --- a/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java +++ b/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java @@ -37,8 +37,16 @@ static Stream knownTags() { Arguments.of(Tags.PEER_SERVICE, KnownTags.PEER_SERVICE_ID), Arguments.of(DDTags.PEER_SERVICE_REMAPPED_FROM, KnownTags.DD_PEER_SERVICE_REMAPPED_FROM_ID), Arguments.of(Tags.HTTP_METHOD, KnownTags.HTTP_METHOD_ID), + Arguments.of( + KnownTags.HTTP_REQUEST_METHOD_ORIGINAL_NAME, KnownTags.HTTP_REQUEST_METHOD_ORIGINAL_ID), Arguments.of(Tags.HTTP_ROUTE, KnownTags.HTTP_ROUTE_ID), Arguments.of(Tags.HTTP_URL, KnownTags.HTTP_URL_ID), + Arguments.of(KnownTags.SERVER_ADDRESS_NAME, KnownTags.SERVER_ADDRESS_ID), + Arguments.of(KnownTags.SERVER_PORT_NAME, KnownTags.SERVER_PORT_ID), + Arguments.of(KnownTags.URL_PATH_NAME, KnownTags.URL_PATH_ID), + Arguments.of(KnownTags.URL_QUERY_NAME, KnownTags.URL_QUERY_ID), + Arguments.of(KnownTags.URL_SCHEME_NAME, KnownTags.URL_SCHEME_ID), + Arguments.of(KnownTags.NETWORK_CLIENT_IP_NAME, KnownTags.NETWORK_CLIENT_IP_ID), Arguments.of(Tags.PEER_HOSTNAME, KnownTags.PEER_HOSTNAME_ID), Arguments.of(Tags.PEER_HOST_IPV4, KnownTags.PEER_IPV4_ID), Arguments.of(Tags.PEER_HOST_IPV6, KnownTags.PEER_IPV6_ID), @@ -64,6 +72,7 @@ static Stream otelNamedTags() { Arguments.of("url.full", KnownTags.HTTP_URL_ID, "http.url"), Arguments.of("user_agent.original", KnownTags.HTTP_USERAGENT_ID, "http.useragent"), Arguments.of("client.address", KnownTags.HTTP_CLIENT_IP_ID, "http.client_ip"), + Arguments.of("network.peer.address", KnownTags.NETWORK_CLIENT_IP_ID, "network.client.ip"), Arguments.of("db.system", KnownTags.DB_TYPE_ID, "db.type"), Arguments.of("db.operation.name", KnownTags.DB_OPERATION_ID, "db.operation"), Arguments.of("db.query.text", KnownTags.DB_STATEMENT_ID, "db.statement"), @@ -173,9 +182,8 @@ void aNameDeclaredPerDirectionIsOneTag() { assertEquals( "client.port", KnownTagCodec.openTelemetryTagOf(KnownTags.PEER_PORT_ID, KnownTagCodec.DIRECTION_INBOUND)); - assertEquals( - "server.port", - KnownTagCodec.openTelemetryTagOf(KnownTags.PEER_PORT_ID, KnownTagCodec.DIRECTION_OUTBOUND)); + // Temporarily remove "server.port" == openTelemetryTagOf(peer.port, outbound) because + // server.port in OTel is also used by server spans assertEquals( Tags.PEER_PORT, KnownTagCodec.openTelemetryTagOf(KnownTags.PEER_PORT_ID, KnownTagCodec.DIRECTION_NONE)); diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/OtelHttpSemanticsTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/OtelHttpSemanticsTest.java new file mode 100644 index 00000000000..1f588967368 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/OtelHttpSemanticsTest.java @@ -0,0 +1,299 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTags; +import java.net.URI; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.tabletest.junit.TableTest; + +class OtelHttpSemanticsTest { + + @ParameterizedTest + @ValueSource(strings = {"GET", "POST", "DELETE", "PATCH", "QUERY", "CONNECT"}) + void setRequestMethodKeepsKnownMethods(String method) { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setRequestMethod(span, method); + + verify(span).setTag(KnownTags.HTTP_METHOD_OTEL_NAME, method); + verify(span, never()).setTag(eq(KnownTags.HTTP_REQUEST_METHOD_ORIGINAL_NAME), anyString()); + } + + @ParameterizedTest + @ValueSource(strings = {"PROPFIND", "get", "Get", "BOGUS"}) + void setRequestMethodNormalizesUnknownMethodsToOther(String method) { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setRequestMethod(span, method); + + verify(span).setTag(KnownTags.HTTP_METHOD_OTEL_NAME, OtelHttpSemantics.OTHER_METHOD); + verify(span).setTag(KnownTags.HTTP_REQUEST_METHOD_ORIGINAL_NAME, method); + } + + @TableTest({ + "Scenario | Method | Expected", + "GET method | GET | GET ", + "POST method | POST | POST ", + "QUERY method | QUERY | QUERY ", + "Extension method | PROPFIND | HTTP ", + "Lowercase method | get | HTTP " + }) + void spanNameMethodUsesNormalizedMethod(String method, String expected) { + assertEquals(expected, OtelHttpSemantics.spanNameMethod(method)); + } + + @Test + void spanNameMethodUsesHttpForNullMethod() { + assertEquals("HTTP", OtelHttpSemantics.spanNameMethod(null)); + } + + @TableTest({ + "Scenario | Method | Expected", + "Known method | GET | GET ", + "Extension method | PROPFIND | HTTP " + }) + void setMethodResourceNameUsesNormalizedMethod(String method, String expected) { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setMethodResourceName(span, method); + + verify(span).setResourceName(expected, ResourceNamePriorities.HTTP_PATH_NORMALIZER); + } + + @Test + void setMethodResourceNameUsesHttpForNullMethod() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setMethodResourceName(span, null); + + verify(span).setResourceName("HTTP", ResourceNamePriorities.HTTP_PATH_NORMALIZER); + } + + @Test + void setClientUrlStripsQueryAndRedactsCredentials() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setClientUrl(span, URI.create("https://u:p@host/a?token=secret")); + + verify(span).setTag(KnownTags.HTTP_URL_OTEL_NAME, "https://REDACTED:REDACTED@host/a"); + verify(span).setTag(KnownTags.URL_QUERY_NAME, "token=secret"); + } + + @Test + void setClientUrlOmitsQueryTagWhenAbsent() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setClientUrl(span, URI.create("http://host/a")); + + verify(span).setTag(KnownTags.HTTP_URL_OTEL_NAME, "http://host/a"); + verify(span, never()).setTag(eq(KnownTags.URL_QUERY_NAME), anyString()); + } + + @TableTest({ + "Scenario | Raw URL | Expected URL ", + "No credentials | http://host:8080/p | http://host:8080/p ", + "Username and password | http://user:pass@host/p | http://REDACTED:REDACTED@host/p ", + "Username only | http://user@host/p | http://REDACTED@host/p ", + "Query and fragment | https://u:p@host:443/a?b=c#frag | https://REDACTED:REDACTED@host:443/a?b=c#frag", + "Credentials in path | https://u:p@host/u:p@path | https://REDACTED:REDACTED@host/u:p@path ", + "Underscore host | http://user:pass@foo_bar/p | http://REDACTED:REDACTED@foo_bar/p ", + "Underscore username | http://user_@host/p | http://REDACTED@host/p ", + "Unicode host | http://user:pass@例え.テスト/p | http://REDACTED:REDACTED@例え.テスト/p ", + "Encoded credentials | http://u%40ser:p%40ss@foo_bar/p | http://REDACTED:REDACTED@foo_bar/p ", + "At sign in password | http://user:p@ss@foo_bar/p | http://REDACTED:REDACTED@foo_bar/p ", + "Empty username | http://:pass@foo_bar/p | http://REDACTED:REDACTED@foo_bar/p ", + "IPv6 host | http://user:pass@[::1]:8080/p | http://REDACTED:REDACTED@[::1]:8080/p ", + "IPv6 username only | http://user@[::1]:8080/p | http://REDACTED@[::1]:8080/p ", + "Scheme relative URL | //user:pass@foo_bar/p | //REDACTED:REDACTED@foo_bar/p ", + "At signs outside host | http://foo_bar/u:p@path?q=u:p@query | http://foo_bar/u:p@path?q=u:p@query ", + "Relative path | /u:p@path | /u:p@path ", + "Opaque URI | mailto:user:pass@host | mailto:user:pass@host " + }) + void redactedUrlRedactsCredentials(String raw, String expected) { + assertEquals(expected, OtelHttpSemantics.redactedUrl(URI.create(raw))); + } + + @TableTest({ + "Scenario | URL | Expected ", + "Query removed | http://host/p?token=secret | http://host/p ", + "Fragment retained | http://host/p#frag | http://host/p#frag ", + "Query before fragment | http://host/p?q=1#frag | http://host/p#frag ", + "Question mark in fragment | http://host/p#frag?q=1 | http://host/p#frag?q=1", + "No query or fragment | http://host/p | http://host/p " + }) + void withoutQueryPreservesFragment(String url, String expected) { + assertEquals(expected, OtelHttpSemantics.withoutQuery(url)); + } + + @TableTest({ + "Scenario | URL | Expected Port", + "Explicit port | http://host:8080/ | 8080 ", + "HTTP default | http://host/ | 80 ", + "HTTPS default | https://host/ | 443 ", + "Unknown scheme | ftp://host/ | -1 " + }) + void serverPortFallsBackToSchemeDefault(String url, int expectedPort) { + assertEquals(expectedPort, OtelHttpSemantics.serverPort(URI.create(url))); + } + + @Test + void setErrorTypeSetsStatusWhenAbsent() { + AgentSpan span = mock(AgentSpan.class); + when(span.getTag(DDTags.ERROR_TYPE)).thenReturn(null); + + OtelHttpSemantics.setErrorType(span, 500); + + verify(span).setTag(DDTags.ERROR_TYPE, "500"); + } + + @Test + void setErrorTypeDoesNotOverrideExistingErrorType() { + AgentSpan span = mock(AgentSpan.class); + when(span.getTag(DDTags.ERROR_TYPE)).thenReturn("java.net.UnknownHostException"); + + OtelHttpSemantics.setErrorType(span, 500); + + verify(span, never()).setTag(eq(DDTags.ERROR_TYPE), any(String.class)); + } + + @TableTest({ + "Scenario | Header | Expected", + "Single host | {host, ' host '} | host ", + "First forwarded host | {'host, proxy', ' host , proxy'} | host " + }) + void forwardedValueUsesFirstTrimmedEntry(String header, String expected) { + assertEquals(expected, OtelHttpSemantics.forwardedValue(header)); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", ", proxy"}) + void forwardedValueReturnsNullForBlankFirstEntry(String header) { + assertNull(OtelHttpSemantics.forwardedValue(header)); + } + + @Test + void forwardedValueReturnsNullForNullHeader() { + assertNull(OtelHttpSemantics.forwardedValue(null)); + } + + @TableTest({ + "Scenario | Value | Expected", + "Valid port | 8080 | 8080 ", + "Minimum port | 1 | 1 ", + "Maximum port | 65535 | 65535 ", + "Invalid port | {0, 65536, -1, abc} | -1 " + }) + void parsePortRejectsOutOfRangeAndMalformedValues(String value, int expected) { + assertEquals(expected, OtelHttpSemantics.parsePort(value)); + } + + @Test + void parsePortReturnsUnsetForEmptyValue() { + assertEquals(-1, OtelHttpSemantics.parsePort("")); + } + + @Test + void parsePortReturnsUnsetForNullValue() { + assertEquals(-1, OtelHttpSemantics.parsePort(null)); + } + + @TableTest({ + "Scenario | Scheme | Port | Expected", + "HTTPS default | {https, HTTPS} | -1 | 443 ", + "HTTP default | {http, HTTP} | -1 | 80 ", + "Unknown scheme | ftp | -1 | -1 " + }) + void serverPortUsesSchemeDefaultWhenPortUnset(String scheme, int port, int expected) { + assertEquals(expected, OtelHttpSemantics.serverPort(scheme, port)); + } + + @Test + void serverPortReturnsUnsetForNullScheme() { + assertEquals(-1, OtelHttpSemantics.serverPort(null, -1)); + } + + @Test + void setServerAddressAndPortUsesRequestHostAndPort() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setServerAddressAndPort(span, null, null, "http", "host", 8080); + + verify(span).setTag(KnownTags.SERVER_ADDRESS_NAME, "host"); + verify(span).setTag(KnownTags.SERVER_PORT_NAME, 8080); + } + + @Test + void setServerAddressAndPortFallsBackToSchemeDefaultPort() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setServerAddressAndPort(span, null, null, "https", "host", -1); + + verify(span).setTag(KnownTags.SERVER_ADDRESS_NAME, "host"); + verify(span).setTag(KnownTags.SERVER_PORT_NAME, 443); + } + + @Test + void setServerAddressAndPortOmitsTagsWhenNothingResolvable() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setServerAddressAndPort(span, null, null, "ftp", null, -1); + + verify(span, never()).setTag(eq(KnownTags.SERVER_ADDRESS_NAME), anyString()); + verify(span, never()).setTag(eq(KnownTags.SERVER_PORT_NAME), anyInt()); + } + + @Test + void setServerAddressAndPortOmitsAddressWhenRequestHostIsEmpty() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setServerAddressAndPort(span, null, null, "http", "", -1); + + verify(span, never()).setTag(eq(KnownTags.SERVER_ADDRESS_NAME), anyString()); + verify(span).setTag(KnownTags.SERVER_PORT_NAME, 80); + } + + @Test + void setServerAddressAndPortPrefersForwardedPortHeader() { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setServerAddressAndPort(span, "proxy:8443", "9443", "http", "host", 8080); + + verify(span).setTag(KnownTags.SERVER_ADDRESS_NAME, "proxy"); + verify(span).setTag(KnownTags.SERVER_PORT_NAME, 9443); + } + + @TableTest({ + "Scenario | Forwarded Host | Expected Address | Expected Port", + "Host only | proxy | proxy | 80 ", + "Host with port | proxy:8443 | proxy | 8443 ", + "Host with invalid port | proxy:bogus | proxy | 80 ", + "IPv6 host | '[::1]' | ::1 | 80 ", + "IPv6 with port | '[::1]:8443' | ::1 | 8443 ", + "IPv6 with invalid suffix | '[::1]bogus' | ::1 | 80 ", + "Unbracketed IPv6 | ::1 | ::1 | 80 ", + "Unclosed IPv6 bracket | '[::1' | '[::1' | 80 " + }) + void setServerAddressAndPortParsesForwardedHost( + String forwardedHost, String expectedAddress, int expectedPort) { + AgentSpan span = mock(AgentSpan.class); + + OtelHttpSemantics.setServerAddressAndPort(span, forwardedHost, null, "http", "host", 8080); + + verify(span).setTag(KnownTags.SERVER_ADDRESS_NAME, expectedAddress); + // A forwarded host suppresses the request port, so the scheme default applies instead. + verify(span).setTag(KnownTags.SERVER_PORT_NAME, expectedPort); + } +} diff --git a/tag-conventions.yaml b/tag-conventions.yaml index b54462a6a62..af2d3283b86 100644 --- a/tag-conventions.yaml +++ b/tag-conventions.yaml @@ -98,10 +98,13 @@ span_types: abstract: true extends: base tags: - - { dd-name: http.method, type: string, required: required, otel-name: http.request.method } - - { dd-name: http.status_code, type: int, required: conditional, otel-name: http.response.status_code } - - { dd-name: network.protocol.version, type: string, required: recommended } # passes through: dd-name already is the OTel name - - { dd-name: http.url, type: string, required: required, otel-name: url.full } # shared by http.server and http.client => one otel-name. url.full is the client-correct rename; server's spec mapping (url.path + url.scheme + url.query) is a one-to-many split reserved for the derivation layer (needs span.kind). TODO(otel): server split. + - { dd-name: http.method, type: string, required: required, otel-name: http.request.method } + - { dd-name: http.request.method_original, type: string, required: conditional } # OTel-only; set only while OTel semantics are enabled + - { dd-name: http.status_code, type: int, required: conditional, otel-name: http.response.status_code } + - { dd-name: network.protocol.version, type: string, required: recommended } # passes through: dd-name already is the OTel name + - { dd-name: http.url, type: string, required: required, otel-name: url.full } # shared by http.server and http.client => one otel-name. url.full is the client-correct rename; server's spec mapping (url.path + url.scheme + url.query) is a one-to-many split reserved for the derivation layer (needs span.kind). TODO(otel): server split. + - { dd-name: server.address, type: string, required: conditional } # OTel-only; set only while OTel semantics are enabled + - { dd-name: server.port, type: int, required: conditional } # OTel-only; set only while OTel semantics are enabled http.server: extends: http @@ -109,15 +112,16 @@ span_types: include: [ peer_address, inbound_peer ] tags: - { dd-name: http.route, type: string, required: conditional } # passes through: dd-name already is the OTel name - - { dd-name: http.hostname, type: string, required: required, otel-name: server.address } # inbound only; on outbound spans server.address is peer.hostname + - { dd-name: http.hostname, type: string, required: required } - { dd-name: http.useragent, type: string, required: recommended, otel-name: user_agent.original, span-kind-neutral: true } - { dd-name: http.query.string, type: string, required: recommended } # not a rename: url.query is also carried inside url.full, while http.url excludes the query (QueryObfuscator re-appends it) + - { dd-name: url.path, type: string, required: conditional } # OTel-only; server replacement for http.url + - { dd-name: url.query, type: string, required: conditional } # OTel-only; server replacement for http.url + - { dd-name: url.scheme, type: string, required: conditional } # OTel-only; server replacement for http.url - { dd-name: servlet.path, type: string, required: optional } - { dd-name: servlet.context, type: string, required: optional } - { dd-name: http.client_ip, type: string, required: recommended, otel-name: client.address, span-kind-neutral: true } - # Inbound only: the client's socket address. Server spans also carry that address in peer.ipv4 / - # peer.ipv6, so do not map those inbound too, or the attribute is exported twice. - - { dd-name: network.client.ip, type: string, required: recommended, otel-name: network.peer.address } + - { dd-name: network.client.ip, type: string, required: recommended, otel-name: network.peer.address, span-kind-neutral: true } # Temporary direction-free alias until name resolution is span-kind-aware http.client: extends: http @@ -169,14 +173,14 @@ mixins: outbound_peer: span-kind: client tags: - - { dd-name: peer.hostname, type: string, required: recommended, otel-name: server.address } - - { dd-name: peer.port, type: int, otel-name: server.port } + - { dd-name: peer.hostname, type: string, required: recommended } + - { dd-name: peer.port, type: int } # inbound_peer — the other end of an inbound connection: the client. inbound_peer: span-kind: server tags: - - { dd-name: peer.port, type: int, otel-name: client.port } + - { dd-name: peer.port, type: int, otel-name: client.port } # ci_visibility — per-span test tags. Its capability flag (_dd.civisibility.enabled) lives in # trace_level, outside this mixin (general rule: capability flags are trace-level, mixins hold the