Proposal for OpenTelemetry2 baggage management

[beskow]

Description

This is a proposal for a potential enhancement for baggage management when using camel-opentelemetry2. The rationale and problem statement can be found in https://issues.apache.org/jira/projects/CAMEL/issues/CAMEL-23349. This seemed to me to be the most obvious and simplest solution, but there might of course be better ways?

In essence, the pull request adds two public methods for setting and getting baggage entries from an OTEL span
(encapsulated by OpenTelemetrySpanAdapter). The crucial detail is that the OpenTelemetrySpanAdapter encapsulates both the baggage and a baggageScope. Hence when adding a baggage entry, a new baggageScope is created which should replace the original baggageScope (which should be properly closed).

I also added a static Exchange context accessor for the OpenTelemetrySpanAdapter, which is not strictly necessary but may be convenient.

Target

• I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

• If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

• I checked that each commit in the pull request has a meaningful subject line and body.

• I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

[github-actions]

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

• First-time contributors require MANUAL approval for the GitHub Actions to run
• You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
• You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
• Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

[github-actions]

🧪 CI tested the following changed modules:

• components/camel-opentelemetry2

All tested modules (8 modules)

• Camel :: JBang :: MCP
• Camel :: JBang :: Plugin :: Route Parser
• Camel :: JBang :: Plugin :: Validate
• Camel :: Launcher :: Container
• Camel :: Observability Services
• Camel :: Opentelemetry 2
• Camel :: YAML DSL :: Validator
• Camel :: YAML DSL :: Validator Maven Plugin

---

⚙️ View full build and test results

[squakez]

Thanks for the PR. As commented in the Jira issue, the problem is that the changes proposed here are not compliant with the design of the component. The concept of the Baggage was something we did not take in consideration when designing the telemetry as it's an Otel specific concept.

I think it makes sense to try to figure it out how to include it, if necessary, changing the design of the abstract component in order to make it easier onboard the concept in Otel.

In any case, it requires a bit more of conceptual discussion before diving into a technical solution.

[squakez]

This is an adapter, it has to know nothing about storage.

[beskow]

Understood, I have removed it.

[squakez]

I think it makes sense to be final to avoid any kind of tampering.

[beskow]

I agree it makes sense to keep final state final, to avoid any kind of tampering. But this is the very essence of the proposed change: The initial bagage set in the constructor (and hence at route initialization, if I understand it correctly):

this.baggage = baggage.toBuilder().put(BAGGAGE_CAMEL_FLAG, "true").build();

cannot be final, if a processor in a route should be able to add to that baggage.

This is precisely the requirement we face: Somewhere in the inbound message (in a header, or as part of the payload) we need to extract a business-defined concept (typically a business correlation id) and make it available as baggage. Since we need to extract it from the inbound message, I seen no other practical way to implement besides in a processor (which by definition will execute after the route is started and the initial, currently final baggage is set).

Is there another way?

[squakez]

The adapter should know nothing about the exchange concept.

[beskow]

Understood, I have removed it.

[squakez]

Potential leak. You close the previous scope, but you have no guaranty that this one is going to be closed by anybody.

[beskow]

Is that really so? If I understand the implicit contract of the OpenTelemetrySpanAdapter, it is created and activated when the exchange starts:

org.apache.camel.telemetry.TracingRoutePolicy.onExchangeBegin() ->
org.apache.camel.telemetry.Tracer.beginEventSpan(..) ->
org.apache.camel.opentelemetry2.OpenTelemetryTracer.activate() ->
org.apache.camel.opentelemetry2.OpenTelemetrySpanAdapter.makeCurrent()

and closed on exchange completion:

org.apache.camel.support.EventHelper.notifyExchangeSent(..) ->
org.apache.camel.telemetry.Tracer.notify(..) ->
org.apache.camel.telemetry.Tracer.endEventSpan(..) ->
org.apache.camel.opentelemetry2.OpenTelemetryTracer.close() ->
org.apache.camel.opentelemetry2.OpenTelemetrySpanAdapter.close()

Hence the baggageScope is "opened" on exchange start and closed at exchange end. If we "mutate" the baggageScope in a processor somewhere in between (by closing the original baggageScope and replacing it with an extended scope), we still have the same guarantee that it is closed at the end of the exchange.

Or am I misunderstanding something?

[beskow]

Thanks for looking into it!

I appreciate the generic abstractions Camel provides to telemetry, shielding it from technical details from the different implementations supported. The baggage concept is indeed specific to otel, which is why I thought it made sense to add support for it in the otel-specific component camel-opentelemetry2.

The OpenTelemetrySpanAdapter implements the generic abstraction (org.apache.camel.telemetry.Span). I proposed to extend it with required functionality for managing the otel-specific baggage concept. Would it be a better alternative to extend the generic abstraction (i.e. org.apache.camel.telemetry.Span) with some abstraction of the "baggage" concept, and leave it as a NOOP in telemetry components other than otel?

[beskow]

IMO, the major problem to include this specific technology is resumed by this statement:
the scope instance must be kept open as long as needed

This is exactly what the OpenTelemetrySpanAdapter already does, for the initial baggage and baggageScope created on exchange start. My proposal leverages that already existing mechanism, but with support for additional baggage entries.

[squakez]

I think the correct way to handle this is to be able to create the Baggage the first time we are generating it:

camel/components/camel-opentelemetry2/src/main/java/org/apache/camel/opentelemetry2/OpenTelemetryTracer.java

Line 101 in 9ad5938

Baggage baggage = null;

- this likely means reviewing the telemetry abstraction and find a way to instruct it to add any baggage information, not necessarily programmatically (ie, it could be by adding some header).

[beskow]

I think the correct way to handle this is to be able to create the Baggage the first time we are generating it:

camel/components/camel-opentelemetry2/src/main/java/org/apache/camel/opentelemetry2/OpenTelemetryTracer.java

Line 101 in 9ad5938

Baggage baggage = null;

• this likely means reviewing the telemetry abstraction and find a way to instruct it to add any baggage information, not necessarily programmatically (ie, it could be by adding some header).

I'm afraid that also happens too early (ie. on exchange start):

org.apache.camel.telemetry.Tracer.TracingRoutePolicy.onExchangeBegin(..) ->
org.apache.camel.telemetry.Tracer.beginEventSpan(..) ->
org.apache.camel.opentelemetry2.OpenTelemetryTracer.OpentelemetrySpanLifecycleManager.create(..)

We need access to the inbound message to extract the "business correlation id" that should be set as baggage. If we do that in a processor, it is too late. It would be ideal if I at that point (in a processor at the beginning of the route) could set a header with a map of baggageEntry=value pairs. But then I guess the creation of the baggage and baggageScope in OpentelemetrySpanLifecycleManager.create(..) would have to be delayed somehow?

[squakez]

We should have all the elements required to make it works. I have some POC that could be good. Here the deal:

You can "decorate" your route with some headers (or property), for example:

            @Override
            public void configure() {
                from("direct:start")
                        .routeId("start")
                        .log("A message")
                        .process(new Processor() {
                            @Override
                            public void process(Exchange exchange) throws Exception {
                                exchange.getIn().setHeader("BAGGAGE_XYZ", "my-val");
                            }
                        })
                        .process(new Processor() {
                            @Override
                            public void process(Exchange exchange) throws Exception {
                                exchange.getIn().setHeader("operation", "fake");
                            }
                        })
                        .to("log:info");
            }

on the otel side (hence, neither touching the general abstraction), we could introduce a check to evaluate the presence of those headers:

...
            if (extractor.get("BAGGAGE_XYZ") != null) {
                baggage = baggage.toBuilder().put("XYZ", extractor.get("BAGGAGE_XYZ").toString()).build();
            }
            System.out.println("********** Span (" + spanName + ") baggage " + baggage);

            return new OpenTelemetrySpanAdapter(builder.startSpan(), baggage);

In this way the baggage is going to be correctly propagated:

********** Span (start) baggage {}
********** Span (start) baggage {camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}
********** Span (log1-log) baggage {camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}
********** Span (process1-process) baggage {camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}
********** Span (process2-process) baggage {XYZ=ImmutableEntry{value=my-val, metadata=ImmutableEntryMetadata{value=}}, camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}
********** Span (to1-to) baggage {XYZ=ImmutableEntry{value=my-val, metadata=ImmutableEntryMetadata{value=}}, camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}
********** Span (log) baggage {XYZ=ImmutableEntry{value=my-val, metadata=ImmutableEntryMetadata{value=}}, camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}

I think this one could be a nice solution and it would introduce the possibility to "program" the baggage in whichever DSL, not necessarily in Java, and, above all, without the need to know the programming concepts of Otel.

wdyt?

[beskow]

Being able to add a baggage entry using a camel header or property would be really great!

In your example above, it looks like you have traceProcessors=true. I hope that is not a requirement, since generating technical spans would not generally be acceptable.

I don't mind using the Otel APIs (they are after abstractions in themselves, not implementation details), but they are clearly not sufficient in this case. As you say, my original proposal depends on Camel implementation details for the otel/camel bridge, which is indeed unfortunate. I certainly agree it would be much better if we can achieve the same result with just Camel-idiomatic constructs (like headers or properties).

My original acceptance test, adapted to the syntax you propose:

    @Test
    void testSetBaggage() throws InterruptedException {
        MockEndpoint mock = getMockEndpoint("mock:baggage");
        mock.expectedMessageCount(1);
        template.sendBody("direct:setBaggage", "Hello");
        mock.assertIsSatisfied();
        mock.getExchanges().forEach(exchange -> {
            String baggage = exchange.getIn().getHeader("baggage", String.class);
            assertTrue(baggage.contains("tenant.id=acme"));
        });
    }

    @Override
    protected RoutesBuilder createRouteBuilder() {
        return new RouteBuilder() {
            @Override
            public void configure() {
                from("direct:setBaggage")
                        .process(new Processor() {
                            @Override
                            public void process(Exchange exchange) {
                                exchange.getIn().setHeader("BAGGAGE_tenant.id", "acme");
                            }
                        })
                        .to("mock:baggage");
            }
        };
    }

Let me know if I can assist you further in this! Thanks!

[beskow]

If it is of any help, here is a minimalistic opentelemetry spring boot starter project with an additional test for automatic baggage inclusion in the MDC which is provided by io.opentelemetry.instrumentation.logback.mdc.v1_0.OpenTelemetryAppender as part of the opentelemetry spring boot starter.

camel-opentelemetry-starter-baggage.zip

The tests succeeds using my original proposal (still in the route, but commented out and replaced with the syntax you propose).

[squakez]

Good point about the processor, and I've verified this would be compatible. I think it can be even simplified using the setHeader which is more idiomatic. So, the route would be like:

                        .setHeader("BAGGAGE_XYZ", constant("Hello World"))
                        .routeId("start")
                        .log("A message")
                        .process(new Processor() {
                            @Override
                            public void process(Exchange exchange) throws Exception {
                                exchange.getIn().setHeader("operation", "fake");
                            }
                        })
                        .to("log:info");

Resulting in:

********** Span (start) baggage {}
********** Span (start) baggage {camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}
********** Span (log) baggage {XYZ=ImmutableEntry{value=Hello World, metadata=ImmutableEntryMetadata{value=}}, camelScope=ImmutableEntry{value=true, metadata=ImmutableEntryMetadata{value=}}}

I'm going to prepare a PR to introduce it, unless you see any possible drawback. Thanks for the time and the help to reason about this new feature.