W3CBaggagePropagator wipe baggage from other propagators

[ddark008]

Describe the bug

I am currently working on developing custom propagators that parse baggage from specific headers.
However, when trying to use these custom propagators alongside the W3CBaggagePropagator, I encounter an unexpected behavior.
It seems that the W3CBaggagePropagator creates a new BaggageBuilder and discards the current context Baggage, preventing my custom propagators from functioning properly.

Best solution use exist baggage:

Baggage baggage = Baggage.fromContext(context);

Steps to reproduce

import java.util.List;

import javax.annotation.Nullable;

import io.opentelemetry.api.baggage.Baggage;
import io.opentelemetry.api.baggage.BaggageBuilder;
import io.opentelemetry.api.internal.StringUtils;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapGetter;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.context.propagation.TextMapSetter;

public class XReqIdPropagator implements TextMapPropagator {
    public static final String X_REQUEST_ID_HEADER = "X-Request-Id";

    @Override
    public List<String> fields() {
        return List.of(X_REQUEST_ID_HEADER);
    }

    @Override
    public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
        Baggage baggage = Baggage.fromContext(context);
        String outgoingReqId = baggage.getEntryValue("req-id-key");
        if (!StringUtils.isNullOrEmpty(outgoingReqId)) {
            setter.set(carrier, X_REQUEST_ID_HEADER, outgoingReqId);
        }
    }

    @Override
    public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
        String incomingReqId = getter.get(carrier, X_REQUEST_ID_HEADER);
        if (!StringUtils.isNullOrEmpty(incomingReqId)) {
           // Use exist Baggage
            Baggage baggage = Baggage.fromContext(context);
            BaggageBuilder builder = baggage.toBuilder();
            builder.put("req-id-key", incomingReqId);
            return context.with(builder.build());
        }
        return context;
    }
}

import com.google.auto.service.AutoService;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurablePropagatorProvider;

@AutoService(ConfigurablePropagatorProvider.class)
public class XReqIdPropagatorProvider implements ConfigurablePropagatorProvider {
    @Override
    public TextMapPropagator getPropagator(ConfigProperties config) {
        return new XReqIdPropagator();
    }

    @Override
    public String getName() {
        return "xreqid";
    }
}

Add to config otel.propagators = tracecontext,baggage,xreqid

Expected behavior

Context contain baggage from all propagators

Actual behavior

Context contain baggage only from W3CBaggagePropagator

Javaagent or library instrumentation version

1.29.0

Environment

JDK: 17
OS: Ubuntu

Additional context

No response

[laurit]

@jkwatson @jack-berg could you verify that current behavior is intended or transfer this to opentelemetry-java if this is a valid concern

[jkwatson]

This is intended behavior as far as I know. The W3C Baggage Propagator is not intended to combine baggage with any other baggage propagator (much as the W3C Trace Context propagator doesn't somehow combine trace contexts from other propagators). I'll transfer the issue, but I'm pretty sure there would need to be a spec change to support this.

[jack-berg]

@ddark008 can't you get the behavior you intend by ensuring that your custom xreqid propagator runs after W3CBaggagePropagator?

[ddark008]

@jack-berg Thank you for your answer!
That will be sufficient. The propagators are runining according to the in the property otel.propogators ?

[jack-berg]

@ddark008 yup you can see the source code that interprets otel.propagators here. We read in the property, translate each to the corresponding TextMapPropagator, and call TextMapPropagator.composite(..) with the resulting collection, which ultimately returns [MultiTextMapPropagator](https://github.com/open-telemetry/opentelemetry-java/blob/main/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java. MultiTextMapPropagator will iterate through the propagators in order and apply each to the resulting context.

So setting otel.propagators=baggage,xreqid should result in xreqid running after W3CBaggagePropagator, and achieve the behavior you want.

[breedx-splk]

@ddark008 please close if this has been satisfied. Thanks! 🙏🏻

[ddark008]

@jack-berg thank you for the detailed response. It would be great if this behavior is documented to ensure it doesn't change during future refactoring.