[
https://issues.apache.org/jira/browse/CAMEL-23349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18082282#comment-18082282
]
Bjorn Beskow commented on CAMEL-23349:
--------------------------------------
[~squakez]:
Having read the tracing design proposal
[https://github.com/apache/camel/blob/main/proposals/tracing.adoc] more
thoroughly,
I understand we move away from a ThreadLocal-based approach to Scope and
BaggageScope (and instead continue to rely on the Otel span with corresponding
baggage to be associated with the Exchange and updated when needed). I
completely agree with that move. The fix in PR
[https://github.com/apache/camel/pull/22916] is fully in line with that. The
OTEL ThreadLocal-based programming model for Baggage clearly needs to be
"adapted" to fit the exchange-centric/event-centric programming model in Camel.
Back to the original problem then:
The OTEL Baggage API mutates the baggage content via the Thread scope (even
though the Baggage object itself is immutable, with a Builder pattern to add
new entries by creating a new, extended instance):
{code:java}
Baggage baggage = Baggage.builder()
.put("tenant.id", "1234")
.build();
try (Scope scope = baggage.storeInContext(Context.current()).makeCurrent()){
// baggage is now tied to the current thread, and hence propagated and
accessible for e.g. logging
System.out.println(Baggage.current().getEntryValue("tenant.id");
}
{code}
If the current span associated with the exchange should be the preferred
carrier of the tracing information, isn't it reasonable that a Camel-idiomatic
Baggage API can extend (i.e. replace) the Baggage object associated with the
exchange? It could look something like
{code:java}
OpenTelemetryBaggageAdapter.builder(exchange)
.put("tenant.id", "1234")
.build();
// baggage is now tied to the current Otel span associated with the Exchange,
and hence propagated and accessible for e.g. logging
System.out.println(BaggageAdapter.current(exchange).getEntryValue("tenant.id");
{code}
To implement such a OpenTelemetryBaggageAdapter, it must be possible to replace
the Baggage instance associated with the exchange with the an extended baggage
instance, i.e. in {{OpenTelemetrySpanAdapter}} the Baggage instance would be
non-final and mutable via a setter:
{code:java}
private Baggage baggage;
...
protected void setBaggage(Baggage baggage) {
this.baggage = baggage;
}
{code}
With such a change, the OpenTelemetryBaggageAdapter can be implemented cleanly:
{code:java}
public class OpenTelemetryBaggageAdapter {
static SpanStorageManagerExchange spanManager = new
SpanStorageManagerExchange();
private OpenTelemetryBaggageAdapter() {
}
public static Baggage current(Exchange exchange) {
Span span = spanManager.peek(exchange);
if (span instanceof OpenTelemetrySpanAdapter spanAdapter) {
return spanAdapter.getBaggage();
} else {
throw new IllegalStateException("No current OTEL span available");
}
}
public static Builder builder(Exchange exchange) {
return new Builder(exchange);
}
public static final class Builder {
private final Exchange exchange;
private final Map<String, String> entries = new LinkedHashMap<>();
private Builder(Exchange exchange) {
this.exchange = exchange;
}
public Builder put(String key, String value) {
entries.put(key, value);
return this;
}
public Baggage build() {
Span span = spanManager.peek(exchange);
if (span instanceof OpenTelemetrySpanAdapter spanAdapter) {
Baggage baggage = spanAdapter.getBaggage();
var builder = baggage.toBuilder();
entries.forEach(builder::put);
baggage = builder.build();
spanAdapter.setBaggage(baggage);
return baggage;
} else {
throw new IllegalStateException("No current OTEL span
available");
}
}
}
}
{code}
This implementation has a fully consistent behaviour with other OTEL
instrumentation libraries, as well as the Camel threading model (I have tested
it with e.g. the threads() construct).
This is an OpenTelemetry-specific solution, but the Baggage concept is itself
OpenTelemetry-specific. The tracing design suggests common concepts should be
extracted into strong abstractions in camel-telemetry, but the implementation
details be kept in the implementation-specific components (for very good
reasons). But that doesn't have to imply a "least common denominator" for the
implementation-specific components, IMHO.
Can I submit a pull request for further discussion?
> Camel OpenTelemetry2 programmatic baggage management
> ----------------------------------------------------
>
> Key: CAMEL-23349
> URL: https://issues.apache.org/jira/browse/CAMEL-23349
> Project: Camel
> Issue Type: Improvement
> Components: camel-opentelemetry
> Affects Versions: 4.18.1, 4.19.0
> Reporter: Bjorn Beskow
> Assignee: Pasquale Congiusti
> Priority: Major
> Fix For: 4.20.0
>
>
> With Camel OpenTelemetry2 and OpenTelemetry spring boot starter, most aspects
> of Span Customization such as adding attributes, events and creating nested
> spans can be done using the OpenTelemetry apis. Programmatically adding OTEL
> baggage is however an exception. The typical use case for OTEL baggage is to
> allow for small pieces of contextual data that need to travel with a request
> across distributed service boundaries, independent of any single span. In our
> business context, it is in the form of a "business correlation id" that is
> captured from a header or from the message payload as the very first
> processor of a boundary component, and should be available to all downstream
> components.
> The OTEL api itself cannot easily be used to add baggage items to a Camel
> route, due to `Scope` handling: Baggage must be added to a baggage scope, and
> is visible and propagated during the lifetime of the scope. The OTEL api
> looks like this:
> // Create baggage with an entry
> Baggage baggage = Baggage.current().builder()
> .put("user-id", "12345")
> .build();
> // Attach baggage to current context
> Context contextWithBaggage = Context.current().with(baggage);
> // Make it current (scoped)
> try (var scope = contextWithBaggage.makeCurrent())
> { // Your code here - baggage is now active }
> Doing this in a processor, the scope instance must be kept open as long as
> needed, typically until the end of the exchange. In theory, the following
> simple albeit brittle solution should work:
>
> Baggage baggage = Baggage.current().toBuilder()
> .put("user-id", "12345")
> .build();
> Context ctxWithBaggage = Context.current().with(baggage);
> Scope scope = ctxWithBaggage.makeCurrent();
> exchange.getExchangeExtension().addOnCompletion(new SynchronizationAdapter() {
> @Override public void onDone(Exchange exchange)
> { scope.close(); }
> });
> In practice however, this doesn't work. A (final) baggageScope is already
> (automatically) created at the start of a Camel route execution, stored on
> the exchange as part of a wrapped instance of
> org.apache.camel.opentelemetry2.OpenTelemetrySpanAdapter and properly closed
> at the end of the route. It is this baggageScope that is passed to the OTEL
> propagator, not the scope created programmatically as in the example above.
> Hence the programmatically added baggage entry will not be propagated.
> To my understanding, the simplest way to programmatically add baggage items
> would be to allow for mutating the baggage scope stored in the
> org.apache.camel.opentelemetry2.OpenTelemetrySpanAdapter. The required pieces
> are already present, but would need to be exposed in a public API (currently,
> both context, baggage and baggage scope are protected).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
