Baggage items are not being put in scope when extracted

[bdeneuter]

Hi,

We send baggage items as headers on Kafka records. When we extract the span in the consumer and put the span in scope with tracer.withSpan(extractedSpan), the bagage items are not available.
I created a test project with a failing test. In this test:

• Add a key to the baggage in the current context
• Use a propagator to inject the context in a map
• Validate the bagage item is in the map
• Use a propagator to extract the context
• Set the extracted span in scope
• Validate that the bagage item is present

I'm using the OpenTelemetry bridge and you find the test project here:
https://github.com/bdeneuter/baggage-test

[bdeneuter]

We were discussing this issue on Slack. I'm posting the conversation here for keeping track:
https://micrometer-metrics.slack.com/archives/C662HUJC9/p1669849341671159

Marcin Grzejszczak
Check this https://micrometer.io/docs/tracing#_micrometer_tracing_baggage_api

Marcin Grzejszczak
For samples try https://github.com/micrometer-metrics/micrometer-samples/tree/main/baggage-producer and https://github.com/micrometer-metrics/micrometer-samples/tree/main/baggage-consumer

Bart De Neuter
Thx @Marcin Grzejszczak
The baggage API is a confusing API. The baggage is present on the span that has been extracted with a propagator. But the baggage doesn't seem to be set and passed to the child spans?
This succeeds:

try (var context = tracer.withSpan(extractedSpan)) {
    assertThat(tracer.getBaggage(ROOT_SERVICE).get(extractedSpan.context())).isEqualTo("testApp");
}

So if I give the context from the extracted span, the baggage is returned.
But when I don't pass the extracted span context explicitly, the baggage is not returned. Which is weird because the extractedSpan is in scope with tracer.withSpan(extractedSpan) . Why does this fail?

try (var context = tracer.withSpan(extractedSpan)) {
   try(var baggage = tracer.getBaggage(ROOT_SERVICE).makeCurrent()) {
       assertThat(tracer.getBaggage(ROOT_SERVICE).get()).isEqualTo("testApp");
   }
}

It is also not very clear from the documentation when you need to use baggage.makeCurrent() and when not. Is it only for getting a value? Is it needed when you pass the context? It seems it is not needed to set a value? (edited)

Marcin Grzejszczak
The baggage is passed to child spans. Which versions are you using? What tracer are you using?

Bart De Neuter
I'm using version 1.0.0 and use the OpenTelemetry bridge

Bart De Neuter
If I put the extracted span in scope and use the propagator to inject the current context in a map, the baggage from the extracted span is not present

Bart De Neuter
So this fails:

try (var context = tracer.withSpan(extractedSpan)) {
    var map = new HashMap<String, String>();
    propagator.inject(tracer.currentTraceContext().context(), map, (carrier, key, value) -> carrier.put(key, value));
    assertThat(map).containsKey(ROOT_SERVICE);
}

Marcin Grzejszczak
let me check that , I mean we have tests that prove that the baggage gets propagated so I have no idea what's going on here. I'll take the baggage example from micrometer-samples and play around with it

Marcin Grzejszczak
where does the extractedSpan take the baggage from? Did you set it manually? Is it coming from a controller?

Marcin Grzejszczak
Have you provided the baggage as a property like this? (edited)

Marcin Grzejszczak

management:
  tracing:
    sampling:
      probability: 1.0
    baggage:
      remote-fields:
        - mybaggage
        - myremotefield

Bart De Neuter
No, I have set the baggage in code with:

tracer.createBaggage(ROOT_STARTED_AT, String.valueOf(Instant.now().toEpochMilli()));
tracer.createBaggage(ROOT_RESOURCE, record.topic());
tracer.createBaggage(ROOT_SERVICE, serviceName);

As I understand correctly, the property remote-fields will set the key as is on the header. The rest of the baggage items are still put on the Kafka record.
I created a test project with a failing test. Should I create a bug ticket so it can be investigated?

Marcin Grzejszczak
you must define the baggage as a remote-field entry as presented above

Marcin Grzejszczak
sure create the project, create a ticket and I will look into that
👍

Bart De Neuter
We tried it but didn't change anything

Marcin Grzejszczak
I think you should try putting those baggage entries in scope

Marcin Grzejszczak
cause especially with OTel stuff needs to be put in scope

Bart De Neuter
Thx for the help. I have created the bug issue here: #122

👍

Marcin Grzejszczak
Maybe we need to improve the API a bit but I know what the problem is. Also it might be that you caught a bug - I'll look into that.

@Test
	void injectAndExtractKeepsTheBaggage() {
		// GIVEN
		var carrier = new HashMap<String, String>();

		var span = tracer.nextSpan().start();
		try(var spanInScope = tracer.withSpan(span)) {
			tracer.createBaggage(KEY_1, VALUE_1);

			// WHEN
			propagator.inject(tracer.currentTraceContext().context(), carrier, HashMap::put);

			// THEN
			assertThat(carrier.get(KEY_1)).isEqualTo(VALUE_1);
		}

		// WHEN
		var extractedSpan = propagator.extract(carrier, HashMap::get).start();

		// THEN
		try(var spanInScope = tracer.withSpan(extractedSpan)) {
			assertThat(tracer.getBaggage(KEY_1).get(extractedSpan.context())).isEqualTo(VALUE_1);
			try(var baggageInScope = tracer.getBaggage(KEY_1).makeCurrent()) {
				assertThat(baggageInScope.get()).isEqualTo(VALUE_1);
			}
		}
	}

so this test works until the last assertion. Instead of calling tracer.getBaggage(KEY_1).makeCurrent() you should call tracer.getBaggage(extractedSpan.context(), KEY_1) . If possible always pass the trace context explicitly (in this case the one from extractedSpan).

Bart De Neuter
I think it should work on the tracer without the need to pass the context of the extracted span. In our production code we would consume from Kafka, set the span in scope and call other services. These other services can cause events that will be published again on Kafka. But in those services, we don't have the reference anymore to the extracted span. So a propagator is being used to inject the baggage from the current scope into the carrier. With the current implementation, we loose the baggage

Bart De Neuter
I added a test to show what I mean:

@Test
	void fakeKafkaExample() {
		// GIVEN
		var span = tracer.nextSpan().start();
		try(var spanInScope = tracer.withSpan(span)) {
			tracer.createBaggage(KEY_1, VALUE_1);
			kafka.publish("event 1");
		}

		// WHEN
		kafka.consume(message -> {

			// THEN
			assertThat(message).isEqualTo("event 1");
			try(var baggage = tracer.getBaggage(KEY_1).makeCurrent()) {
				assertThat(baggage.get()).isEqualTo(VALUE_1);
			}

			// WHEN
			kafka.publish("event 2");

			// THEN
			kafka.consume(message2 -> {
				assertThat(message2).isEqualTo("event 2");
				try(var baggage = tracer.getBaggage(KEY_1).makeCurrent()) {
					assertThat(baggage.get()).isEqualTo(VALUE_1);
				}
			});

		});
	}

Marcin Grzejszczak
Sure, so I think the bug is about us not reading the current context properly. So if you take the current context from the bean and manually pass it to getBaggage then it should work fine (edited)

Bart De Neuter
I added a test to validate if baggage is passed to child spans and this test passes. So it seems it is a bug when the baggage is extracted and not placed correctly on the extracted span. The baggage is there because you can get the baggage when using the context of the extracted span directly. But it is not set correctly because when the span is put in scope, the baggage is not on the tracer.