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

Avoid duplicate error logging when multiple destinations per binding #2998

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

hightea
Copy link
Contributor

@hightea hightea commented Sep 3, 2024

When using multiple destinations for one binding, the errors are logged as many times as the number of destinations.
For an example, when an error occurs in the following binding :

spring.cloud.stream.bindings.process-in-0.consumer.max-attempts=1
spring.cloud.stream.bindings.process-in-0.destination=one,two,three

This results in three successive logs from the org.springframework.integration.handler.LoggingHandler

This pull request fixes this by bridging only once the bindingErrorHandler whith the globalErrorHandler

@olegz
Copy link
Contributor

olegz commented Sep 3, 2024

I like the PR, but I wonder if we should assume that some destination will get successful delivery, a single log could be confusing. What do you think exposing your code via some boolean flag? I can even agree that the default path wil be to a single log, but having a flag would allow users to change back to the original behavior.
Let me know what you think. . .

@hightea
Copy link
Contributor Author

hightea commented Sep 3, 2024

This is only for consumers (registerErrorInfrastructure(ConsumerDestination, ...)).
Every consumer for the binding (one per "destination") share the same binderErrorChannel. With the current behavior, every error is logged as many times as the number of destinations, but in our case the "destinations" are inputs so a message originates from one destination only, and when an error occurs, it's logged several times.
Note that the logs are emmitted successively without any intermediate processing

However, I can add a flag if you think it's a better option, but I don't see any case where we'll want multiple logs.

@olegz
Copy link
Contributor

olegz commented Sep 3, 2024

I see it now. I'll merge.

@hightea
Copy link
Contributor Author

hightea commented Sep 3, 2024

Thank you !

@hightea
Copy link
Contributor Author

hightea commented Sep 5, 2024

FYI : I've rebased my branch on master to make the tests pass

@olegz olegz merged commit f456405 into spring-cloud:main Sep 5, 2024
1 check passed
@hightea hightea deleted the fix-duplicate-errors-logs branch September 5, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants