Skip to content
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.

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

Implementation of BaggagePropagator and BaggageContext #8330

Merged
merged 34 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0192caf
initial implementation and unit tests for inject/extract
mhlidd Feb 3, 2025
fd89905
adding break to switch statement[C
mhlidd Feb 3, 2025
fd19e75
better check for max bytes
mhlidd Feb 4, 2025
c6d3a0e
updating key encoder and adding test case
mhlidd Feb 5, 2025
7aa0d4b
cleanup
mhlidd Feb 5, 2025
2ef1887
updating encoder and byte checks
mhlidd Feb 6, 2025
1987e9d
cleanup
mhlidd Feb 6, 2025
11d1350
introducing baggagecontext
mhlidd Feb 7, 2025
64b1303
initial migration to contextAPI
mhlidd Feb 9, 2025
73e0384
updating tests for inject/extract
mhlidd Feb 10, 2025
5ef9357
finishing migration to baggage propagator
mhlidd Feb 10, 2025
b53e449
adding support for injector only or extractor only
mhlidd Feb 11, 2025
22d8a31
updating PR comments
mhlidd Feb 11, 2025
a73bc87
cleanup
mhlidd Feb 11, 2025
b37dcbd
adding null context check
mhlidd Feb 11, 2025
36940e8
updating PR comments
mhlidd Feb 13, 2025
188e82b
updating tests
mhlidd Feb 13, 2025
58234e4
adding baggagecache
mhlidd Feb 19, 2025
660529d
updating baggagecache
mhlidd Feb 19, 2025
a09abe1
cleanup
mhlidd Feb 19, 2025
2140924
Merge branch 'master' into mhlidd/otel_baggage_extract/inject
mhlidd Feb 24, 2025
a3dda56
updating pr comments
mhlidd Feb 24, 2025
bfe69fc
updating PR comments and fixing CI
mhlidd Feb 25, 2025
a0df83a
adding OTEL PercentEncoder and utilizing it in BaggagePropagator
mhlidd Feb 27, 2025
831b564
nit fixes
mhlidd Feb 27, 2025
e0a9709
hiding unsafeKey and unsafeValue
mhlidd Feb 27, 2025
b2731e8
change from atomicinteger to int[]
mhlidd Feb 27, 2025
e7f533d
creating EscapedData class to store escaped data
mhlidd Feb 28, 2025
61803e5
Merge branch 'master' into mhlidd/otel_baggage_extract/inject
mhlidd Feb 28, 2025
9916590
nit comments and merge conflicts
mhlidd Feb 28, 2025
586468c
spotless
mhlidd Feb 28, 2025
21d2a43
Merge branch 'master' into mhlidd/otel_baggage_extract/inject
mhlidd Mar 3, 2025
bebdf7c
updating build file
mhlidd Mar 3, 2025
c4f6d7d
nit update
mhlidd Mar 3, 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
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.api;

import static datadog.trace.api.TracePropagationStyle.BAGGAGE;
import static datadog.trace.api.TracePropagationStyle.DATADOG;
import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT;
import static java.util.Arrays.asList;
Expand Down Expand Up @@ -78,9 +79,11 @@ public final class ConfigDefaults {
static final int DEFAULT_PARTIAL_FLUSH_MIN_SPANS = 1000;
static final boolean DEFAULT_PROPAGATION_EXTRACT_LOG_HEADER_NAMES_ENABLED = false;
static final Set<TracePropagationStyle> DEFAULT_TRACE_PROPAGATION_STYLE =
new LinkedHashSet<>(asList(DATADOG, TRACECONTEXT));
new LinkedHashSet<>(asList(DATADOG, TRACECONTEXT, BAGGAGE));
static final Set<PropagationStyle> DEFAULT_PROPAGATION_STYLE =
new LinkedHashSet<>(asList(PropagationStyle.DATADOG));
static final int DEFAULT_TRACE_BAGGAGE_MAX_ITEMS = 64;
static final int DEFAULT_TRACE_BAGGAGE_MAX_BYTES = 8192;
static final boolean DEFAULT_JMX_FETCH_ENABLED = true;
static final boolean DEFAULT_TRACE_AGENT_V05_ENABLED = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ public enum TracePropagationStyle {
// W3C trace context propagation style
// https://www.w3.org/TR/trace-context-1/
TRACECONTEXT,
// W3C baggage support
// https://www.w3.org/TR/baggage/
BAGGAGE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to skip this value from HttpCoded createExtractor and createInjector

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neither inject or extract currently have a case handling Baggage!

Copy link
Contributor

Choose a reason for hiding this comment

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

The goal is to avoid emitting the following log entries for Baggage propagation style:

  • "No implementation found to inject propagation style: {}"
  • "No implementation found to extract propagation style: {}"

It will be misleading during escalation 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah that makes sense. I added a case to just break and not add anything!

// None does not extract or inject
NONE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ public final class TracerConfig {
public static final String TRACE_PROPAGATION_STYLE_EXTRACT = "trace.propagation.style.extract";
public static final String TRACE_PROPAGATION_STYLE_INJECT = "trace.propagation.style.inject";
public static final String TRACE_PROPAGATION_EXTRACT_FIRST = "trace.propagation.extract.first";
public static final String TRACE_BAGGAGE_MAX_ITEMS = "trace.baggage.max.items";
public static final String TRACE_BAGGAGE_MAX_BYTES = "trace.baggage.max.bytes";

public static final String ENABLE_TRACE_AGENT_V05 = "trace.agent.v0.5.enabled";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static datadog.trace.api.DDTags.DJM_ENABLED;
import static datadog.trace.api.DDTags.DSM_ENABLED;
import static datadog.trace.api.DDTags.PROFILING_CONTEXT_ENGINE;
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.BAGGAGE_CONCERN;
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.DSM_CONCERN;
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.TRACING_CONCERN;
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.XRAY_TRACING_CONCERN;
Expand Down Expand Up @@ -77,6 +78,7 @@
import datadog.trace.common.writer.WriterFactory;
import datadog.trace.common.writer.ddintake.DDIntakeTraceInterceptor;
import datadog.trace.context.TraceScope;
import datadog.trace.core.baggage.BaggagePropagator;
import datadog.trace.core.datastreams.DataStreamsMonitoring;
import datadog.trace.core.datastreams.DefaultDataStreamsMonitoring;
import datadog.trace.core.flare.TracerFlarePoller;
Expand Down Expand Up @@ -719,6 +721,9 @@ private CoreTracer(
if (config.isDataStreamsEnabled()) {
Propagators.register(DSM_CONCERN, this.dataStreamsMonitoring.propagator());
}
if (config.isBaggagePropagationEnabled()) {
Propagators.register(BAGGAGE_CONCERN, new BaggagePropagator(config));
}
Comment on lines +724 to +726
Copy link
Contributor

Choose a reason for hiding this comment

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

Open for discussion:
I really wonder if we should not register is as non default nonetheless.
If an instrumentation requests to extract baggage (Propagator.forConcern(BAGGAGE_CONCERN)), should we return a noop propagator or the baggage propagator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good question. The way I see it is that baggage is a feature for Datadog users that want to send more information downstream. IMO the users should have full say on whether they want baggage to be enabled, and instrumentation baggage is just a supporting cast. Thus I think we can return a noop propagator in those situations. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the noop would be the right thing to do. Mostly wondering if it will conflict with OTel support or not.
We can revisit it later 😉


this.tagInterceptor =
null == tagInterceptor ? new TagInterceptor(new RuleFlags(config)) : tagInterceptor;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package datadog.trace.core.baggage;

import datadog.context.Context;
import datadog.context.propagation.CarrierSetter;
import datadog.context.propagation.CarrierVisitor;
import datadog.context.propagation.Propagator;
import datadog.trace.api.Config;
import datadog.trace.bootstrap.instrumentation.api.BaggageContext;
import datadog.trace.core.util.EscapedData;
import datadog.trace.core.util.PercentEscaper;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.function.BiConsumer;
import javax.annotation.ParametersAreNonnullByDefault;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@ParametersAreNonnullByDefault
public class BaggagePropagator implements Propagator {
private static final Logger log = LoggerFactory.getLogger(BaggagePropagator.class);
private static final PercentEscaper UTF_ESCAPER = PercentEscaper.create();
static final String BAGGAGE_KEY = "baggage";
private final Config config;
private final boolean injectBaggage;
private final boolean extractBaggage;

public BaggagePropagator(Config config) {
this.injectBaggage = config.isBaggageInject();
this.extractBaggage = config.isBaggageExtract();
this.config = config;
}

// use primarily for testing purposes
public BaggagePropagator(boolean injectBaggage, boolean extractBaggage) {
this.injectBaggage = injectBaggage;
this.extractBaggage = extractBaggage;
config = Config.get();
}

@Override
public <C> void inject(Context context, C carrier, CarrierSetter<C> setter) {
int maxItems = config.getTraceBaggageMaxItems();
int maxBytes = config.getTraceBaggageMaxBytes();
//noinspection ConstantValue
if (!this.injectBaggage
|| maxItems == 0
|| maxBytes == 0
|| context == null
|| carrier == null
|| setter == null) {
return;
}

BaggageContext baggageContext = BaggageContext.fromContext(context);
if (baggageContext == null) {
log.debug("BaggageContext instance is missing from the following context {}", context);
return;
}

String baggageHeader = baggageContext.getW3cBaggageHeader();
if (baggageHeader != null) {
setter.set(carrier, BAGGAGE_KEY, baggageHeader);
return;
}

int processedBaggage = 0;
int currentBytes = 0;
StringBuilder baggageText = new StringBuilder();
for (final Map.Entry<String, String> entry : baggageContext.asMap().entrySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment: this is where using Collections.unmodifiableMap() will get better.
It will avoid allocating a new Map during injection call.

// if there are already baggage items processed, add and allocate bytes for a comma
int extraBytes = 1;
if (processedBaggage != 0) {
baggageText.append(',');
extraBytes++;
}

EscapedData escapedKey = UTF_ESCAPER.escapeKey(entry.getKey());
EscapedData escapedVal = UTF_ESCAPER.escapeValue(entry.getValue());

baggageText.append(escapedKey.getData());
baggageText.append('=');
baggageText.append(escapedVal.getData());

processedBaggage++;
// reached the max number of baggage items allowed
if (processedBaggage == maxItems) {
break;
}
// Drop newest k/v pair if adding it leads to exceeding the limit
if (currentBytes + escapedKey.getSize() + escapedVal.getSize() + extraBytes > maxBytes) {
baggageText.setLength(currentBytes);
break;
}
currentBytes += escapedKey.getSize() + escapedVal.getSize() + extraBytes;
}

String baggageString = baggageText.toString();
baggageContext.setW3cBaggageHeader(baggageString);
setter.set(carrier, BAGGAGE_KEY, baggageString);
}

@Override
public <C> Context extract(Context context, C carrier, CarrierVisitor<C> visitor) {
//noinspection ConstantValue
if (!this.extractBaggage || context == null || carrier == null || visitor == null) {
return context;
}
BaggageContextExtractor baggageContextExtractor = new BaggageContextExtractor();
visitor.forEachKeyValue(carrier, baggageContextExtractor);
BaggageContext extractedContext = baggageContextExtractor.extractedContext;
if (extractedContext == null) {
return context;
}
return extractedContext.storeInto(context);
}

public static class BaggageContextExtractor implements BiConsumer<String, String> {
private BaggageContext extractedContext;

BaggageContextExtractor() {}

/** URL decode value */
private String decode(final String value) {
String decoded = value;
try {
decoded = URLDecoder.decode(value, "UTF-8");
} catch (final UnsupportedEncodingException | IllegalArgumentException e) {
log.debug("Failed to decode {}", value);
}
return decoded;
}

private Map<String, String> parseBaggageHeaders(String input) {
Map<String, String> baggage = new HashMap<>();
char keyValueSeparator = '=';
char pairSeparator = ',';
int start = 0;

int pairSeparatorInd = input.indexOf(pairSeparator);
pairSeparatorInd = pairSeparatorInd == -1 ? input.length() : pairSeparatorInd;
int kvSeparatorInd = input.indexOf(keyValueSeparator);
while (kvSeparatorInd != -1) {
int end = pairSeparatorInd;
if (kvSeparatorInd > end) {
log.debug(
"Dropping baggage headers due to key with no value {}", input.substring(start, end));
return Collections.emptyMap();
}
String key = decode(input.substring(start, kvSeparatorInd).trim());
String value = decode(input.substring(kvSeparatorInd + 1, end).trim());
if (key.isEmpty() || value.isEmpty()) {
log.debug("Dropping baggage headers due to empty k/v {}:{}", key, value);
return Collections.emptyMap();
}
baggage.put(key, value);

kvSeparatorInd = input.indexOf(keyValueSeparator, pairSeparatorInd + 1);
pairSeparatorInd = input.indexOf(pairSeparator, pairSeparatorInd + 1);
pairSeparatorInd = pairSeparatorInd == -1 ? input.length() : pairSeparatorInd;
start = end + 1;
}
return baggage;
}

@Override
public void accept(String key, String value) {
// Only process tags that are relevant to baggage
if (key != null && key.equalsIgnoreCase(BAGGAGE_KEY)) {
Map<String, String> baggage = parseBaggageHeaders(value);
if (!baggage.isEmpty()) {
extractedContext = BaggageContext.create(baggage, value);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ private static Map<TracePropagationStyle, Injector> createInjectors(
case TRACECONTEXT:
result.put(style, W3CHttpCodec.newInjector(reverseBaggageMapping));
break;
case BAGGAGE:
break;
default:
log.debug("No implementation found to inject propagation style: {}", style);
break;
Expand Down Expand Up @@ -159,6 +161,8 @@ public static Extractor createExtractor(
case TRACECONTEXT:
extractors.add(W3CHttpCodec.newExtractor(config, traceConfigSupplier));
break;
case BAGGAGE:
break;
default:
log.debug("No implementation found to extract propagation style: {}", style);
break;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package datadog.trace.core.util;

public class EscapedData {
private String data;
private int size;

public EscapedData(String data, int size) {
this.data = data;
this.size = size;
}

public EscapedData() {
this.data = "";
this.size = 0;
}

public String getData() {
return data;
}

public int getSize() {
return size;
}

public void setData(String data) {
this.data = data;
}

public void incrementSize() {
size++;
}

public void addSize(int delta) {
size += delta;
}
}
Loading
Loading