Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -77,6 +77,7 @@ public final class ConfigDefaults {
static final int DEFAULT_SCOPE_ITERATION_KEEP_ALIVE = 30; // in seconds
static final int DEFAULT_PARTIAL_FLUSH_MIN_SPANS = 1000;
static final boolean DEFAULT_PROPAGATION_EXTRACT_LOG_HEADER_NAMES_ENABLED = false;
// TODO: add BAGGAGE to the list of default propagation styles
static final Set<TracePropagationStyle> DEFAULT_TRACE_PROPAGATION_STYLE =
new LinkedHashSet<>(asList(DATADOG, TRACECONTEXT));
static final Set<PropagationStyle> DEFAULT_PROPAGATION_STYLE =
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 propagation style
// 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.

Like DSM, Baggage is not a trace propagation style.
I'm introducing a new API for non tracing propagator.

// None does not extract or inject
NONE;

Expand Down
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:
result.put(style, W3CBaggageHttpCodec.newInjector(reverseBaggageMapping));
default:
log.debug("No implementation found to inject propagation style: {}", style);
break;
Expand Down Expand Up @@ -159,6 +161,9 @@ public static Extractor createExtractor(
case TRACECONTEXT:
extractors.add(W3CHttpCodec.newExtractor(config, traceConfigSupplier));
break;
case BAGGAGE:
extractors.add(W3CBaggageHttpCodec.newExtractor(config, traceConfigSupplier));
break;
default:
log.debug("No implementation found to extract propagation style: {}", style);
break;
Expand Down Expand Up @@ -228,6 +233,7 @@ public <C> TagContext extract(
context = extractedContext;
// Stop extraction if only extracting first valid context and drop everything else
if (this.extractFirst) {
// TODO: change this logic to always extract baggage
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a due to the fact it is not a tracing propagator.
I would rather see it integrated differently (maybe a as a decorator like I did for DSM).

break;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
package datadog.trace.core.propagation;

import static datadog.trace.api.TracePropagationStyle.BAGGAGE;
import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_DROP;
import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP;
import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue;
import static datadog.trace.core.propagation.PropagationTags.HeaderType.W3C;

import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.NANOSECONDS;

import datadog.trace.api.Config;
import datadog.trace.api.DD128bTraceId;
import datadog.trace.api.DDSpanId;
import datadog.trace.api.DDTags;
import datadog.trace.api.DDTraceId;
import datadog.trace.api.TraceConfig;
import datadog.trace.api.TracePropagationStyle;
import datadog.trace.api.internal.util.LongStringUtils;
import datadog.trace.api.sampling.PrioritySampling;
import datadog.trace.api.sampling.SamplingMechanism;
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
import datadog.trace.bootstrap.instrumentation.api.TagContext;
import datadog.trace.core.DDSpanContext;

import java.util.Map;
import java.util.TreeMap;
import java.util.function.Supplier;

import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** A codec designed for HTTP transport via headers using W3C "baggage" header */
class W3CBaggageHttpCodec {
Copy link
Contributor

Choose a reason for hiding this comment

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

Overall, how do we expect to handle properties? Should they be considered as part of the value?

Copy link
Member Author

@lucaspimentel lucaspimentel Jan 15, 2025

Choose a reason for hiding this comment

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

Do you mean these properties from the W3C spec?

baggage: key1=value1;property1;property2

OpenTelemetry doesn't support them (they simplified a few things from the W3C spec). A user can set "value;property" as a baggage value, but the semicolon will be encoded to %3B in the header (and later decoded) since ; is not allowed in the value itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

OpenTelemetry doesn't support them

Not yet 😇 But according to the doc:

Left opaque to allow for future functionality.

No, I was thinking:

  • We want to introduce W3C baggage propagator (according the PR title :p)
  • W3C baggage header will not always come from an OTel upstream service, so you might have headers with values having properties (and not URL encoded).
  • So, how is it supposed to be handled on our side? Are properties part of the value or not?

private static final Logger log = LoggerFactory.getLogger(W3CHttpCodec.class);

static final String BAGGAGE_KEY = "baggage";

private W3CBaggageHttpCodec() {
// This class should not be created. This also makes code coverage checks happy.
}

public static HttpCodec.Injector newInjector(Map<String, String> invertedBaggageMapping) {
return new Injector(invertedBaggageMapping);
}

private static class Injector implements HttpCodec.Injector {

private final Map<String, String> invertedBaggageMapping;

public Injector(Map<String, String> invertedBaggageMapping) {
assert invertedBaggageMapping != null;
this.invertedBaggageMapping = invertedBaggageMapping;
}

@Override
public <C> void inject(
final DDSpanContext context, final C carrier, final AgentPropagation.Setter<C> setter) {
StringBuilder baggageHeader = new StringBuilder();

for (final Map.Entry<String, String> entry : context.baggageItems()) {
if (baggageHeader.length() > 0) {
baggageHeader.append(",");
}

String header = invertedBaggageMapping.get(entry.getKey());
header = header != null ? header : entry.getKey();
String value = HttpCodec.encodeBaggage(entry.getValue());

baggageHeader.append(header).append("=").append(value);
}

setter.set(carrier, BAGGAGE_KEY, baggageHeader.toString());
}
}

public static HttpCodec.Extractor newExtractor(
Config config, Supplier<TraceConfig> traceConfigSupplier) {
return new TagContextExtractor(traceConfigSupplier, () -> new W3CBaggageContextInterpreter(config));
}

private static class W3CBaggageContextInterpreter extends ContextInterpreter {

private W3CBaggageContextInterpreter(Config config) {
super(config);
}

@Override
public TracePropagationStyle style() {
return BAGGAGE;
}

@Override
public boolean accept(String key, String value) {
if (null == key || key.isEmpty()) {
return true;
}
if (LOG_EXTRACT_HEADER_NAMES) {
log.debug("Header: {}", key);
}

char first = Character.toLowerCase(key.charAt(0));

if (first == 'b' && BAGGAGE_KEY.equalsIgnoreCase(key)) {
try {
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
}

for (String entry : value.split(",")) {
String[] keyValue = entry.split("=", 2);
if (keyValue.length == 2) {
baggage.put(
HttpCodec.decode(keyValue[0].trim()),
HttpCodec.decode(keyValue[1].trim())
);
}
}
} catch (RuntimeException e) {
invalidateContext();
log.debug("Exception when extracting baggage", e);
return false;
}
}
return true;
}

private long extractEndToEndStartTime(String value) {
try {
return MILLISECONDS.toNanos(Long.parseLong(value));
} catch (RuntimeException e) {
log.debug("Ignoring invalid end-to-end start time {}", value, e);
return 0;
}
}

private static String trim(String input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a benefit compared to String.trim() ?

Copy link
Member Author

@lucaspimentel lucaspimentel Jan 15, 2025

Choose a reason for hiding this comment

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

Happy to change it. I just copied this from W3CHttpCodec.java 😅

if (input == null) {
return "";
}
final int last = input.length() - 1;
if (last == 0) {
return input;
}
int start;
for (start = 0; start <= last; start++) {
char c = input.charAt(start);
if (c != '\t' && c != ' ') {
break;
}
}
int end;
for (end = last; end > start; end--) {
char c = input.charAt(end);
if (c != '\t' && c != ' ') {
break;
}
}
if (start == 0 && end == last) {
return input;
} else {
return input.substring(start, end + 1);
}
}
}
}
Loading