Spring integration error when using MessageRequestReplyReceiverContext data in observation predicate

[brianmarting]

In what version(s) of Spring Integration are you seeing this issue?

id 'org.springframework.boot' version '3.1.1'

implementation("org.springframework.integration:spring-integration-ip")

Describe the bug

When using the ObservationPredicate with Spring boot micrometer, I created some rules in order to observe certain contexts. But when using the context MessageRequestReplyReceiverContext and using some of its data in order to create a predicate, the rest of the chain of the application just fails with the exception:

java.lang.ClassCastException: class io.micrometer.observation.Observation$Context cannot be cast to class org.springframework.integration.support.management.observation.MessageRequestReplyReceiverContext (io.micrometer.observation.Observation$Context and org.springframework.integration.support.management.observation.MessageRequestReplyReceiverContext are in unnamed module of loader 'app')

Our application flow is just a simple TCP nio inbound, with a broker outbound. But it seems to be going wrong before we receive it in the the TCP inbound.

        observationRegistry.observationConfig()
                .observationPredicate((name, context) -> shouldBeObserved(context));

Example of usage in observation predicate when using headers:

        ...
        
        if (context instanceof MessageRequestReplyReceiverContext messageRequestReplyReceiverContext) {
            return Optional.ofNullable(messageRequestReplyReceiverContext.getCarrier())
                    .map(Message::getHeaders)
                    .map(headers -> !headers.containsKey("SOMEKEY"))
                    .orElse(true);
        }

       ...

Example of usage in observation predicate when using payload:

        ...
        
        if (context instanceof MessageRequestReplyReceiverContext messageRequestReplyReceiverContext) {
            return Optional.ofNullable(messageRequestReplyReceiverContext.getCarrier())
                    .map(Message::getPayload)
                    .map(payload -> new String((byte[]) payload, StandardCharsets.UTF_8))
                    .map(payload -> !payload.equals(SOMEDATA))
                    .orElse(true);
        }

       ...

Exception:

Expected behavior

Expected to not break the application when simply casting the payload or using the headers in the generic message.

[artembilan]

I think the problem is in the Micrometer Observation.
I believe your ObservationPredicate returns false, therefore Observation.createNotStarted() returns for us a NoopObservation.
Then that observeWithContext() is called against NoopObservation.CONTEXT which is indeed not a MessageRequestReplyReceiverContext.

We probably can do some workaround in Spring Integration for this or similar API usage, but the real bug and possible fix must go to Micrometer Observation.

CC @marcingrzejszczak , @jonatan-ivanov

[artembilan]

Right. My theory is correct.
Just modified our IntegrationObservabilityZipkinTests for this code:

observationRegistry.observationConfig()
					.observationPredicate((name, context) -> !(context instanceof MessageRequestReplyReceiverContext));

and got the same exception:

Caused by: java.lang.ClassCastException: class io.micrometer.observation.Observation$Context cannot be cast to class org.springframework.integration.support.management.observation.MessageRequestReplyReceiverContext (io.micrometer.observation.Observation$Context and org.springframework.integration.support.management.observation.MessageRequestReplyReceiverContext are in unnamed module of loader 'app')
	at io.micrometer.observation.Observation.observeWithContext(Observation.java:603)
	at org.springframework.integration.gateway.MessagingGatewaySupport.sendAndReceiveWithObservation(MessagingGatewaySupport.java:602)
	at org.springframework.integration.gateway.MessagingGatewaySupport.sendAndReceive(MessagingGatewaySupport.java:553)
	at org.springframework.integration.gateway.MessagingGatewaySupport.sendAndReceiveMessage(MessagingGatewaySupport.java:532)
	at org.springframework.integration.support.management.observation.IntegrationObservabilityZipkinTests$TestMessagingGatewaySupport.process(IntegrationObservabilityZipkinTests.java:179)

Not sure what is the fix in Micrometer should be, but that observeWithContext() is broken when we are in a NoopObservation mode.

[brianmarting]

Thanks for checking, we will then just wait for the fix 😄

[artembilan]

Yeah... Only as a workaround for you right now I see as a downgrade to Spring Integration 6.0.6 where we don't use that observeWithContext().

[marcingrzejszczak]

I guess the behaviour should be that if there's a no op observation the function is not called at all, right?

[artembilan]

That's not correct, @marcingrzejszczak .
Our logic is like this:

	.<MessageRequestReplyReceiverContext, Message<?>>observeWithContext((ctx) -> {
					Message<?> replyMessage = doSendAndReceive(requestChannel, object, requestMessage);
					if (replyMessage != null) {
						ctx.setResponse(replyMessage);
					}
					return replyMessage;
				})

So, that doSendAndReceive() must be called anyway.

If there is no way to propagate a proper Context to the NoopObservation for this observeWithContext(), then it has to be deprecated and the fix I've provided in the PR for this issue is what we have to deal with from now on.

I remember that exactly me asked for this kind of fluent API to avoid a context variable extraction in our code, so my apologies for inconvenience 😄

[garyrussell]

I wonder if we could have another overloaded observation method? Something like

    default <T extends Observation.Context> Observation observation(@Nullable ObservationConvention<T> customConvention,
            ObservationConvention<T> defaultConvention, Supplier<T> contextSupplier, ObservationRegistry registry
            T noopContext) {

Where the noopContext is used by the NoopOpservation (but I see it will need adding a generic parameter to NoopObservation).

??

[garyrussell]

but I see it will need adding a generic parameter to NoopObservation

Actually, no, the cast is avoided even if stored in the Context field.

[artembilan]

Yeah... The point is that NoopObservation is a constant and current architecture is not aimed to modify it with our requested context.

Perhaps observeWithContext() may provide a null for context when NoopObservation, but that would expose a required knowledge about observation API internals with such a changed state.
This way I think more and more that better to deprecate this observeWithContext().

Not sure, though, how everything else works since it just delegates to this method anyway:

    default void observe(Runnable runnable) {
        observeWithContext((context) -> {
            runnable.run();
            return null; // returned value is ignored
        });
    }

[artembilan]

Looks like there is no the fix in Micrometer Observation yet, so we have to go for proposed workaround for our upcoming releases.

Thanks