-
Notifications
You must be signed in to change notification settings - Fork 26.6k
Support Preferred Network Interface via Spring Environment and Fix Early Host Resolution #15604
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
Conversation
…face config from Spring Environment - Removed local host info logging from FailsafeLogger to prevent premature NetUtils host resolution. This issue occurs because WelcomeLogoApplicationListener logs messages before Spring configuration is fully loaded, causing NetUtils to cache incorrect host data and ignore preferred interface settings. - Introduced DubboNetInterfaceConfigApplicationListener to extract `dubbo.network.interface.preferred` from the Spring Environment and inject it into system properties. This listener uses ApplicationContextInitializedEvent to ensure the configuration (including from config centers) is available before NetUtils is triggered.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 3.3 #15604 +/- ##
============================================
+ Coverage 61.06% 61.09% +0.02%
- Complexity 11693 11694 +1
============================================
Files 1909 1910 +1
Lines 86783 86800 +17
Branches 13094 13098 +4
============================================
+ Hits 52994 53030 +36
+ Misses 28373 28356 -17
+ Partials 5416 5414 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| private String appendContextMessage(String msg) { | ||
| return " [DUBBO] " + msg + ", dubbo version: " + Version.getVersion() + ", current host: " | ||
| + NetUtils.getLocalHost(); | ||
| return " [DUBBO] " + msg + ", dubbo version: " + Version.getVersion(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if apply this pr, the current host information will be logged at where? could you provide your log information about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, if the printing of this host information is not removed, it will cause premature caching of the network interface card (NIC) that should be used in WelcomeLogoApplicationListener. The execution timing of WelcomeLogoApplicationListener may occur before Spring loads the configuration files, resulting in the NIC configuration specified in the configuration files failing to take effect in NetUtils.
Therefore, if host information logging is mandatory:
- Could the execution of
WelcomeLogoApplicationListenerbe deferred? - Alternatively, would it be acceptable to omit logging the host information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the host information should be logged for existed exception checking apps, you'd better log it at other location instead of omitting it directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the execution timing of WelcomeLogoApplicationListener and re-enabled the logging of local host information.
…the ApplicationEnvironmentPreparedEvent phase to the ApplicationContextInitializedEvent phase. It is now triggered after DubboNetInterfaceConfigApplicationListener to ensure proper network interface configuration is applied. Additionally, restored the host information appending behavior in FailsafeLogger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes premature host resolution from FailsafeLogger and introduces proper network interface configuration support through Spring Environment. The change addresses an issue where network interface configuration was being evaluated too early in the application startup process, preventing user-defined configuration from taking effect.
- Introduces
DubboNetInterfaceConfigApplicationListenerto handle network interface configuration from Spring Environment - Changes
WelcomeLogoApplicationListenerto useApplicationContextInitializedEventinstead ofApplicationEnvironmentPreparedEvent - Registers the new listener in Spring's auto-configuration
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
DubboNetInterfaceConfigApplicationListener.java |
New listener that reads network interface configuration from Spring Environment and sets system properties |
spring.factories |
Registers the new network interface configuration listener |
WelcomeLogoApplicationListener.java |
Changes event type to ensure proper timing after Spring Environment is fully initialized |
DubboNetInterfaceConfigApplicationListenerTest.java |
Test coverage for the new network interface configuration functionality |
Comments suppressed due to low confidence (2)
dubbo-spring-boot-project/dubbo-spring-boot/src/test/java/org/apache/dubbo/spring/boot/context/event/DubboNetInterfaceConfigApplicationListenerTest.java:64
- The test uses a custom listener instead of testing the actual
DubboNetInterfaceConfigApplicationListener. This doesn't verify that the real implementation works correctly with theApplicationContextInitializedEvent.
static class NetworkInterfaceApplicationListener
dubbo-spring-boot-project/dubbo-spring-boot/src/test/java/org/apache/dubbo/spring/boot/context/event/DubboNetInterfaceConfigApplicationListenerTest.java:65
- The test listener uses
ApplicationEnvironmentPreparedEventwhile the actual implementation usesApplicationContextInitializedEvent. This mismatch means the test doesn't verify the correct event handling timing.
implements ApplicationListener<ApplicationEnvironmentPreparedEvent> {
RainYuY
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…rly Host Resolution (apache#15604)
This change removes the logic in FailsafeLogger that appends the local host information to log messages. The motivation is that FailsafeLogger is used by the WelcomeLogoApplicationListener to print logs very early during application startup—before Spring’s configuration files are fully loaded. As a result, NetUtils resolves and caches the local host information prematurely. This prevents the application from re-evaluating the preferred network interface later based on user-defined configuration. To avoid this issue, the host resolution in FailsafeLogger has been removed.
A new class, DubboNetInterfaceConfigApplicationListener, has been introduced to retrieve the preferred network interface configuration from Spring’s Environment and set it as a system property—since NetUtils reads this value from environment variables. This listener listens to the ApplicationContextInitializedEvent, which ensures that the Spring Environment (including values from configuration centers) is fully prepared when the listener executes.