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

Add toString hook for GenericMessage to let end-users obfuscate non-public information in logs #24727

Open
artembilan opened this issue Mar 18, 2020 · 6 comments
Assignees
Labels
in: messaging Issues in messaging modules (jms, messaging) status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement
Milestone

Comments

@artembilan
Copy link
Member

See spring-projects/spring-integration#3222 for more info.

An idea to have something like GenericMessage .setToStringFunction(Function<Message<?>, String>) which is going to be called from the Message.toString().
So, end-user can provide any possible way for presenting message in logs.

Thanks

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 18, 2020
@sbrannen sbrannen changed the title Add toString hook for GenericMessage to let end-users to obfuscate non public information in logs Add toString hook for GenericMessage to let end-users obfuscate non-public information in logs Mar 19, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 19, 2020

We something similar in MessageHeaderAccessor which given a Payload prints headers with payload as text if the mime type allows it, possibly truncating to 80 chars. This is in getShortLogMessage and getDetailedLogMessage and used for the STOMP support which relies on MessageHeaderAccessor which extends MessageHeaders and is typically available throughout.

One concern is avoiding per message object creation, which could certainly happen with a lambda friendly method signature.

Another thought is the need to customize the message depending on the context such as a log level given a specific log category, or specific location in the code. It seems that such a component wouldn't have all the context to make a decision and that the framework should still be able to say, format with all details, or format something more minimal.

Another alternative would be to applying formatting externally wherever messages are logged which is of course more work. Yet another idea is the ability to specify sensitive headers on GenericMessage and excluding those from toString.

@rstoyanchev rstoyanchev added the in: messaging Issues in messaging modules (jms, messaging) label Mar 19, 2020
@rstoyanchev rstoyanchev self-assigned this Mar 19, 2020
@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Mar 19, 2020
@artembilan
Copy link
Member Author

Thanks, @rstoyanchev , for feedback!

Well, I know that we have those getShortLogMessage and getDetailedLogMessage, which really don't help here too much. Since we still may produce a sensible info into logs. More over it is going to be a lot of work to revise the whole framework and its logging hooks to make a decision to log the whole message or not.

Also, as you pointed that out, it really becomes unreachable goal to make a decision on the log level without know a target application context.

You concern about "per message object creation" is slightly not critical since our idea is to allow end-user to configure an AbstractIntegrationMessageBuilder with such a single lambda or another more complex end-user logic to format this or that message.

We may also introduce some sensitive header to identify if message should go through the formatting function or not, but that's again should not be just against headers: the payload may have some NPI as well...

Another option, if you still against setToStringFunction(), of course, do nothing and recommend to turn off logging for the framework (ERROR should be enough) and log messages only from the target project applying its formatting requirements.
In fact Spring Integration provides a LoggingHandler with a setLogExpression option where you can apply whatever is valid for your application: https://docs.spring.io/spring-integration/docs/current/reference/html/messaging-endpoints.html#logging-channel-adapter.

I'm really up to disabling logging for the framework and let target project to decide what and how to log...
I've just raised this to see other opinion.

Thanks

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 19, 2020
@garyrussell
Copy link
Contributor

Another problem is that most exceptions are MessagingException with failedMessage which is toString()d in printStackTrace().

Surely we don't want to suppress logging of exceptions?

@rstoyanchev
Copy link
Contributor

I didn't mean to suggest that get[Short|Long]Message can help but rather thinking in the other direction. If we do introduce a message formatting mechanism, how does it relate to the get[Short|Long]Message methods, and can it replace them?

I'm not against doing something here, but a Function is limiting because it cannot vary the formatting depending on the context (e.g. debug vs trace). I was thinking some sort of MessageFormatter that accepts input about which headers to filter in, and maybe a something also to determine if the payload can be displayed as text. Then it would provide methods to format the message depending on the log level.

This can then be used to log a Message according to the log level, and it can be used in toString() for MessagingException logging.

@artembilan
Copy link
Member Author

Thanks, @rstoyanchev , for your discernment!

I don't the whole tool yet (that MessageFormatter), but I feel like it is going to be a big change for Spring Integration to use it everywhere we log Message unlike a single function injected into every message from a builder... But yeah, we don't insist yet. It might be even the fact that we leave whatever we have in the framework with recommendation to turn off logging for it and let end-users to use that new instrument when they do their logging with messages.

We probably can discuss implementation details later, when we have already something in hands.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 31, 2020

i think the original proposal and my suggestion aren't necessarily mutually exclusive. Such a formatter could be set on, and obtained from a Message, and it could be integrated into the toString() implementation of the message. But fair enough that a concrete POC would be make it easier to discuss further.

@rstoyanchev rstoyanchev added this to the 5.x Backlog milestone Sep 7, 2021
@rstoyanchev rstoyanchev added status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Sep 7, 2021
@jhoeller jhoeller modified the milestones: 6.x Backlog, 6.0.x Nov 1, 2021
@rstoyanchev rstoyanchev modified the milestones: 6.0.x, 6.1.x Dec 9, 2022
@jhoeller jhoeller modified the milestones: 6.1.x, 6.x Backlog Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants