You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "AzazelSensei (via GitHub)" <gi...@apache.org> on 2026/08/15 08:46:35 UTC
[PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
AzazelSensei opened a new pull request, #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252
`Log4j2EventListener` is also created from `spring.factories`, so `@ConditionalOnProperty` never applies and `spring.cloud.config.watch.enabled=false` does nothing.
I check that property in `onApplicationEvent` (via `EnvironmentAware` or the event source context). Explicit `false` skips the reload. Missing or true keeps the old behavior.
Fixes #4244
## Checklist
* Base your changes on `2.x` branch if you are targeting Log4j 2; use `main` otherwise
* `./mvnw verify` succeeds ([the build instructions](https://logging.apache.org/log4j/2.x/development.html#building))
* Non-trivial changes contain an entry file in the `src/changelog/.2.x.x` directory
* Tests are provided
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252#issuecomment-5714567536
@AzazelSensei thanks, all four points are in and the test fails without the listener change. `spotless:check` still fails on the `new Log4j2EventListener()` call in `Log4j2EventListenerTest.java`. Can you run `./mvnw spotless:apply` and push?
Please check the `Building.adoc` its has all the steps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252#issuecomment-5438072349
`log4j-spring-cloud-config-client` uses JUnit 4 and, sadly, `pom.xml` is missing the `org.junit.vintage:junit-vintage-engine` test dependency. @ramanathan1504, can you take care of this one, please?
Maybe in a follow-up PR, we can migrate `log4j-spring-cloud-config-client` to JUnit 5. Though I'd not be surprised if it turns out to be a tough job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252#discussion_r4028998925
##########
log4j-spring-cloud-config-client/src/main/java/org/apache/logging/log4j/spring/cloud/config/client/Log4j2EventListener.java:
##########
@@ -20,17 +20,48 @@
import org.apache.logging.log4j.Logger;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.cloud.context.environment.EnvironmentChangeEvent;
+import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationListener;
+import org.springframework.context.EnvironmentAware;
+import org.springframework.core.env.Environment;
import org.springframework.stereotype.Component;
@Component
@ConditionalOnProperty(value = "spring.cloud.config.watch.enabled")
-public class Log4j2EventListener implements ApplicationListener<EnvironmentChangeEvent> {
+public class Log4j2EventListener implements ApplicationListener<EnvironmentChangeEvent>, EnvironmentAware {
Review Comment:
`setEnvironment` is never called on the instance from `spring.factories`, and nothing scans this package for `@Component`. Can `EnvironmentAware` and the field go, so only the event source check is left?
##########
src/changelog/.2.x.x/4244_honor_watch_enabled_on_event_listener.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="4244" link="https://github.com/apache/logging-log4j2/issues/4244"/>
Review Comment:
Add pr details like issue tag
##########
log4j-spring-cloud-config-client/src/main/java/org/apache/logging/log4j/spring/cloud/config/client/Log4j2EventListener.java:
##########
@@ -20,17 +20,48 @@
import org.apache.logging.log4j.Logger;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.cloud.context.environment.EnvironmentChangeEvent;
+import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationListener;
+import org.springframework.context.EnvironmentAware;
+import org.springframework.core.env.Environment;
import org.springframework.stereotype.Component;
@Component
@ConditionalOnProperty(value = "spring.cloud.config.watch.enabled")
-public class Log4j2EventListener implements ApplicationListener<EnvironmentChangeEvent> {
+public class Log4j2EventListener implements ApplicationListener<EnvironmentChangeEvent>, EnvironmentAware {
private static Logger LOGGER = LogManager.getLogger(Log4j2EventListener.class);
+ private Environment environment;
+
+ @Override
+ public void setEnvironment(final Environment environment) {
+ this.environment = environment;
+ }
@Override
public void onApplicationEvent(final EnvironmentChangeEvent environmentChangeEvent) {
+ if (!isWatchEnabled(environmentChangeEvent)) {
+ LOGGER.debug("Ignoring environment change event; spring.cloud.config.watch.enabled is false");
+ return;
+ }
LOGGER.debug("Application change event triggered");
WatchEventManager.publishEvent();
}
+
+ /**
+ * {@code spring.factories} constructs this listener outside the bean factory, so
+ * {@code @ConditionalOnProperty} never applies. Honor the same property here.
+ */
+ private boolean isWatchEnabled(final EnvironmentChangeEvent event) {
+ Environment env = this.environment;
+ if (env == null) {
+ final Object source = event.getSource();
+ if (source instanceof ApplicationContext) {
+ env = ((ApplicationContext) source).getEnvironment();
+ }
+ }
+ if (env == null) {
+ return true;
+ }
+ return !Boolean.FALSE.equals(env.getProperty("spring.cloud.config.watch.enabled", Boolean.class));
Review Comment:
`getProperty` with `Boolean.class` throws `ConversionFailedException` for a value like `maybe`, and that fails `/actuator/refresh`. This matches `@ConditionalOnProperty`, which never throws.
```suggestion
return !"false".equalsIgnoreCase(env.getProperty("spring.cloud.config.watch.enabled"));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252#issuecomment-5617156096
This `PR` Should land after #4317 because the module has run no tests since removed `junit-vintage-engine` while `Log4j2EventListenerTest stayed` on JUnit 4.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
Posted by "AzazelSensei (via GitHub)" <gi...@apache.org>.
AzazelSensei commented on PR #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252#issuecomment-5702325157
Dropped EnvironmentAware and the field. The check only uses the event source now, and I compare the property as a string so a value like maybe does not throw on /actuator/refresh. Changelog has the PR tag. doesNotReloadWhenWatchDisabled uses watchManager() so it compiles after the 2.x merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Honor spring.cloud.config.watch.enabled on Log4j2EventListener [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 merged PR #4252:
URL: https://github.com/apache/logging-log4j2/pull/4252
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org