From 256107213f447ab92b8828b4f2550403df704316 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 27 Jun 2025 08:51:47 -0400 Subject: [PATCH 1/5] Saving branch --- .../instrumentation/decorator/HttpServerDecorator.java | 9 +++++++++ internal-api/src/main/java/datadog/trace/api/Config.java | 4 ++++ .../trace/bootstrap/config/provider/ConfigConverter.java | 6 ++++++ .../trace/bootstrap/config/provider/ConfigProvider.java | 1 + .../src/test/groovy/datadog/trace/api/ConfigTest.groovy | 3 +++ 5 files changed, 23 insertions(+) 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 b286863db5b..3cb12da020f 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 @@ -642,14 +642,23 @@ static ResponseHeaderTagClassifier create(AgentSpan span, Map he private final AgentSpan span; private final Map headerTags; + private final String wildcardHeaderPrefix; public ResponseHeaderTagClassifier(AgentSpan span, Map headerTags) { this.span = span; this.headerTags = headerTags; + if(headerTags.size() > 0) + System.out.println("responseHeaderTags Post-Processing\nsize: " + headerTags.size() + "; " + headerTags.keySet().toArray()[0] + " : " + headerTags.get(headerTags.keySet().toArray()[0])); + this.wildcardHeaderPrefix = this.headerTags.getOrDefault("*", null); } @Override public boolean accept(String key, String value) { + System.out.println("key: " + key); + if (wildcardHeaderPrefix != null) { + System.out.println("wildcardKey: " + wildcardHeaderPrefix + key); + span.setTag(wildcardHeaderPrefix + key, value); + } String mappedKey = headerTags.get(key.toLowerCase(Locale.ROOT)); if (mappedKey != null) { span.setTag(mappedKey, value); 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 8674954e210..1fc0e8cbc4b 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -833,12 +833,16 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins logIgnoredSettingWarning(RESPONSE_HEADER_TAGS, HEADER_TAGS, ".legacy.parsing.enabled"); } } else { + System.out.println("requestHeaderTags: "); requestHeaderTags = configProvider.getMergedMapWithOptionalMappings( "http.request.headers.", true, HEADER_TAGS, REQUEST_HEADER_TAGS); + System.out.println("responseHeaderTags: "); responseHeaderTags = configProvider.getMergedMapWithOptionalMappings( "http.response.headers.", true, HEADER_TAGS, RESPONSE_HEADER_TAGS); + if(responseHeaderTags.size() > 0) + System.out.println("responseHeaderTags Post-Processing\nsize: " + responseHeaderTags.size() + "; " + responseHeaderTags.keySet().toArray()[0] + " : " + responseHeaderTags.get(responseHeaderTags.keySet().toArray()[0])); } requestHeaderTagsCommaAllowed = configProvider.getBoolean(REQUEST_HEADER_TAGS_COMMA_ALLOWED, true); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java index 98d4af50d3c..b25ee67b193 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java @@ -312,14 +312,20 @@ private static void loadMapWithOptionalMapping( String value; if (delimiter == mapPos) { value = trimmedHeader(str, delimiter + 1, end, false); + System.out.println("delimiter==mapPos value: " + value); // tags must start with a letter if (!value.isEmpty() && !Character.isLetter(value.charAt(0))) { throw new BadFormatException( "Illegal tag starting with non letter for key '" + key + "'"); } } else { + if(key.charAt(0) == '*'){ + map.put(key, defaultPrefix); + return; + } if (Character.isLetter(key.charAt(0))) { value = defaultPrefix + Strings.normalizedHeaderTag(key); + System.out.println("else value: " + value); } else { // tags must start with a letter throw new BadFormatException( diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 0a2715dd11a..59437b63b2a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -318,6 +318,7 @@ public Map getMergedMapWithOptionalMappings( for (String key : keys) { for (int i = sources.length - 1; 0 <= i; i--) { String value = sources[i].get(key); +// System.out.println("value: " + value); Map parsedMap = ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys); if (!parsedMap.isEmpty()) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 7735fc0f40b..9913e150087 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -165,6 +165,7 @@ class ConfigTest extends DDSpecification { private static final DD_JMXFETCH_METRICS_CONFIGS_ENV = "DD_JMXFETCH_METRICS_CONFIGS" private static final DD_TRACE_AGENT_PORT_ENV = "DD_TRACE_AGENT_PORT" private static final DD_AGENT_PORT_LEGACY_ENV = "DD_AGENT_PORT" + private static final DD_TRACE_HEADER_TAGS = "DD_TRACE_HEADER_TAGS" private static final DD_TRACE_REPORT_HOSTNAME = "DD_TRACE_REPORT_HOSTNAME" private static final DD_RUNTIME_METRICS_ENABLED_ENV = "DD_RUNTIME_METRICS_ENABLED" private static final DD_TRACE_LONG_RUNNING_ENABLED = "DD_TRACE_EXPERIMENTAL_LONG_RUNNING_ENABLED" @@ -568,6 +569,7 @@ class ConfigTest extends DDSpecification { environmentVariables.set(DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH, "42") environmentVariables.set(DD_TRACE_LONG_RUNNING_ENABLED, "true") environmentVariables.set(DD_TRACE_LONG_RUNNING_FLUSH_INTERVAL, "81") + environmentVariables.set(DD_TRACE_HEADER_TAGS, "*") when: def config = new Config() @@ -587,6 +589,7 @@ class ConfigTest extends DDSpecification { config.xDatadogTagsMaxLength == 42 config.isLongRunningTraceEnabled() config.getLongRunningTraceFlushInterval() == 81 + config.responseHeaderTags == ["*":"http.response.headers."] } def "sys props override env vars"() { From 0f8e74691fa934355f43655b69e2b93574793f32 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 30 Jun 2025 17:16:44 -0500 Subject: [PATCH 2/5] setting up structure and adding initial unit test for header tags --- .../decorator/HttpServerDecorator.java | 6 +- .../decorator/HttpServerDecoratorTest.groovy | 122 ++++++++++++------ 2 files changed, 81 insertions(+), 47 deletions(-) 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 3cb12da020f..c84eca2b56d 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 @@ -647,17 +647,13 @@ static ResponseHeaderTagClassifier create(AgentSpan span, Map he public ResponseHeaderTagClassifier(AgentSpan span, Map headerTags) { this.span = span; this.headerTags = headerTags; - if(headerTags.size() > 0) - System.out.println("responseHeaderTags Post-Processing\nsize: " + headerTags.size() + "; " + headerTags.keySet().toArray()[0] + " : " + headerTags.get(headerTags.keySet().toArray()[0])); this.wildcardHeaderPrefix = this.headerTags.getOrDefault("*", null); } @Override public boolean accept(String key, String value) { - System.out.println("key: " + key); if (wildcardHeaderPrefix != null) { - System.out.println("wildcardKey: " + wildcardHeaderPrefix + key); - span.setTag(wildcardHeaderPrefix + key, value); + span.setTag((wildcardHeaderPrefix + key).toLowerCase(Locale.ROOT), value); } String mappedKey = headerTags.get(key.toLowerCase(Locale.ROOT)); if (mappedKey != null) { diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy index daf09cad3e4..6cb97a9c578 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.api.gateway.Flow import datadog.trace.api.gateway.InstrumentationGateway import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot +import datadog.trace.api.TraceConfig import datadog.trace.bootstrap.ActiveSubsystems import datadog.trace.bootstrap.instrumentation.api.AgentPropagation import datadog.trace.bootstrap.instrumentation.api.AgentSpan @@ -350,6 +351,30 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { null | null | false } + def "test response headers with trace.header.tags"() { + setup: + injectSysConfig("trace.header.tags", headerTags) + def traceConfig = Mock(TraceConfig) + traceConfig.getResponseHeaderTags() >> [(headerTags.split(":")[0].toLowerCase()): headerTags.split(":")[1]] + + def responseSpan = Mock(AgentSpan) + responseSpan.traceConfig() >> traceConfig + + def decorator = newDecorator() + + when: + decorator.onResponse(responseSpan, resp) + + then: + for (Map.Entry entry : expectedTag.entrySet()) { + responseSpan.getTag(entry.getKey()).equals(entry.getValue()) + } + + where: + headerTags | resp | expectedTag + "X-Custom-Header:abc" | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | [abc:"custom-value"] + } + @Override def newDecorator() { return newDecorator(null) @@ -361,61 +386,74 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { } return new HttpServerDecorator>() { - @Override - protected TracerAPI tracer() { - return tracer - } + @Override + protected TracerAPI tracer() { + return tracer + } - @Override - protected String[] instrumentationNames() { - return ["test1", "test2"] - } + @Override + protected String[] instrumentationNames() { + return ["test1", "test2"] + } - @Override - protected CharSequence component() { - return "test-component" - } + @Override + protected CharSequence component() { + return "test-component" + } - @Override - protected AgentPropagation.ContextVisitor> getter() { - return ContextVisitors.stringValuesMap() - } + @Override + protected AgentPropagation.ContextVisitor> getter() { + return ContextVisitors.stringValuesMap() + } - @Override - protected AgentPropagation.ContextVisitor responseGetter() { - return null - } + @Override + protected AgentPropagation.ContextVisitor responseGetter() { + return new MapCarrierVisitor() + } - @Override - CharSequence spanName() { - return "http-test-span" - } + @Override + CharSequence spanName() { + return "http-test-span" + } - @Override - protected String method(Map m) { - return m.method - } + @Override + protected String method(Map m) { + return m.method + } - @Override - protected URIDataAdapter url(Map m) { - return m.url == null ? null : new URIDefaultDataAdapter(m.url) - } + @Override + protected URIDataAdapter url(Map m) { + return m.url == null ? null : new URIDefaultDataAdapter(m.url) + } - @Override - protected String peerHostIP(Map m) { - return m.peerIp - } + @Override + protected String peerHostIP(Map m) { + return m.peerIp + } - @Override - protected int peerPort(Map m) { - return m.port == null ? 0 : m.port - } + @Override + protected int peerPort(Map m) { + return m.port == null ? 0 : m.port + } + @Override + protected int status(Map m) { + return m.status == null ? 0 : m.status + } + + static class MapCarrierVisitor + implements AgentPropagation.ContextVisitor { @Override - protected int status(Map m) { - return m.status == null ? 0 : m.status + void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) { + Map headers = carrier.headers + if (headers != null) { + for (Map.Entry entry : headers.entrySet()) { + classifier.accept(entry.key, entry.value) + } + } } } + } } def "test startSpan and InstrumentationGateway"() { From c56858fb956da3748e5e3936700e59096938e823 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 1 Jul 2025 12:23:41 -0500 Subject: [PATCH 3/5] fixing and adding unit tests --- .../decorator/HttpServerDecoratorTest.groovy | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy index 6cb97a9c578..17d83190ca8 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy @@ -34,6 +34,7 @@ import static datadog.trace.api.gateway.Events.EVENTS class HttpServerDecoratorTest extends ServerDecoratorTest { def span = Mock(AgentSpan) + def respHeaders = ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json'] boolean origAppSecActive @@ -353,13 +354,18 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { def "test response headers with trace.header.tags"() { setup: - injectSysConfig("trace.header.tags", headerTags) def traceConfig = Mock(TraceConfig) - traceConfig.getResponseHeaderTags() >> [(headerTags.split(":")[0].toLowerCase()): headerTags.split(":")[1]] - + traceConfig.getResponseHeaderTags() >> headerTags + + def tags = [:] + def responseSpan = Mock(AgentSpan) responseSpan.traceConfig() >> traceConfig - + responseSpan.setTag(_, _) >> { String k, String v -> + tags[k] = v + return responseSpan + } + def decorator = newDecorator() when: @@ -367,12 +373,14 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { then: for (Map.Entry entry : expectedTag.entrySet()) { - responseSpan.getTag(entry.getKey()).equals(entry.getValue()) + assert tags[entry.getKey()] == entry.getValue() } where: - headerTags | resp | expectedTag - "X-Custom-Header:abc" | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | [abc:"custom-value"] + headerTags | resp | expectedTag + [:] | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | [:] + ["x-custom-header": "abc"] | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | [abc:"custom-value"] + ["*": "datadog.response.headers."] | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | ["datadog.response.headers.x-custom-header":"custom-value", "datadog.response.headers.content-type":"application/json"] } @Override From b6a465b9699d10c4bc4efaa0ff218323f67e8992 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 1 Jul 2025 14:30:36 -0500 Subject: [PATCH 4/5] updating unit tests --- .../decorator/HttpServerDecoratorTest.groovy | 115 +++++++++--------- .../main/java/datadog/trace/api/Config.java | 4 - .../config/provider/ConfigConverter.java | 4 +- .../config/provider/ConfigProvider.java | 1 - .../datadog/trace/api/ConfigTest.groovy | 1 + .../provider/ConfigConverterTest.groovy | 37 +++--- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy index 17d83190ca8..a64c7751802 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy @@ -366,7 +366,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { return responseSpan } - def decorator = newDecorator() + def decorator = newDecorator(null, false) when: decorator.onResponse(responseSpan, resp) @@ -385,83 +385,86 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { @Override def newDecorator() { - return newDecorator(null) + return newDecorator(null, true) } - def newDecorator(TracerAPI tracer) { + def newDecorator(TracerAPI tracer, boolean noopResponseGetter) { if (!tracer) { tracer = AgentTracer.NOOP_TRACER } return new HttpServerDecorator>() { - @Override - protected TracerAPI tracer() { - return tracer - } + @Override + protected TracerAPI tracer() { + return tracer + } - @Override - protected String[] instrumentationNames() { - return ["test1", "test2"] - } + @Override + protected String[] instrumentationNames() { + return ["test1", "test2"] + } - @Override - protected CharSequence component() { - return "test-component" - } + @Override + protected CharSequence component() { + return "test-component" + } - @Override - protected AgentPropagation.ContextVisitor> getter() { - return ContextVisitors.stringValuesMap() - } + @Override + protected AgentPropagation.ContextVisitor> getter() { + return ContextVisitors.stringValuesMap() + } - @Override - protected AgentPropagation.ContextVisitor responseGetter() { - return new MapCarrierVisitor() - } + @Override + protected AgentPropagation.ContextVisitor responseGetter() { + if (noopResponseGetter){ + return null + } + return new MapCarrierVisitor() + } - @Override - CharSequence spanName() { - return "http-test-span" - } + @Override + CharSequence spanName() { + return "http-test-span" + } - @Override - protected String method(Map m) { - return m.method - } + @Override + protected String method(Map m) { + return m.method + } - @Override - protected URIDataAdapter url(Map m) { - return m.url == null ? null : new URIDefaultDataAdapter(m.url) - } + @Override + protected URIDataAdapter url(Map m) { + return m.url == null ? null : new URIDefaultDataAdapter(m.url) + } - @Override - protected String peerHostIP(Map m) { - return m.peerIp - } + @Override + protected String peerHostIP(Map m) { + return m.peerIp + } - @Override - protected int peerPort(Map m) { - return m.port == null ? 0 : m.port - } + @Override + protected int peerPort(Map m) { + return m.port == null ? 0 : m.port + } - @Override - protected int status(Map m) { - return m.status == null ? 0 : m.status - } + @Override + protected int status(Map m) { + return m.status == null ? 0 : m.status + } - static class MapCarrierVisitor + static class MapCarrierVisitor implements AgentPropagation.ContextVisitor { - @Override - void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) { - Map headers = carrier.headers - if (headers != null) { - for (Map.Entry entry : headers.entrySet()) { - classifier.accept(entry.key, entry.value) + @Override + void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) { + Map headers = carrier.headers + if (headers != null) { + for (Map.Entry entry : headers.entrySet()) { + classifier.accept(entry.key, entry.value) + } } } } } - } } def "test startSpan and InstrumentationGateway"() { @@ -495,7 +498,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { getUniversalCallbackProvider() >> cbpAppSec // no iast callbacks, so this is equivalent getDataStreamsMonitoring() >> Mock(DataStreamsMonitoring) } - def decorator = newDecorator(mTracer) + def decorator = newDecorator(mTracer, true) when: decorator.startSpan("test", headers, null) 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 1fc0e8cbc4b..8674954e210 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -833,16 +833,12 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins logIgnoredSettingWarning(RESPONSE_HEADER_TAGS, HEADER_TAGS, ".legacy.parsing.enabled"); } } else { - System.out.println("requestHeaderTags: "); requestHeaderTags = configProvider.getMergedMapWithOptionalMappings( "http.request.headers.", true, HEADER_TAGS, REQUEST_HEADER_TAGS); - System.out.println("responseHeaderTags: "); responseHeaderTags = configProvider.getMergedMapWithOptionalMappings( "http.response.headers.", true, HEADER_TAGS, RESPONSE_HEADER_TAGS); - if(responseHeaderTags.size() > 0) - System.out.println("responseHeaderTags Post-Processing\nsize: " + responseHeaderTags.size() + "; " + responseHeaderTags.keySet().toArray()[0] + " : " + responseHeaderTags.get(responseHeaderTags.keySet().toArray()[0])); } requestHeaderTagsCommaAllowed = configProvider.getBoolean(REQUEST_HEADER_TAGS_COMMA_ALLOWED, true); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java index b25ee67b193..40736355887 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java @@ -312,20 +312,18 @@ private static void loadMapWithOptionalMapping( String value; if (delimiter == mapPos) { value = trimmedHeader(str, delimiter + 1, end, false); - System.out.println("delimiter==mapPos value: " + value); // tags must start with a letter if (!value.isEmpty() && !Character.isLetter(value.charAt(0))) { throw new BadFormatException( "Illegal tag starting with non letter for key '" + key + "'"); } } else { - if(key.charAt(0) == '*'){ + if (key.charAt(0) == '*') { map.put(key, defaultPrefix); return; } if (Character.isLetter(key.charAt(0))) { value = defaultPrefix + Strings.normalizedHeaderTag(key); - System.out.println("else value: " + value); } else { // tags must start with a letter throw new BadFormatException( diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 59437b63b2a..0a2715dd11a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -318,7 +318,6 @@ public Map getMergedMapWithOptionalMappings( for (String key : keys) { for (int i = sources.length - 1; 0 <= i; i--) { String value = sources[i].get(key); -// System.out.println("value: " + value); Map parsedMap = ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys); if (!parsedMap.isEmpty()) { diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 9913e150087..0149e2e636e 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -589,6 +589,7 @@ class ConfigTest extends DDSpecification { config.xDatadogTagsMaxLength == 42 config.isLongRunningTraceEnabled() config.getLongRunningTraceFlushInterval() == 81 + config.requestHeaderTags == ["*":"http.request.headers."] config.responseHeaderTags == ["*":"http.response.headers."] } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy index 14a0c2f6ae1..c82ab15ae8e 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy @@ -135,32 +135,35 @@ class ConfigConverterTest extends DDSpecification { def "test parseMapWithOptionalMappings"() { when: - def result = ConfigConverter.parseMapWithOptionalMappings(mapString, "test", "", lowercaseKeys) + def result = ConfigConverter.parseMapWithOptionalMappings(mapString, "test", defaultPrefix, lowercaseKeys) then: result == expected where: - mapString | expected | lowercaseKeys - "header1:one,header2:two" | [header1: "one", header2: "two"] | false - "header1:one, header2:two" | [header1: "one", header2: "two"] | false - "header1,header2:two" | [header1: "header1", header2: "two"] | false - "Header1:one,header2:two" | [header1: "one", header2: "two"] | true - "\"header1:one,header2:two\"" | ["\"header1": "one", header2: "two\""] | true - "header1" | [header1: "header1"] | true - ",header1:tag" | [header1: "tag"] | true - "header1:tag," | [header1: "tag"] | true - "header:tag:value" | [header: "tag:value"] | true - "" | [:] | true - null | [:] | true + mapString | expected | lowercaseKeys | defaultPrefix + "header1:one,header2:two" | [header1: "one", header2: "two"] | false | "" + "header1:one, header2:two" | [header1: "one", header2: "two"] | false | "" + "header1,header2:two" | [header1: "header1", header2: "two"] | false | "" + "Header1:one,header2:two" | [header1: "one", header2: "two"] | true | "" + "\"header1:one,header2:two\"" | ["\"header1": "one", header2: "two\""] | true | "" + "header1" | [header1: "header1"] | true | "" + ",header1:tag" | [header1: "tag"] | true | "" + "header1:tag," | [header1: "tag"] | true | "" + "header:tag:value" | [header: "tag:value"] | true | "" + "" | [:] | true | "" + null | [:] | true | "" + // Test for wildcard header tags + "*" | ["*":"datadog.response.headers."] | true | "datadog.response.headers" + "*:" | [:] | true | "datadog.response.headers" // logs warning: Illegal key only tag starting with non letter '1header' - "1header,header2:two" | [:] | true + "1header,header2:two" | [:] | true | "" // logs warning: Illegal tag starting with non letter for key 'header' - "header::tag" | [:] | true + "header::tag" | [:] | true | "" // logs warning: Illegal empty key at position 0 - ":tag" | [:] | true + ":tag" | [:] | true | "" // logs warning: Illegal empty key at position 11 - "header:tag,:tag" | [:] | true + "header:tag,:tag" | [:] | true | "" } } From 13d420fc40a99bdab799c9f2cc1977daf629a527 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Wed, 2 Jul 2025 11:03:33 -0500 Subject: [PATCH 5/5] responding PR comments --- .../decorator/HttpServerDecorator.java | 2 +- .../decorator/HttpServerDecoratorTest.groovy | 44 +++++++++---------- .../config/provider/ConfigConverter.java | 2 + .../provider/ConfigConverterTest.groovy | 3 +- 4 files changed, 25 insertions(+), 26 deletions(-) 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 c84eca2b56d..33e2ed2ebd8 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 @@ -647,7 +647,7 @@ static ResponseHeaderTagClassifier create(AgentSpan span, Map he public ResponseHeaderTagClassifier(AgentSpan span, Map headerTags) { this.span = span; this.headerTags = headerTags; - this.wildcardHeaderPrefix = this.headerTags.getOrDefault("*", null); + this.wildcardHeaderPrefix = this.headerTags.get("*"); } @Override diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy index a64c7751802..14261722ad1 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy @@ -34,7 +34,17 @@ import static datadog.trace.api.gateway.Events.EVENTS class HttpServerDecoratorTest extends ServerDecoratorTest { def span = Mock(AgentSpan) - def respHeaders = ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json'] + + static class MapCarrierVisitor + implements AgentPropagation.ContextVisitor { + @Override + void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) { + Map headers = carrier.headers + headers?.each { + classifier.accept(it.key, it.value) + } + } + } boolean origAppSecActive @@ -366,14 +376,16 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { return responseSpan } - def decorator = newDecorator(null, false) + def decorator = newDecorator(null, new MapCarrierVisitor()) when: decorator.onResponse(responseSpan, resp) then: - for (Map.Entry entry : expectedTag.entrySet()) { - assert tags[entry.getKey()] == entry.getValue() + if (expectedTag){ + expectedTag.each { + assert tags[it.key] == it.value + } } where: @@ -385,10 +397,10 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { @Override def newDecorator() { - return newDecorator(null, true) + return newDecorator(null, null) } - def newDecorator(TracerAPI tracer, boolean noopResponseGetter) { + def newDecorator(TracerAPI tracer, AgentPropagation.ContextVisitor contextVisitor) { if (!tracer) { tracer = AgentTracer.NOOP_TRACER } @@ -416,10 +428,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { @Override protected AgentPropagation.ContextVisitor responseGetter() { - if (noopResponseGetter){ - return null - } - return new MapCarrierVisitor() + return contextVisitor } @Override @@ -451,19 +460,6 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { protected int status(Map m) { return m.status == null ? 0 : m.status } - - static class MapCarrierVisitor - implements AgentPropagation.ContextVisitor { - @Override - void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) { - Map headers = carrier.headers - if (headers != null) { - for (Map.Entry entry : headers.entrySet()) { - classifier.accept(entry.key, entry.value) - } - } - } - } } } @@ -498,7 +494,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { getUniversalCallbackProvider() >> cbpAppSec // no iast callbacks, so this is equivalent getDataStreamsMonitoring() >> Mock(DataStreamsMonitoring) } - def decorator = newDecorator(mTracer, true) + def decorator = newDecorator(mTracer, null) when: decorator.startSpan("test", headers, null) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java index 40736355887..b081b704369 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java @@ -318,7 +318,9 @@ private static void loadMapWithOptionalMapping( "Illegal tag starting with non letter for key '" + key + "'"); } } else { + // If wildcard exists, we do not allow other header mappings if (key.charAt(0) == '*') { + map.clear(); map.put(key, defaultPrefix); return; } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy index c82ab15ae8e..a914aa0cc14 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy @@ -156,7 +156,8 @@ class ConfigConverterTest extends DDSpecification { // Test for wildcard header tags "*" | ["*":"datadog.response.headers."] | true | "datadog.response.headers" "*:" | [:] | true | "datadog.response.headers" - + "*,header1:tag" | ["*":"datadog.response.headers."] | true | "datadog.response.headers" + "header1:tag,*" | ["*":"datadog.response.headers."] | true | "datadog.response.headers" // logs warning: Illegal key only tag starting with non letter '1header' "1header,header2:two" | [:] | true | "" // logs warning: Illegal tag starting with non letter for key 'header'