From 71101ac8d0695b16533cd406621878d4f1fb08f5 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 28 May 2019 18:28:55 -0700 Subject: [PATCH] Add http.query.string tag when enabled Disabled by default. Enable for http servers with: * System Property: `dd.http.server.tag.query-string=true` * Environment Variable: `DD_HTTP_SERVER_TAG_QUERY_STRING=true` Enable for http clients with: * System Property: `dd.http.client.tag.query-string=true` * Environment Variable: `DD_HTTP_CLIENT_TAG_QUERY_STRING=true` --- .../agent/decorator/HttpClientDecorator.java | 5 +++ .../agent/decorator/HttpServerDecorator.java | 5 +++ .../decorator/HttpClientDecoratorTest.groovy | 34 +++++++++++++------ .../decorator/HttpServerDecoratorTest.groovy | 34 +++++++++++++------ .../agent/test/base/HttpClientTest.groovy | 8 ++++- .../main/java/datadog/trace/api/Config.java | 22 ++++++++++++ 6 files changed, 85 insertions(+), 23 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpClientDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpClientDecorator.java index a57e6ccb99..e0450e21b9 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpClientDecorator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpClientDecorator.java @@ -61,6 +61,11 @@ public abstract class HttpClientDecorator extends ClientDecor } Tags.HTTP_URL.set(span, urlNoParams.toString()); + + if (Config.get().isHttpClientTagQueryString()) { + span.setTag("http.query.string", url.getQuery()); + span.setTag("http.fragment.string", url.getFragment()); + } } } catch (final Exception e) { log.debug("Error tagging url", e); diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java index 5e89056af4..9e9f7c56c7 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java @@ -67,6 +67,11 @@ public abstract class HttpServerDecorator extends } Tags.HTTP_URL.set(span, urlNoParams.toString()); + + if (Config.get().isHttpServerTagQueryString()) { + span.setTag("http.query.string", url.getQuery()); + span.setTag("http.fragment.string", url.getFragment()); + } } } catch (final Exception e) { log.debug("Error tagging url", e); diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpClientDecoratorTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpClientDecoratorTest.groovy index 0eccd44bf3..6bd5c12360 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpClientDecoratorTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpClientDecoratorTest.groovy @@ -44,16 +44,22 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { true | [method: "test-method", url: testUrl, host: "test-host", port: 555] } - def "test url handling"() { + def "test url handling for #url"() { setup: def decorator = newDecorator() when: - decorator.onRequest(span, req) + withConfigOverride(Config.HTTP_CLIENT_TAG_QUERY_STRING, "$tagQueryString") { + decorator.onRequest(span, req) + } then: - if (expected) { - 1 * span.setTag(Tags.HTTP_URL.key, expected) + if (expectedUrl) { + 1 * span.setTag(Tags.HTTP_URL.key, expectedUrl) + } + if (expectedUrl && tagQueryString) { + 1 * span.setTag("http.query.string", expectedQuery) + 1 * span.setTag("http.fragment.string", expectedFragment) } 1 * span.setTag(Tags.HTTP_METHOD.key, null) 1 * span.setTag(Tags.PEER_HOSTNAME.key, null) @@ -61,13 +67,19 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { 0 * _ where: - url | expected - null | null - "" | "/" - "/path?query" | "/path" - "https://host:0" | "https://host/" - "https://host/path" | "https://host/path" - "http://host:99/path?query#fragment" | "http://host:99/path" + tagQueryString | url | expectedUrl | expectedQuery | expectedFragment + false | null | null | null | null + false | "" | "/" | "" | null + false | "/path?query" | "/path" | "" | null + false | "https://host:0" | "https://host/" | "" | null + false | "https://host/path" | "https://host/path" | "" | null + false | "http://host:99/path?query#fragment" | "http://host:99/path" | "" | null + true | null | null | null | null + true | "" | "/" | null | null + true | "/path?encoded+%28query%29%3F" | "/path" | "encoded+(query)?" | null + true | "https://host:0" | "https://host/" | null | null + true | "https://host/path" | "https://host/path" | null | null + true | "http://host:99/path?query#encoded+%28fragment%29%3F" | "http://host:99/path" | "query" | "encoded+(fragment)?" req = [url: url == null ? null : new URI(url)] } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy index 7848b4cc30..3716a9bb4e 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy @@ -34,28 +34,40 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { [method: "test-method", url: URI.create("http://123:8080/some/path")] | "http://123:8080/some/path" } - def "test url handling"() { + def "test url handling for #url"() { setup: def decorator = newDecorator() when: - decorator.onRequest(span, req) + withConfigOverride(Config.HTTP_SERVER_TAG_QUERY_STRING, "$tagQueryString") { + decorator.onRequest(span, req) + } then: - if (expected) { - 1 * span.setTag(Tags.HTTP_URL.key, expected) + if (expectedUrl) { + 1 * span.setTag(Tags.HTTP_URL.key, expectedUrl) + } + if (expectedUrl && tagQueryString) { + 1 * span.setTag("http.query.string", expectedQuery) + 1 * span.setTag("http.fragment.string", expectedFragment) } 1 * span.setTag(Tags.HTTP_METHOD.key, null) 0 * _ where: - url | expected - null | null - "" | "/" - "/path?query" | "/path" - "https://host:0" | "https://host/" - "https://host/path" | "https://host/path" - "http://host:99/path?query#fragment" | "http://host:99/path" + tagQueryString | url | expectedUrl | expectedQuery | expectedFragment + false | null | null | null | null + false | "" | "/" | "" | null + false | "/path?query" | "/path" | "" | null + false | "https://host:0" | "https://host/" | "" | null + false | "https://host/path" | "https://host/path" | "" | null + false | "http://host:99/path?query#fragment" | "http://host:99/path" | "" | null + true | null | null | null | null + true | "" | "/" | null | null + true | "/path?encoded+%28query%29%3F" | "/path" | "encoded+(query)?" | null + true | "https://host:0" | "https://host/" | null | null + true | "https://host/path" | "https://host/path" | null | null + true | "http://host:99/path?query#encoded+%28fragment%29%3F" | "http://host:99/path" | "query" | "encoded+(fragment)?" req = [url: url == null ? null : new URI(url)] } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy index 77729217fb..106966aceb 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy @@ -70,7 +70,9 @@ abstract class HttpClientTest extends AgentTestRu @Unroll def "basic #method request #url"() { when: - def status = doRequest(method, server.address.resolve(url)) + def status = withConfigOverride(Config.HTTP_CLIENT_TAG_QUERY_STRING, "$tagQueryString") { + doRequest(method, url) + } then: status == 200 @@ -350,6 +352,10 @@ abstract class HttpClientTest extends AgentTestRu "$Tags.HTTP_STATUS.key" status } "$Tags.HTTP_URL.key" "${uri.resolve(uri.path)}" + if (tagQueryString) { + "http.query.string" uri.query + "http.fragment.string" { it == null || it == uri.fragment } // Optional + } "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" uri.port "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index ffda653d25..9c7fd6b064 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -57,6 +57,8 @@ public class Config { public static final String HEADER_TAGS = "trace.header.tags"; public static final String HTTP_SERVER_ERROR_STATUSES = "http.server.error.statuses"; public static final String HTTP_CLIENT_ERROR_STATUSES = "http.client.error.statuses"; + public static final String HTTP_SERVER_TAG_QUERY_STRING = "http.server.tag.query-string"; + public static final String HTTP_CLIENT_TAG_QUERY_STRING = "http.client.tag.query-string"; public static final String HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN = "trace.http.client.split-by-domain"; public static final String PARTIAL_FLUSH_MIN_SPANS = "trace.partial.flush.min.spans"; public static final String RUNTIME_CONTEXT_FIELD_INJECTION = @@ -98,6 +100,8 @@ public class Config { parseIntegerRangeSet("500-599", "default"); private static final Set DEFAULT_HTTP_CLIENT_ERROR_STATUSES = parseIntegerRangeSet("400-499", "default"); + private static final boolean DEFAULT_HTTP_SERVER_TAG_QUERY_STRING = true; + private static final boolean DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING = true; private static final boolean DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN = false; private static final int DEFAULT_PARTIAL_FLUSH_MIN_SPANS = 1000; private static final String DEFAULT_PROPAGATION_STYLE_EXTRACT = PropagationStyle.DATADOG.name(); @@ -142,6 +146,8 @@ public class Config { @Getter private final Map headerTags; @Getter private final Set httpServerErrorStatuses; @Getter private final Set httpClientErrorStatuses; + @Getter private final boolean httpServerTagQueryString; + @Getter private final boolean httpClientTagQueryString; @Getter private final boolean httpClientSplitByDomain; @Getter private final Integer partialFlushMinSpans; @Getter private final boolean runtimeContextFieldInjection; @@ -198,6 +204,14 @@ public class Config { getIntegerRangeSettingFromEnvironment( HTTP_CLIENT_ERROR_STATUSES, DEFAULT_HTTP_CLIENT_ERROR_STATUSES); + httpServerTagQueryString = + getBooleanSettingFromEnvironment( + HTTP_SERVER_TAG_QUERY_STRING, DEFAULT_HTTP_SERVER_TAG_QUERY_STRING); + + httpClientTagQueryString = + getBooleanSettingFromEnvironment( + HTTP_CLIENT_TAG_QUERY_STRING, DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING); + httpClientSplitByDomain = getBooleanSettingFromEnvironment( HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN); @@ -280,6 +294,14 @@ public class Config { getPropertyIntegerRangeValue( properties, HTTP_CLIENT_ERROR_STATUSES, parent.httpClientErrorStatuses); + httpServerTagQueryString = + getPropertyBooleanValue( + properties, HTTP_SERVER_TAG_QUERY_STRING, parent.httpServerTagQueryString); + + httpClientTagQueryString = + getPropertyBooleanValue( + properties, HTTP_CLIENT_TAG_QUERY_STRING, parent.httpClientTagQueryString); + httpClientSplitByDomain = getPropertyBooleanValue( properties, HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, parent.httpClientSplitByDomain);