Spring Integration library instrumentation

[mateuszrzeszutek]

Spring-integration is based on spring-messaging and serves as a base for spring-cloud-stream (and probably many other spring libs). It's basically an implementation of the Enterprise Integration Patterns idea (like Camel).
The central interface in spring-messaging/integration is MessageChannel - it connects different components/pieces of logic/cloud applications (e.g. connects RabbitMQ listener to underlying impl agnostic spring MessageHandler). Fortunately Spring provides a ChannelInterceptor interface that allows us to inject some code into various phases of MessageChannel lifecycle.

Initially I wanted to implement just the context propagation without any tracing (spans would be created by e.g. RabbitMQ or Kafka instrumentations), but once I started working on that and dug into the spring code I realized that I'll have to create at least one span that'll hold a link to the context received from a remote message. It also became apparent to me that instrumenting just the send() method is enough and receive() interceptor callbacks won't propagate anything.
I think that this instrumentation is probably mostly correct (it preserves and propagates the context), but I haven't tested it with a real spring-cloud-stream scenario -- will do that in one of the next PRs.

This PR includes a new addition to the Instrumenter API: the SpanLinkExtractor.

[iNikem]

I don't understand why we don't do anything on receive? Who is going to "restore" the context that we propagated?

[trask]

👍

[trask]

same question about beforeHandle and afterMessageHandled

[mateuszrzeszutek]

Spring Javadocs:

Invoked inside the Runnable submitted to the Executor after calling the target MessageHandler regardless of the outcome (i.e. Exception raised or not) thus allowing for proper resource cleanup.

In addition to that, these two methods actually seem to be sort of deprecated in spring integration/cloud-stream, as they're only used once in one MessageChannel implementation from spring-messaging that is not used anywhere in the spring code.

[mateuszrzeszutek]

I don't understand why we don't do anything on receive? Who is going to "restore" the context that we propagated?

One reason why I didn't touch the receive interceptor methods is because they all execute before receive() returns, and thus cannot set context. receive() is also only used in PollableChannels, never in SubscribableChannels.

It's a bit unintuitive, but the way spring-integration works is send() instrumentation both extracts the context from the message and injects the new one (with the newly created span) into it - this way if this channel calls MessageHandlers directly on send() (like DirectChannel does) handlers will see the current context based on the one extracted from the message; if the channel transmits the message somewhere else (an external queue, like Kafka) there will always be another MessageChannel whose send() will be called by that "somewhere else" (a Kafka consumer, for example).

[iNikem]

Why is this a link and not a parent? E.g. in Kafka instrumentation we use context from the message as a parent. And I believe that current semantic conventions also dictate this.

[mateuszrzeszutek]

Current messaging semantic conventions? Spring messaging/integration is not actually a messaging library (contrary to its name), so you can't really compare it to Kafka - you can have multiple MessageChannels configured to send messages to each other without any sort of message queue involved (and, as far as I understand it, most of spring-integration projects look like that).

I used the messaging convention as a base for this insturmentation though, particularly the batch scenarios: in both of them there's a separate CONSUMER span for receving that has no parent and separate CONSUMER span for processing that links to the PRODUCER extracted from the message. I though that this is a bit similar to the spring integration scenario, where a span might have two parents: one "local", one taken from the incoming message.

Also, I'm planning to make this behavior configurable: there'll be an option to always set the context extracted from the message as the parent, and optionally link the "local" one if it contains a different span.

[mateuszrzeszutek]

@anuraaga @trask What do you think about it?

Short version: right now the parent of the MessageChannel is always the "local" current span, and whatever comes with the message is added as a link. When there's no local span only link is added.
Another option is to always use the span context extracted from the message (and fall back to Context.current() if there's none).
Which one does make more sense as the default behavior? Is there any other option?

[iNikem]

We discussed this today during "special topics" meeting and it seems that using propagated context as parent makes more sense than using current or implicit context. When you use message listener (and in case of SubscribableChannel that is what you do, right), then your code will be called by the framework code. And you usually don't care about framework loops. So it still makes sense to have PRODUCER span as your parent. You have only "processing" part, not "receiving" one.

[mateuszrzeszutek]

👍

[mateuszrzeszutek]

Done - thanks for the discussion!