Skip to content

Commit 6f4fc42

Browse files
mhliddmhlidd
authored
Adding wildcard feature for DD_TRACE_HEADER_TAGS and enabling for Http Response headers (#9067)
* Saving branch * setting up structure and adding initial unit test for header tags * fixing and adding unit tests * updating unit tests * responding PR comments
1 parent 99ecab7 commit 6f4fc42

File tree

5 files changed

+86
-22
lines changed

5 files changed

+86
-22
lines changed

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,14 +642,19 @@ static ResponseHeaderTagClassifier create(AgentSpan span, Map<String, String> he
642642

643643
private final AgentSpan span;
644644
private final Map<String, String> headerTags;
645+
private final String wildcardHeaderPrefix;
645646

646647
public ResponseHeaderTagClassifier(AgentSpan span, Map<String, String> headerTags) {
647648
this.span = span;
648649
this.headerTags = headerTags;
650+
this.wildcardHeaderPrefix = this.headerTags.get("*");
649651
}
650652

651653
@Override
652654
public boolean accept(String key, String value) {
655+
if (wildcardHeaderPrefix != null) {
656+
span.setTag((wildcardHeaderPrefix + key).toLowerCase(Locale.ROOT), value);
657+
}
653658
String mappedKey = headerTags.get(key.toLowerCase(Locale.ROOT));
654659
if (mappedKey != null) {
655660
span.setTag(mappedKey, value);

‎dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import datadog.trace.api.gateway.Flow
77
import datadog.trace.api.gateway.InstrumentationGateway
88
import datadog.trace.api.gateway.RequestContext
99
import datadog.trace.api.gateway.RequestContextSlot
10+
import datadog.trace.api.TraceConfig
1011
import datadog.trace.bootstrap.ActiveSubsystems
1112
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation
1213
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
@@ -34,6 +35,17 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
3435

3536
def span = Mock(AgentSpan)
3637

38+
static class MapCarrierVisitor
39+
implements AgentPropagation.ContextVisitor<Map> {
40+
@Override
41+
void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) {
42+
Map<String, String> headers = carrier.headers
43+
headers?.each {
44+
classifier.accept(it.key, it.value)
45+
}
46+
}
47+
}
48+
3749
boolean origAppSecActive
3850

3951
void setup() {
@@ -350,12 +362,45 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
350362
null | null | false
351363
}
352364

365+
def "test response headers with trace.header.tags"() {
366+
setup:
367+
def traceConfig = Mock(TraceConfig)
368+
traceConfig.getResponseHeaderTags() >> headerTags
369+
370+
def tags = [:]
371+
372+
def responseSpan = Mock(AgentSpan)
373+
responseSpan.traceConfig() >> traceConfig
374+
responseSpan.setTag(_, _) >> { String k, String v ->
375+
tags[k] = v
376+
return responseSpan
377+
}
378+
379+
def decorator = newDecorator(null, new MapCarrierVisitor())
380+
381+
when:
382+
decorator.onResponse(responseSpan, resp)
383+
384+
then:
385+
if (expectedTag){
386+
expectedTag.each {
387+
assert tags[it.key] == it.value
388+
}
389+
}
390+
391+
where:
392+
headerTags | resp | expectedTag
393+
[:] | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | [:]
394+
["x-custom-header": "abc"] | [status: 200, headers: ['X-Custom-Header': 'custom-value', 'Content-Type': 'application/json']] | [abc:"custom-value"]
395+
["*": "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"]
396+
}
397+
353398
@Override
354399
def newDecorator() {
355-
return newDecorator(null)
400+
return newDecorator(null, null)
356401
}
357402

358-
def newDecorator(TracerAPI tracer) {
403+
def newDecorator(TracerAPI tracer, AgentPropagation.ContextVisitor<Map> contextVisitor) {
359404
if (!tracer) {
360405
tracer = AgentTracer.NOOP_TRACER
361406
}
@@ -383,7 +428,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
383428

384429
@Override
385430
protected AgentPropagation.ContextVisitor<Map> responseGetter() {
386-
return null
431+
return contextVisitor
387432
}
388433

389434
@Override
@@ -449,7 +494,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
449494
getUniversalCallbackProvider() >> cbpAppSec // no iast callbacks, so this is equivalent
450495
getDataStreamsMonitoring() >> Mock(DataStreamsMonitoring)
451496
}
452-
def decorator = newDecorator(mTracer)
497+
def decorator = newDecorator(mTracer, null)
453498

454499
when:
455500
decorator.startSpan("test", headers, null)

‎internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,12 @@ private static void loadMapWithOptionalMapping(
318318
"Illegal tag starting with non letter for key '" + key + "'");
319319
}
320320
} else {
321+
// If wildcard exists, we do not allow other header mappings
322+
if (key.charAt(0) == '*') {
323+
map.clear();
324+
map.put(key, defaultPrefix);
325+
return;
326+
}
321327
if (Character.isLetter(key.charAt(0))) {
322328
value = defaultPrefix + Strings.normalizedHeaderTag(key);
323329
} else {

‎internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ class ConfigTest extends DDSpecification {
165165
private static final DD_JMXFETCH_METRICS_CONFIGS_ENV = "DD_JMXFETCH_METRICS_CONFIGS"
166166
private static final DD_TRACE_AGENT_PORT_ENV = "DD_TRACE_AGENT_PORT"
167167
private static final DD_AGENT_PORT_LEGACY_ENV = "DD_AGENT_PORT"
168+
private static final DD_TRACE_HEADER_TAGS = "DD_TRACE_HEADER_TAGS"
168169
private static final DD_TRACE_REPORT_HOSTNAME = "DD_TRACE_REPORT_HOSTNAME"
169170
private static final DD_RUNTIME_METRICS_ENABLED_ENV = "DD_RUNTIME_METRICS_ENABLED"
170171
private static final DD_TRACE_LONG_RUNNING_ENABLED = "DD_TRACE_EXPERIMENTAL_LONG_RUNNING_ENABLED"
@@ -568,6 +569,7 @@ class ConfigTest extends DDSpecification {
568569
environmentVariables.set(DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH, "42")
569570
environmentVariables.set(DD_TRACE_LONG_RUNNING_ENABLED, "true")
570571
environmentVariables.set(DD_TRACE_LONG_RUNNING_FLUSH_INTERVAL, "81")
572+
environmentVariables.set(DD_TRACE_HEADER_TAGS, "*")
571573

572574
when:
573575
def config = new Config()
@@ -587,6 +589,8 @@ class ConfigTest extends DDSpecification {
587589
config.xDatadogTagsMaxLength == 42
588590
config.isLongRunningTraceEnabled()
589591
config.getLongRunningTraceFlushInterval() == 81
592+
config.requestHeaderTags == ["*":"http.request.headers."]
593+
config.responseHeaderTags == ["*":"http.response.headers."]
590594
}
591595

592596
def "sys props override env vars"() {

‎internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -135,32 +135,36 @@ class ConfigConverterTest extends DDSpecification {
135135

136136
def "test parseMapWithOptionalMappings"() {
137137
when:
138-
def result = ConfigConverter.parseMapWithOptionalMappings(mapString, "test", "", lowercaseKeys)
138+
def result = ConfigConverter.parseMapWithOptionalMappings(mapString, "test", defaultPrefix, lowercaseKeys)
139139

140140
then:
141141
result == expected
142142

143143
where:
144-
mapString | expected | lowercaseKeys
145-
"header1:one,header2:two" | [header1: "one", header2: "two"] | false
146-
"header1:one, header2:two" | [header1: "one", header2: "two"] | false
147-
"header1,header2:two" | [header1: "header1", header2: "two"] | false
148-
"Header1:one,header2:two" | [header1: "one", header2: "two"] | true
149-
"\"header1:one,header2:two\"" | ["\"header1": "one", header2: "two\""] | true
150-
"header1" | [header1: "header1"] | true
151-
",header1:tag" | [header1: "tag"] | true
152-
"header1:tag," | [header1: "tag"] | true
153-
"header:tag:value" | [header: "tag:value"] | true
154-
"" | [:] | true
155-
null | [:] | true
156-
144+
mapString | expected | lowercaseKeys | defaultPrefix
145+
"header1:one,header2:two" | [header1: "one", header2: "two"] | false | ""
146+
"header1:one, header2:two" | [header1: "one", header2: "two"] | false | ""
147+
"header1,header2:two" | [header1: "header1", header2: "two"] | false | ""
148+
"Header1:one,header2:two" | [header1: "one", header2: "two"] | true | ""
149+
"\"header1:one,header2:two\"" | ["\"header1": "one", header2: "two\""] | true | ""
150+
"header1" | [header1: "header1"] | true | ""
151+
",header1:tag" | [header1: "tag"] | true | ""
152+
"header1:tag," | [header1: "tag"] | true | ""
153+
"header:tag:value" | [header: "tag:value"] | true | ""
154+
"" | [:] | true | ""
155+
null | [:] | true | ""
156+
// Test for wildcard header tags
157+
"*" | ["*":"datadog.response.headers."] | true | "datadog.response.headers"
158+
"*:" | [:] | true | "datadog.response.headers"
159+
"*,header1:tag" | ["*":"datadog.response.headers."] | true | "datadog.response.headers"
160+
"header1:tag,*" | ["*":"datadog.response.headers."] | true | "datadog.response.headers"
157161
// logs warning: Illegal key only tag starting with non letter '1header'
158-
"1header,header2:two" | [:] | true
162+
"1header,header2:two" | [:] | true | ""
159163
// logs warning: Illegal tag starting with non letter for key 'header'
160-
"header::tag" | [:] | true
164+
"header::tag" | [:] | true | ""
161165
// logs warning: Illegal empty key at position 0
162-
":tag" | [:] | true
166+
":tag" | [:] | true | ""
163167
// logs warning: Illegal empty key at position 11
164-
"header:tag,:tag" | [:] | true
168+
"header:tag,:tag" | [:] | true | ""
165169
}
166170
}

0 commit comments

Comments
 (0)