Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ErrorMessages are mangled by observed message channels #9198

Closed
SkylerArnold-Corista opened this issue May 31, 2024 · 1 comment
Closed

ErrorMessages are mangled by observed message channels #9198

SkylerArnold-Corista opened this issue May 31, 2024 · 1 comment

Comments

@SkylerArnold-Corista
Copy link

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

6.3.0

Describe the bug

When micrometer observation is enabled for a MessageChannel that's being used for error handling, the ErrorMessages that are sent through the channel are silently converted into MutableMessages, and the originalMessage is lost.

From the example repo linked below:

    @Bean
    public IntegrationFlow integrationFlow() {
        return IntegrationFlow.fromSupplier(() -> new GenericMessage<>(UUID.randomUUID().toString()), c -> c.poller(p -> p
                        .fixedDelay(3000)
                        .errorChannel("errorChannel")  // THE PROBLEMATIC BIT
                        .taskExecutor(exampleTaskExecutor())
                ))
                .handle(m -> {
                    throw new RuntimeException();
                })
                .get();
    }

If I set up a service activator to handle the ErrorMessages published to errorChannel, it can't actually receive ErrorMessages and instead it only gets MutableMessage<Throwable>s.

This is an issue in my real project because my error handling logic needs access to the original message (from ErrorMessage::getOriginalMessage), which isn't possible if the message gets converted into a MutableMessage.

I've done a bit of digging and it seems this is happening in AbstractMessageChannel::sendWithObservation and MutableMessage::of. I assume this is an issue any time you send a non-mutable message type through an observed channel & expect it to come out the other side as the same type, not just for my usecase.

Workaround

This is similar to this issue that was raised in the Sleuth project a while ago. Unfortunately, the recommended workaround for that situation doesn't work here, because the interceptors are applied to the message after it's already been converted to a MutableMessage. I also tried to use a custom ErrorMessageStrategy to wrap the ErrorMessage in a MutableMessage before it gets sent from the MessagePublishingErrorHandler, but sadly the ErrorMessageStrategy can only return ErrorMessage types, so that didn't work.

Instead, I'm currently just disabling observation for my error handling channel all together, which has the unfortunate effect of causing the traceId of the logs for the error handling code not to line up with the traceId of the rest of the IntegrationFlow.

Sample

Here's a small example that reproduces the problem. If you run the project you'll see a lot of ugly stack traces, instead of the expected Error from [...] logs that should be happening. If you uncomment the !errorChannel observation pattern, the problem goes away.

@SkylerArnold-Corista SkylerArnold-Corista added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels May 31, 2024
@artembilan artembilan added this to the 6.4.0-M1 milestone Jun 3, 2024
@artembilan artembilan added in: core (EOL) for: backport-to-6.2.x for: backport-to-6.3.x and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jun 3, 2024
@artembilan
Copy link
Member

Yeah... I see the problem: didn't think about tracing on error channels.
Will fix it shortly.
As a workaround I can suggest only an extension of the ErrorMessage where you would put originalMessage into headers.
This way, even if it is swwapped to the MutableMessage in the tracing channel, that info is still going to be available in the respective custom header.

spring-builds pushed a commit that referenced this issue Jun 3, 2024
Fixes: #9198

When observation is enabled on the `MessageChannel`, the message to send is converted to a `MutableMessage`.
In case of `ErrorMessage` this causes a loss of `originalMessage` and may lead to `ClassCastException`
in the target error handler.

* Check for `ErrorMessage` in the `AbstractMessageChannel.sendWithObservation()` and create a new one
as a copy of original request, but including observation headers before performing `sendInternal()`
* Modify `IntegrationObservabilityZipkinTests` to verify an observation with an `ErrorMessage` and its handler.

(cherry picked from commit b0cbacf)
spring-builds pushed a commit that referenced this issue Jun 3, 2024
Fixes: #9198

When observation is enabled on the `MessageChannel`, the message to send is converted to a `MutableMessage`.
In case of `ErrorMessage` this causes a loss of `originalMessage` and may lead to `ClassCastException`
in the target error handler.

* Check for `ErrorMessage` in the `AbstractMessageChannel.sendWithObservation()` and create a new one
as a copy of original request, but including observation headers before performing `sendInternal()`
* Modify `IntegrationObservabilityZipkinTests` to verify an observation with an `ErrorMessage` and its handler.

(cherry picked from commit b0cbacf)
EddieChoCho pushed a commit to EddieChoCho/spring-integration that referenced this issue Jun 26, 2024
…ssage`

Fixes: spring-projects#9198

When observation is enabled on the `MessageChannel`, the message to send is converted to a `MutableMessage`.
In case of `ErrorMessage` this causes a loss of `originalMessage` and may lead to `ClassCastException`
in the target error handler.

* Check for `ErrorMessage` in the `AbstractMessageChannel.sendWithObservation()` and create a new one
as a copy of original request, but including observation headers before performing `sendInternal()`
* Modify `IntegrationObservabilityZipkinTests` to verify an observation with an `ErrorMessage` and its handler.

**Auto-cherry-pick to `6.3.x` & `6.2.x`**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants