Skip to content

Commit 822ffc0

Browse files
mccullsmcculls
authored
Apply default tag name when setting header tags via dynamic-config (#5557)
* Rename method to clarify this is for HTTP request tags * Allow use of more complex dynamic-config key/value mappers * Apply default tag name when setting header tags via dynamic-config * Reset test tracer's dynamic-config when new config is injected * Move normalizedHeaderTag to allow re-use and verify normalization is applied when generating default header tag names * Test custom header tag name is not normalized
1 parent 3821f5d commit 822ffc0

File tree

9 files changed

+159
-81
lines changed

9 files changed

+159
-81
lines changed

‎dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.groovy‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,12 @@ abstract class AgentTestRunner extends DDSpecification implements AgentBuilder.L
311311
}
312312
}
313313

314+
@Override
315+
void rebuildConfig() {
316+
super.rebuildConfig()
317+
TEST_TRACER?.rebuildTraceConfig(Config.get())
318+
}
319+
314320
void cleanup() {
315321
if (isTestAgentEnabled()) {
316322
// save Datadog environment to DDAgentWriter header

‎dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,21 @@ private CoreTracer(
662662
this.profilingContextIntegration = profilingContextIntegration;
663663
}
664664

665+
/** Used by AgentTestRunner to inject configuration into the test tracer. */
666+
public void rebuildTraceConfig(Config config) {
667+
dynamicConfig
668+
.initial()
669+
.setDebugEnabled(config.isDebugEnabled())
670+
.setRuntimeMetricsEnabled(config.isRuntimeMetricsEnabled())
671+
.setLogsInjectionEnabled(config.isLogsInjectionEnabled())
672+
.setDataStreamsEnabled(config.isDataStreamsEnabled())
673+
.setServiceMapping(config.getServiceMapping())
674+
.setHeaderTags(config.getRequestHeaderTags())
675+
.setBaggageMapping(config.getBaggageMapping())
676+
.setTraceSampleRate(config.getTraceSampleRate())
677+
.apply();
678+
}
679+
665680
@Override
666681
protected void finalize() {
667682
try {

‎dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public ContextInterpreter reset(TraceConfig traceConfig) {
221221
collectIpHeaders =
222222
this.clientIpWithoutAppSec
223223
|| this.clientIpResolutionEnabled && ActiveSubsystems.APPSEC_ACTIVE;
224-
headerTags = traceConfig.getHeaderTags();
224+
headerTags = traceConfig.getRequestHeaderTags();
225225
baggageMapping = traceConfig.getBaggageMapping();
226226
return this;
227227
}

‎dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ class CoreTracerTest extends DDCoreSpecification {
175175
when:
176176
def tracer = tracerBuilder().build()
177177
// Datadog extractor gets placed first
178-
def taggedHeaders = tracer.extractor.extractors[0].traceConfigSupplier.get().headerTags
178+
def taggedHeaders = tracer.extractor.extractors[0].traceConfigSupplier.get().requestHeaderTags
179179

180180
then:
181181
tracer.defaultSpanTags == map
@@ -428,7 +428,8 @@ class CoreTracerTest extends DDCoreSpecification {
428428
}
429429
and:
430430
tracer.captureTraceConfig().serviceMapping == [:]
431-
tracer.captureTraceConfig().headerTags == [:]
431+
tracer.captureTraceConfig().requestHeaderTags == [:]
432+
tracer.captureTraceConfig().responseHeaderTags == [:]
432433
tracer.captureTraceConfig().traceSampleRate == null
433434

434435
when:
@@ -447,11 +448,20 @@ class CoreTracerTest extends DDCoreSpecification {
447448
,
448449
"tracing_header_tags":
449450
[{
450-
"header": "User-Agent",
451-
"tag_name": "http.user_agent"
451+
"header": "Cookie",
452+
"tag_name": ""
452453
}, {
453454
"header": "Referer",
454455
"tag_name": "http.referer"
456+
}, {
457+
"header": " Some.Header ",
458+
"tag_name": ""
459+
}, {
460+
"header": "C!!!ont_____ent----tYp!/!e",
461+
"tag_name": ""
462+
}, {
463+
"header": "this.header",
464+
"tag_name": "whatever.the.user.wants.this.header"
455465
}]
456466
,
457467
"tracing_sampling_rate": 0.5
@@ -462,7 +472,20 @@ class CoreTracerTest extends DDCoreSpecification {
462472

463473
then:
464474
tracer.captureTraceConfig().serviceMapping == ['foobar':'bar', 'snafu':'foo']
465-
tracer.captureTraceConfig().headerTags == ['user-agent':'http.user_agent', 'referer':'http.referer']
475+
tracer.captureTraceConfig().requestHeaderTags == [
476+
'cookie':'http.request.headers.cookie',
477+
'referer':'http.referer',
478+
'some.header':'http.request.headers.some_header',
479+
'c!!!ont_____ent----typ!/!e':'http.request.headers.c___ont_____ent----typ_/_e',
480+
'this.header':'whatever.the.user.wants.this.header'
481+
]
482+
tracer.captureTraceConfig().responseHeaderTags == [
483+
'cookie':'http.response.headers.cookie',
484+
'referer':'http.referer',
485+
'some.header':'http.response.headers.some_header',
486+
'c!!!ont_____ent----typ!/!e':'http.response.headers.c___ont_____ent----typ_/_e',
487+
'this.header':'whatever.the.user.wants.this.header'
488+
]
466489
tracer.captureTraceConfig().traceSampleRate == 0.5
467490

468491
when:
@@ -471,7 +494,8 @@ class CoreTracerTest extends DDCoreSpecification {
471494

472495
then:
473496
tracer.captureTraceConfig().serviceMapping == [:]
474-
tracer.captureTraceConfig().headerTags == [:]
497+
tracer.captureTraceConfig().requestHeaderTags == [:]
498+
tracer.captureTraceConfig().responseHeaderTags == [:]
475499
tracer.captureTraceConfig().traceSampleRate == null
476500

477501
cleanup:

‎internal-api/src/main/java/datadog/trace/api/DynamicConfig.java‎

Lines changed: 66 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,31 @@
55
import static datadog.trace.api.config.GeneralConfig.TRACE_DEBUG;
66
import static datadog.trace.api.config.TraceInstrumentationConfig.LOGS_INJECTION_ENABLED;
77
import static datadog.trace.api.config.TracerConfig.BAGGAGE_MAPPING;
8-
import static datadog.trace.api.config.TracerConfig.HEADER_TAGS;
8+
import static datadog.trace.api.config.TracerConfig.REQUEST_HEADER_TAGS;
9+
import static datadog.trace.api.config.TracerConfig.RESPONSE_HEADER_TAGS;
910
import static datadog.trace.api.config.TracerConfig.SERVICE_MAPPING;
1011
import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLE_RATE;
1112
import static datadog.trace.util.CollectionUtils.tryMakeImmutableMap;
1213

14+
import datadog.trace.util.Strings;
1315
import java.util.Collection;
1416
import java.util.Collections;
1517
import java.util.HashMap;
1618
import java.util.Locale;
1719
import java.util.Map;
1820
import java.util.function.BiFunction;
21+
import java.util.function.Function;
1922

2023
/** Manages dynamic configuration for a particular {@link Tracer} instance. */
2124
public final class DynamicConfig<S extends DynamicConfig.Snapshot> {
25+
26+
static final Function<Map.Entry<String, String>, String> KEY = DynamicConfig::key;
27+
static final Function<Map.Entry<String, String>, String> VALUE = DynamicConfig::value;
28+
static final Function<Map.Entry<String, String>, String> LOWER_KEY = DynamicConfig::lowerKey;
29+
static final Function<Map.Entry<String, String>, String> REQUEST_TAG = DynamicConfig::requestTag;
30+
static final Function<Map.Entry<String, String>, String> RESPONSE_TAG =
31+
DynamicConfig::responseTag;
32+
2233
BiFunction<Builder, S, S> snapshotFactory;
2334

2435
S initialSnapshot;
@@ -68,17 +79,17 @@ public final class Builder {
6879
boolean dataStreamsEnabled;
6980

7081
Map<String, String> serviceMapping;
71-
Map<String, String> headerTags;
82+
Map<String, String> requestHeaderTags;
83+
Map<String, String> responseHeaderTags;
7284
Map<String, String> baggageMapping;
7385

74-
boolean overrideResponseTags;
75-
7686
Double traceSampleRate;
7787

7888
Builder(Snapshot snapshot) {
7989
if (null == snapshot) {
8090
this.serviceMapping = Collections.emptyMap();
81-
this.headerTags = Collections.emptyMap();
91+
this.requestHeaderTags = Collections.emptyMap();
92+
this.responseHeaderTags = Collections.emptyMap();
8293
this.baggageMapping = Collections.emptyMap();
8394
this.logsInjectionEnabled = ConfigDefaults.DEFAULT_LOGS_INJECTION_ENABLED;
8495
} else {
@@ -89,11 +100,10 @@ public final class Builder {
89100
this.dataStreamsEnabled = snapshot.dataStreamsEnabled;
90101

91102
this.serviceMapping = snapshot.serviceMapping;
92-
this.headerTags = snapshot.headerTags;
103+
this.requestHeaderTags = snapshot.requestHeaderTags;
104+
this.responseHeaderTags = snapshot.responseHeaderTags;
93105
this.baggageMapping = snapshot.baggageMapping;
94106

95-
this.overrideResponseTags = snapshot.overrideResponseTags;
96-
97107
this.traceSampleRate = snapshot.traceSampleRate;
98108
}
99109
}
@@ -126,8 +136,8 @@ public Builder setHeaderTags(Map<String, String> headerTags) {
126136
if (Config.get().getRequestHeaderTags().equals(headerTags)
127137
&& !Config.get().getResponseHeaderTags().equals(headerTags)) {
128138
// using static config; don't override separate static config for response header tags
129-
this.headerTags = cleanMapping(headerTags.entrySet(), true, true);
130-
this.overrideResponseTags = false;
139+
this.requestHeaderTags = Config.get().getRequestHeaderTags();
140+
this.responseHeaderTags = Config.get().getResponseHeaderTags();
131141
return this;
132142
} else {
133143
return setHeaderTags(headerTags.entrySet());
@@ -140,19 +150,19 @@ public Builder setBaggageMapping(Map<String, String> baggageMapping) {
140150

141151
public Builder setServiceMapping(
142152
Collection<? extends Map.Entry<String, String>> serviceMapping) {
143-
this.serviceMapping = cleanMapping(serviceMapping, false, false);
153+
this.serviceMapping = cleanMapping(serviceMapping, KEY, VALUE);
144154
return this;
145155
}
146156

147157
public Builder setHeaderTags(Collection<? extends Map.Entry<String, String>> headerTags) {
148-
this.headerTags = cleanMapping(headerTags, true, true);
149-
this.overrideResponseTags = true;
158+
this.requestHeaderTags = cleanMapping(headerTags, LOWER_KEY, REQUEST_TAG);
159+
this.responseHeaderTags = cleanMapping(headerTags, LOWER_KEY, RESPONSE_TAG);
150160
return this;
151161
}
152162

153163
public Builder setBaggageMapping(
154164
Collection<? extends Map.Entry<String, String>> baggageMapping) {
155-
this.baggageMapping = cleanMapping(baggageMapping, true, false);
165+
this.baggageMapping = cleanMapping(baggageMapping, LOWER_KEY, VALUE);
156166
return this;
157167
}
158168

@@ -163,19 +173,11 @@ public Builder setTraceSampleRate(Double traceSampleRate) {
163173

164174
private Map<String, String> cleanMapping(
165175
Collection<? extends Map.Entry<String, String>> mapping,
166-
boolean lowerCaseKeys,
167-
boolean lowerCaseValues) {
176+
Function<Map.Entry<String, String>, String> keyMapper,
177+
Function<Map.Entry<String, String>, String> valueMapper) {
168178
final Map<String, String> cleanedMapping = new HashMap<>(mapping.size() * 4 / 3);
169179
for (Map.Entry<String, String> association : mapping) {
170-
String key = association.getKey().trim();
171-
if (lowerCaseKeys) {
172-
key = key.toLowerCase(Locale.ROOT);
173-
}
174-
String value = association.getValue().trim();
175-
if (lowerCaseValues) {
176-
value = value.toLowerCase(Locale.ROOT);
177-
}
178-
cleanedMapping.put(key, value);
180+
cleanedMapping.put(keyMapper.apply(association), valueMapper.apply(association));
179181
}
180182
return tryMakeImmutableMap(cleanedMapping);
181183
}
@@ -195,6 +197,36 @@ public DynamicConfig<S> apply() {
195197
}
196198
}
197199

200+
static String key(Map.Entry<String, String> association) {
201+
return Strings.trim(association.getKey());
202+
}
203+
204+
static String value(Map.Entry<String, String> association) {
205+
return Strings.trim(association.getValue());
206+
}
207+
208+
static String lowerKey(Map.Entry<String, String> association) {
209+
return key(association).toLowerCase(Locale.ROOT);
210+
}
211+
212+
static String requestTag(Map.Entry<String, String> association) {
213+
String requestTag = value(association);
214+
if (requestTag.isEmpty()) {
215+
// normalization is only applied when generating default tag names; see ConfigConverter
216+
requestTag = "http.request.headers." + Strings.normalizedHeaderTag(association.getKey());
217+
}
218+
return requestTag;
219+
}
220+
221+
static String responseTag(Map.Entry<String, String> association) {
222+
String responseTag = value(association);
223+
if (responseTag.isEmpty()) {
224+
// normalization is only applied when generating default tag names; see ConfigConverter
225+
responseTag = "http.response.headers." + Strings.normalizedHeaderTag(association.getKey());
226+
}
227+
return responseTag;
228+
}
229+
198230
static void reportConfigChange(Snapshot newSnapshot) {
199231
Map<String, Object> update = new HashMap<>();
200232

@@ -204,7 +236,8 @@ static void reportConfigChange(Snapshot newSnapshot) {
204236
update.put(DATA_STREAMS_ENABLED, newSnapshot.dataStreamsEnabled);
205237

206238
update.put(SERVICE_MAPPING, newSnapshot.serviceMapping);
207-
update.put(HEADER_TAGS, newSnapshot.headerTags);
239+
update.put(REQUEST_HEADER_TAGS, newSnapshot.requestHeaderTags);
240+
update.put(RESPONSE_HEADER_TAGS, newSnapshot.responseHeaderTags);
208241
update.put(BAGGAGE_MAPPING, newSnapshot.baggageMapping);
209242

210243
maybePut(update, TRACE_SAMPLE_RATE, newSnapshot.traceSampleRate);
@@ -227,11 +260,10 @@ public static class Snapshot implements TraceConfig {
227260
final boolean dataStreamsEnabled;
228261

229262
final Map<String, String> serviceMapping;
230-
final Map<String, String> headerTags;
263+
final Map<String, String> requestHeaderTags;
264+
final Map<String, String> responseHeaderTags;
231265
final Map<String, String> baggageMapping;
232266

233-
final boolean overrideResponseTags;
234-
235267
final Double traceSampleRate;
236268

237269
protected Snapshot(DynamicConfig<?>.Builder builder, Snapshot oldSnapshot) {
@@ -242,11 +274,10 @@ protected Snapshot(DynamicConfig<?>.Builder builder, Snapshot oldSnapshot) {
242274
this.dataStreamsEnabled = builder.dataStreamsEnabled;
243275

244276
this.serviceMapping = builder.serviceMapping;
245-
this.headerTags = builder.headerTags;
277+
this.requestHeaderTags = builder.requestHeaderTags;
278+
this.responseHeaderTags = builder.responseHeaderTags;
246279
this.baggageMapping = builder.baggageMapping;
247280

248-
this.overrideResponseTags = builder.overrideResponseTags;
249-
250281
this.traceSampleRate = builder.traceSampleRate;
251282
}
252283

@@ -276,13 +307,13 @@ public Map<String, String> getServiceMapping() {
276307
}
277308

278309
@Override
279-
public Map<String, String> getHeaderTags() {
280-
return headerTags;
310+
public Map<String, String> getRequestHeaderTags() {
311+
return requestHeaderTags;
281312
}
282313

283314
@Override
284315
public Map<String, String> getResponseHeaderTags() {
285-
return overrideResponseTags ? headerTags : Config.get().getResponseHeaderTags();
316+
return responseHeaderTags;
286317
}
287318

288319
@Override

‎internal-api/src/main/java/datadog/trace/api/TraceConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public interface TraceConfig {
1515

1616
Map<String, String> getServiceMapping();
1717

18-
Map<String, String> getHeaderTags();
18+
Map<String, String> getRequestHeaderTags();
1919

2020
Map<String, String> getResponseHeaderTags();
2121

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

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ private static void loadMapWithOptionalMapping(
249249
}
250250
} else {
251251
if (Character.isLetter(key.charAt(0))) {
252-
value = defaultPrefix + normalizedHeaderTag(key);
252+
value = defaultPrefix + Strings.normalizedHeaderTag(key);
253253
} else {
254254
// tags must start with a letter
255255
throw new BadFormatException(
@@ -302,36 +302,6 @@ private static String trimmedHeader(String str, int start, int end, boolean lowe
302302
}
303303
}
304304

305-
@Nonnull
306-
private static String normalizedHeaderTag(String str) {
307-
if (str.isEmpty()) {
308-
return "";
309-
}
310-
StringBuilder builder = new StringBuilder(str.length());
311-
int firstNonWhiteSpace = -1;
312-
int lastNonWhitespace = -1;
313-
for (int i = 0; i < str.length(); i++) {
314-
char c = str.charAt(i);
315-
if (Character.isWhitespace(c)) {
316-
builder.append('_');
317-
} else {
318-
firstNonWhiteSpace = firstNonWhiteSpace == -1 ? i : firstNonWhiteSpace;
319-
lastNonWhitespace = i;
320-
if (Character.isLetterOrDigit(c) || c == '_' || c == '-' || c == '/') {
321-
builder.append(Character.toLowerCase(c));
322-
} else {
323-
builder.append('_');
324-
}
325-
}
326-
}
327-
if (firstNonWhiteSpace == -1) {
328-
return "";
329-
} else {
330-
str = builder.substring(firstNonWhiteSpace, lastNonWhitespace + 1);
331-
return str;
332-
}
333-
}
334-
335305
@Nonnull
336306
@SuppressForbidden
337307
static BitSet parseIntegerRangeSet(@Nonnull String str, final String settingName)

0 commit comments

Comments
 (0)