8000 Enabling baggage cache to support limits and non-ascii characters by mhlidd · Pull Request #8713 · DataDog/dd-trace-java · GitHub
[go: up one dir, main page]
Skip to content

Enabling baggage cache to support limits and non-ascii characters #8713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 7, 2025
+141 −39
Merged

Enabling baggage cache to support limits and non-ascii characters #8713

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0ac63be
enabling baggage cache to support limits
mhlidd Apr 21, 2025
eb3ae15
updating limits to consider non-ascii characters
mhlidd Apr 22, 2025
03d4b1c
adding edge case
mhlidd Apr 25, 2025
e111266
PR comments
mhlidd Apr 29, 2025
576aab8
feat(core): Simplify baggage config usage
PerfectSlayer May 2, 2025
f9c53d2
feat(core): Add non ASCII character baggage extraction
PerfectSlayer May 2, 2025
a1e5ffb
Merge branch 'master' into mhlidd/update_baggage_limits
mhlidd May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
enabling baggage cache to support limits
  • Loading branch information
@mhlidd
mhlidd committed Apr 21, 2025
commit 0ac63be92d11eb0a3f0ae3c3b2d2d942cf46b2f1
27 changes: 23 additions & 4 deletions dd-trace-core/src/main/java/datadog/trace/core/baggage/BaggagePropagator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ public <C> Context extract(Context context, C carrier, CarrierVisitor<C> visitor
if (!this.extractBaggage || context == null || carrier == null || visitor == null) {
return context;
}
BaggageExtractor baggageExtractor = new BaggageExtractor();
BaggageExtractor baggageExtractor =
new BaggageExtractor(config.getTraceBaggageMaxItems(), config.getTraceBaggageMaxBytes());
visitor.forEachKeyValue(carrier, baggageExtractor);
return baggageExtractor.extracted == null ? context : context.with(baggageExtractor.extracted);
}
Expand All @@ -117,8 +118,15 @@ private static class BaggageExtractor implements BiConsumer<String, String> {
private static final char KEY_VALUE_SEPARATOR = '=';
private static final char PAIR_SEPARATOR = ',';
private Baggage extracted;

private BaggageExtractor() {}
private final int maxItems;
private final int maxBytes;
private String w3cHeader;

private BaggageExtractor(int maxItems, int maxBytes) {
this.maxItems = maxItems;
this.maxBytes = maxBytes;
this.w3cHeader = "";
}

/** URL decode value */
private String decode(final String value) {
Expand All @@ -134,6 +142,7 @@ private String decode(final String value) {
private Map<String, String> parseBaggageHeaders(String input) {
Map<String, String> baggage = new HashMap<>();
int start = 0;
boolean truncatedCache = false;
int pairSeparatorInd = input.indexOf(PAIR_SEPARATOR);
pairSeparatorInd = pairSeparatorInd == -1 ? input.length() : pairSeparatorInd;
int kvSeparatorInd = input.indexOf(KEY_VALUE_SEPARATOR);
Expand All @@ -152,11 +161,21 @@ private Map<String, String> parseBaggageHeaders(String input) {
}
baggage.put(key, value);

if (!truncatedCache && (end > this.maxBytes || baggage.size() > this.maxItems)) {
this.w3cHeader = input.substring(0, start - 1); // -1 to ignore the k/v separator
truncatedCache = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should break here if you reach max size or max bytes.
It will simply the if clause too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we don't cache here, we still need to maintain all of the raw baggage data so we can't break.

}

kvSeparatorInd = input.indexOf(KEY_VALUE_SEPARATOR, pairSeparatorInd + 1);
pairSeparatorInd = input.indexOf(PAIR_SEPARATOR, pairSeparatorInd + 1);
pairSeparatorInd = pairSeparatorInd == -1 ? input.length() : pairSeparatorInd;
start = end + 1;
}

if (!truncatedCache) {
this.w3cHeader = input;
}

return baggage;
}

Expand All @@ -166,7 +185,7 @@ public void accept(String key, String value) {
if (BAGGAGE_KEY.equalsIgnoreCase(key)) {
Map<String, String> baggage = parseBaggageHeaders(value);
if (!baggage.isEmpty()) {
this.extracted = Baggage.create(baggage, value);
this.extracted = Baggage.create(baggage, this.w3cHeader);
}
}
}
Expand Down
49 changes: 46 additions & 3 deletions dd-trace-core/src/test/groovy/datadog/trace/core/baggage/BaggagePropagatorTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class BaggagePropagatorTest extends DDSpecification {
["abcdefg": "hijklmnopq♥"] | "abcdefg=hijklmnopq%E2%99%A5"
}

def "test baggage item limit"() {
def "test baggage inject item limit"() {
setup:
injectSysConfig("trace.baggage.max.items", '2')
propagator = new BaggagePropagator(true, true) //creating a new instance after injecting config
Expand All @@ -79,7 +79,7 @@ class BaggagePropagatorTest extends DDSpecification {
[key1: "val1", key2: "val2", key3: "val3"] | "key1=val1,key2=val2"
}

def "test baggage bytes limit"() {
def "test baggage inject bytes limit"() {
setup:
injectSysConfig("trace.baggage.max.bytes", '20')
propagator = new BaggagePropagator(true, true) //creating a new instance after injecting config
Expand Down Expand Up @@ -139,7 +139,7 @@ class BaggagePropagatorTest extends DDSpecification {
"=" | _
}

def "testing baggage cache"(){
def "test baggage cache"(){
setup:
def headers = [
(BAGGAGE_KEY) : baggageHeader,
Expand All @@ -163,4 +163,47 @@ class BaggagePropagatorTest extends DDSpecification {
"key1=val1,key2=val2,foo=bar" | ["key1": "val1", "key2": "val2", "foo": "bar"]
"%22%2C%3B%5C%28%29%2F%3A%3C%3D%3E%3F%40%5B%5D%7B%7D=%22%2C%3B%5C" | ['",;\\()/:<=>?@[]{}': '",;\\']
}

def "test baggage cache items limit"(){
setup:
injectSysConfig("trace.baggage.max.items", "2")
propagator = new BaggagePropagator(true, true) //creating a new instance after injecting config
def headers = [
(BAGGAGE_KEY) : baggageHeader,
]

when:
context = this.propagator.extract(context, headers, ContextVisitors.stringValuesMap())

then:
Baggage baggageContext = Baggage.fromContext(context)
baggageContext.getW3cHeader() as String == cachedString

where:
baggageHeader | cachedString
"key1=val1,key2=val2" | "key1=val1,key2=val2"
"key1=val1,key2=val2,key3=val3" | "key1=val1,key2=val2"
"key1=val1,key2=val2,key3=val3,key4=val4" | "key1=val1,key2=val2"
}

def "test baggage cache bytes limit"(){
setup:
injectSysConfig("trace.baggage.max.bytes", "20")
propagator = new BaggagePropagator(true, true) //creating a new instance after injecting config
def headers = [
(BAGGAGE_KEY) : baggageHeader,
]

when:
context = this.propagator.extract(context, headers, ContextVisitors.stringValuesMap())

then:
Baggage baggageContext = Baggage.fromContext(context)
baggageContext.getW3cHeader() as String == cachedString

where:
baggageHeader | cachedString
"key1=val1,key2=val2" | "key1=val1,key2=val2"
"key1=val1,key2=val2,key3=val3" | "key1=val1,key2=val2"
}
}
0