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 logging message if logging isn't enabled for AbstractReplyProducingMessageHandler where no reply is required #9705

Closed
mitchmcd18 opened this issue Dec 9, 2024 · 2 comments · Fixed by #9707

Comments

@mitchmcd18
Copy link
Contributor

mitchmcd18 commented Dec 9, 2024

Expected Behavior

Currently when any class that implements AbstractReplyProducingMessageHandler doesn't produce a reply it will log the message, even if no reply is required. The message should only be logged if isLoggingEnabled() returns true as a handler that doesn't require a reply may be a normal operation that is expected, and if we've set loggingEnabled to false it shouldn't log the message.

Current Behavior

Currently if no reply is returned and is not required it will always produce a debug level log containing the full message.

Context

The line which logs the message is contained here:
https://github.com/spring-projects/spring-integration/blob/main/spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractReplyProducingMessageHandler.java#L159

I would imagine that it should be similar to this line where we first check if logging is enabled before logging
https://github.com/spring-projects/spring-integration/blob/main/spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractMessageHandler.java#L61

Our requirement for this is that in some cases a Splitter may not always produce an output (as the original payload it received contained a list that was empty). We have strict requirements regarding logging of the original message we received, no standard output should contain the original message and we've already made use of the loggingEnabled field to avoid logging the message in most cases. A potential workaround is to have the handler return something and have a downstream handler discard the message, however this seems a bit unnecessary if there is already a capability to avoid logging in an expected operation.

I am happy to make the change if needed.

@mitchmcd18 mitchmcd18 added status: waiting-for-triage The issue need to be evaluated and its future decided type: enhancement labels Dec 9, 2024
@artembilan
Copy link
Member

Sure!
Totally makes sense.
Feel free to contribute the fix: https://github.com/spring-projects/spring-integration/blob/main/CONTRIBUTING.adoc.

At the same time, I wonder why would one enable debug logging for org.springframework.integration category, if logging is so sensitive from your requirements.

Thanks

@artembilan artembilan modified the milestones: 6.5.0-M1, 6.4.1 Dec 9, 2024
@artembilan artembilan added in: core for: backport-to-6.3.x and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Dec 9, 2024
@mitchmcd18
Copy link
Contributor Author

Sure!

Do agree, I could disable it for those cases but also wouldn't want to restrict the ability to enable more fine-grained logging there if we needed to debug some issues.

Will look at raising a PR soon!

spring-builds pushed a commit that referenced this issue Dec 10, 2024
Fixes: #9705
Issue link: #9705

Currently when any class that implements `AbstractReplyProducingMessageHandler` doesn't produce a reply it will log the message, even if no reply is required.
The message should only be logged if `isLoggingEnabled()` returns true as a handler that doesn't require a reply may be a normal operation that is expected, and if we've set `loggingEnabled` to false it shouldn't log the message.

* Prevent logging when no reply is provided for an `AbstractReplyProducingMessageHandler` if logging is disabled

(cherry picked from commit 7fc104a)
pull bot pushed a commit to 86dh/spring-integration that referenced this issue Dec 10, 2024
…or logging enabled

Fixes: spring-projects#9705
Issue link: spring-projects#9705

Currently when any class that implements `AbstractReplyProducingMessageHandler` doesn't produce a reply it will log the message, even if no reply is required. 
The message should only be logged if `isLoggingEnabled()` returns true as a handler that doesn't require a reply may be a normal operation that is expected, and if we've set `loggingEnabled` to false it shouldn't log the message.

* Prevent logging when no reply is provided for an `AbstractReplyProducingMessageHandler` if logging is disabled

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