Skip to content

Event externalization bootstrap should log which event types it's configured to externalize. #1130

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

Closed
odrotbohm opened this issue Mar 28, 2025 · 6 comments
Assignees
Labels
in: event publication registry Event publication registry type: improvement Minor improvements
Milestone

Comments

@odrotbohm
Copy link
Member

The analysis of this sample project here made obvious, that it's difficult to detect that events from modules not included in test runs are not subject to application module scoped integration tests. A simple log message that describes which events are selected for externalization.

We should add a bit of debug-level logging to help to diagnose such problems.

@sangjun121
Copy link
Contributor

Hi! You previously asked if I was interested in this issue, and after reading through the code, I’ve decided to give it a try

I was thinking of updating the supports(…) method in DefaultEventExternalizationConfiguration to add the logging. Does that sound like the right place?

Also, one quick question: would it be acceptable to add a logging dependency to spring-modulith-events-api for this purpose?

Thanks in advance!

@odrotbohm
Copy link
Member Author

odrotbohm commented Mar 31, 2025

I wouldn't want to issue that log over and over again on each attempt. Especially as the predicate makes it difficult to produce human-readable output why we select a particular event for externalization or not.

I think EventExternalizationAutoConfiguration.eventExternalizationConfiguration(…) would be a good place to log the configured setup, as it's the place in which we create the configuration to be applied for the rest of the application's runtime. Slf4j should be on the classpath for that module already.

Oh, I don't think we need any elaborate testing, as it's log output only.

@odrotbohm odrotbohm assigned sangjun121 and unassigned odrotbohm Mar 31, 2025
@sangjun121
Copy link
Contributor

sangjun121 commented Mar 31, 2025

I have a quick clarification regarding the debug logging for event externalization.

In the EventExternalizationAutoConfiguration.eventExternalizationConfiguration() method, it looks like we only configure the filter using EventExternalizationConfiguration.defaults(...), and the actual decision about whether an event is externalized or not happens later when the predicate is applied.

Given that, I wanted to confirm your intention:
Are you suggesting we simply log the configuration setup itself (such as base packages and applied filters), or did you mean to also log which events are actually going to be externalized?

The reason I ask is that determining the actual externalized events at this point would likely require scanning the entire classpath — which might be beyond the intended responsibility of this configuration class and could introduce unnecessary overhead.

Thanks in advance for your guidance!

@odrotbohm
Copy link
Member Author

Are you suggesting we simply log the configuration setup itself (such as base packages and applied filters), or did you mean to also log which events are actually going to be externalized?

The former.

@sangjun121
Copy link
Contributor

Thank you for the clarification!
I’ve made the necessary changes based on your feedback and opened a PR with the update.
Appreciate your time and guidance!

odrotbohm pushed a commit that referenced this issue Apr 1, 2025
odrotbohm added a commit that referenced this issue Apr 1, 2025
Simplify output to one line. Formatting, authorship.
@odrotbohm
Copy link
Member Author

That's polished and merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: event publication registry Event publication registry type: improvement Minor improvements
Projects
None yet
Development

No branches or pull requests

2 participants