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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
