[Messaging] [JMS] custom created spans do not have a proper parent when using .receive() to handle a message

[cbos]

With JMS there are 2 ways to get a message and handle it:

// Perform a lookup on the queue
Queue queue = (Queue) initialContext.lookup(QUEUE_REF);

// Create a JMS Session
Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);

// Create consumer
consumer = session.createConsumer(queue);

consumer.setMessageListener(message -> {
	// while in this message listener the 'process' context is active
	handleMessage(session, (MapMessage) message, producer);
});

// Perform a lookup on the queue
Queue queue = (Queue) initialContext.lookup(QUEUE_REF);

// Create a JMS Session
Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);

// Create consumer
consumer = session.createConsumer(queue);

MapMessage message = (MapMessage) consumer.receive();
// The 'receive' context is only active in the receive method above, the context has stopped as soon as handleMessage is invoked.
handleMessage(session, message, producer);

Due to the way the auto instrumentation is creating the span and the context, the context is already closed as soon as you 'leave' the receive method.
Any custom created span in handleMessage is 'parentless'/not related to span which marks the receive.
Due to that the trace is broken.

Solution

The context needs to be extended in such a way that you can create custom spans and have the in the same 'receive' context.

A potential solution can look like this:

Here is an example implementation: ing-bank@fcfd92b
This was part of a different PR, but has been reverted to prevent mixing things.

[cbos]

@trask / @laurit
I separated this topic from the PR #13192 and made it a new issue.
But apparently, I created it in the wrong way, it does not have labels attached right now. How to get that attached in the right way? Due to lack of permissions, I cannot change labels.

[laurit]

@cbos what labels do you want?

[cbos]

I think at least 'enhancement'. I don't know it triage is needed or a label for that is needed as well.

Solution direct can be discussed on this ticket or based on a PR. The referred code can be used to create a PR as well.

[laurit]

If I understand correctly the proposal is to leave the context for receive operation active until receive is called again. The problem with this approach is that when receive is called without calling receive again it may lead to leaking a scope which results in all subsequent spans from that being under the receive span. Since users generally will perceive such situation as a bug I'd be reluctant to accept this. What if we did something similar to what is proposed in #11259 Add a message header with the receive context, so that user could restore the receive context from that header (will require manual instrumentation). Idk whether it is actually possible to add properties to a jms message. Maybe would need provide an api that allows getting context for the message. Under the hood that api could use virtual fields or a weak map.

[trask]

@cbos also check out open-telemetry/opentelemetry-java-contrib#1855

it's not automatic instrumentation because of the issues @laurit describes, but is designed to help users manually bridge receive -> process

Add a message header with the receive context, so that user could restore the receive context from that header (will require manual instrumentation).

cc @Cirilla-zmh wdyt about this approach? if it can work it could further simplify the manual instrumentation required

[Cirilla-zmh]

@trask Thank you! I think messaging wrappers are exactly what were designed to solve issues like this!

Add a message header with the receive context, so that user could restore the receive context from that header (will require manual instrumentation).

Yes, we do adopt a similar approach for messaging systems that use polling-mode consumers. However, not all systems provide available user properties or headers where we can safely store the context (see mqtt message). Therefore, I believe that the "receive context" still plays a necessary role, and users should be able to access this context through messaging wrappers.

[cbos]

@trask / @Cirilla-zmh
Thanks for your replies. I will look into this to see how to get that to work, also in the situation we have at hand.

[cbos]

@trask / @Cirilla-zmh
I have looked into the links you provided and the explanation given.

But I think this is a different situation.

For JMS messaging instrumentation is supported.
It also creates a span based on the data send with the message.

You can get the received message via 2 API's.

One is via a message listener:

            // Perform a lookup on the queue
            Queue queue = (Queue) initialContext.lookup(QUEUE_REF);

            // Create a JMS Session
            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);

            // Create consumer and producer
            consumer = session.createConsumer(queue);

            consumer.setMessageListener(message -> {

//As soon as you handling a message here, there is 'context' created. Any code which creates a span, has the context and proper parent-child relation is created. 
                handleMessage(session, (MapMessage) message);
            });


@WithSpan(kind = SpanKind.INTERNAL, value = "doAnalyticsProcessing")
private void doAnalyticsProcessing(Session session, MapMessage message){
...
}

This works without any problem.

But you can also do it in a different way:

                    // Perform a lookup on the queue
                    Queue queue = (Queue) initialContext.lookup(QUEUE_REF);

                    // Create a JMS Session
                    Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);

                    // Create consumer and producer
                    MessageConsumer consumer = session.createConsumer(queue);

                    while (true) {
                        MapMessage message = (MapMessage) consumer.receive();
//The .receive method closes the 'context' directly, no active context is active
// Any span created here is missing the context.                        
handleMessage(session, message);
                    }

@WithSpan(kind = SpanKind.INTERNAL, value = "doAnalyticsProcessing")
private void doAnalyticsProcessing(Session session, MapMessage message){
...
}

In the above code you can see that the context is missing as soon as the .receive() message is closed.
This is all auto instrumentation.

You can also create spans with configuration, by setting the environment variable OTEL_INSTRUMENTATION_METHODS_INCLUDE, see
https://opentelemetry.io/docs/zero-code/java/agent/annotations/#creating-spans-around-methods-with-otelinstrumentationmethodsinclude

These spans lack the context as well.

The message wrapper as proposed in open-telemetry/opentelemetry-java-contrib#1855 does not solve the problem for 2 reaseons:

• This proposed solution is also about manually instrumenting the code, while that is not possible in all cases
• This will create a second span related to the 'parent' (send span at the procuder) based on the message, but there is already a span created in the consumer, so the new span should be related to the 'receive' span, rather then the 'send' span of the producer. This problem is also indicated at Opentelemetry Kafka consumer doesnt inject context in headers #11259 (comment) In this comment

This will the be the outcome:

topic send (producer span for trace 1)
  ├─ topic process (consumer span for trace 1)
  └─ nested (nested application span for trace 1)

topic send (producer span for trace 2)
  ├─ topic process (consumer span for trace 2)
  └─ nested (nested application span for trace 2)

While I expect this:

topic send (producer span for trace 1)
  └─ topic process (consumer span for trace 1)
      └─ nested (nested application span for trace 1)

topic send (producer span for trace 2)
 └─ topic process (consumer span for trace 2)
     └─ nested (nested application span for trace 2)

A solution could be that in the '.receive()' handling the 'receive context' is added as attribute to the message or attach it with the VirtualField way to the message.
When creating the span, it can check for that context and relate it to the context, but that makes it harder, where to look for the context when creating the span.
And I think I should be done in such a way that span created by setting OTEL_INSTRUMENTATION_METHODS_INCLUDE should work as well.

The refered solution solves this, but I agree with @laurit this can result in a leak of the context, which might introduce other problems. So it at least should be disbled by default and made available with a feature toggle.

[Cirilla-zmh]

This will create a second span related to the 'parent' (send span at the procuder) based on the message, but there is already a span created in the consumer, so the new span should be related to the 'receive' span, rather then the 'send' span of the producer.

@cbos Yes! The 'messaging-wrappers' is not a complete solution for your use case, as I mentioned in the TODO I left in my PR (see kafka-client/build.gradle.kts).

The point is that we cannot access the 'receive context' outside the instrumentation project, unlike what we do in kafka-clients:

• We attach the 'receive context' here:
  

  opentelemetry-java-instrumentation/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/KafkaConsumerInstrumentation.java

  Lines 88 to 98 in 887f9cd

  	// we're storing the context of the receive span so that process spans can use it as
  	// parent context even though the span has ended
  	// this is the suggested behavior according to the spec batch receive scenario:
  	// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/messaging/messaging-spans.md#batch-receiving
  	// we're attaching the consumer to the records to be able to retrieve things like consumer
  	// group or clientId later
  	KafkaConsumerContextUtil.set(records, context, consumer);
  	for (ConsumerRecord<?, ?> record : records) {
  	KafkaConsumerContextUtil.set(record, context, consumer);
  	}

• We get the 'receive context' and make it as the parent of process span here:
  

  opentelemetry-java-instrumentation/instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/common/v0_11/internal/TracingIterator.java

  Lines 46 to 47 in 887f9cd

  	// use the receive CONSUMER as parent if it's available
  	this.parentContext = receiveContext != null ? receiveContext : Context.current();

A solution could be that in the '.receive()' handling the 'receive context' is added as attribute to the message or attach it with the VirtualField way to the message.

Yes. This is exactly the solution I prefer.

So in your case, perhaps we should simply attach the 'receive context' to the message and retrieve it when creating the process span using messaging-wrappers: (after migrating the 'messaging-wrappers' to instrumentation @trask)

Another solution is to inject the trace context of the receive context into the headers or user properties of the message, in which case the messaging-wrappers would no longer be necessary.

[cbos]

@Cirilla-zmh
Thanks for your explanation.

If the instrumentation library attaches the context (either via a propagator or via VirtualField) to a message, then you also can create an OpenTelemetry Agent Extension which can use that to retrieve the context and create a span.
In the example above to create a span with the right context for the .handleMessage() method.

It might very per messaging system, but I can imagine that received messages are read-only. And I can imagine that for example for JMS, it is only an interface in Java or Jakarta, that per implementation it might vary. There are quite a number of vendors supporting JMS.
Attaching the context to the received message as property via a propagator might work in some cases, but might fail if the message is read-only. I think Virtual Fields are a safer solution in these situations.

The PR about messaging-wrappers is mainly about creating the 'process' span, but that should then also be able to deal with the already attached receive context.

Once it is in place, also a custom Agent extension can use the messaging-wrapper to create a process span.

I think these solutions can work for situations like this.

[Cirilla-zmh]

If the instrumentation library attaches the context (either via a propagator or via VirtualField) to a message, then you also can create an OpenTelemetry Agent Extension which can use that to retrieve the context and create a span.

Sure. Messaging-wrappers are designed for those who are not familiar with OpenTelemetry Java instrumentation. It's just one option, similar to using a pre-built library instrumentation. ;)

Attaching the context to the received message as property via a propagator might work in some cases, but might fail if the message is read-only. I think Virtual Fields are a safer solution in these situations.

Yes! I think this mechanism should become a standard convention—at least for Java instrumentation.

[cbos]

@Cirilla-zmh

Another 'wild' idea:

    @WithSpan
    private void handleMessage(Session session, 
                               @ParentContextCarrier MapMessage message, 
                               MessageProducer producer) {

}

If the Context is always attached in the same way as mentioned above with a VirtualField.

Then the WithSpanInstrumentation can retrieve that context as well.

opentelemetry-java-instrumentation/instrumentation/opentelemetry-instrumentation-annotations-1.16/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationannotations/WithSpanInstrumentation.java

Line 94 in bf76c1f

Context current = AnnotationSingletons.getContextForMethod(method);

The implementation currently only checks it should reuse the inherited context or use the root context.
But based on the 'Method' object, the annotated parameters can be retrieved.
So the @ParentContextCarrier value can retrieved and the parent context can be read from the VirtualField of that object.

Based on that parentContext the new span can be created.

[Cirilla-zmh]

Based on that parentContext the new span can be created.

I agree that this is a reasonable suggestion. However, implementing such instrumentation might be kind of complex — we would need to introduce a high-level VirtualFieldHolder abstraction to make it accessible for both kafka-instrumentation and withSpan-instrumentation. While this is definitely achievable, it could add a fair amount of complexity.

Moreover, I believe this use case is fairly specific to messaging systems, and extending this behavior into a general-purpose API might not be necessary from my perspective.

Additionally, I think the purpose of messaging wrappers goes beyond just propagating trace context — they also serve to provide users with a convenient and lightweight way to instrument their 'process' logic, without having to worry too much about where exactly they should capture attributes or metrics.