-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add option to provide URIs to monitor in addition to the config file #3501
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
base: 2.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to you under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.logging.log4j.core; | ||
|
||
import static org.awaitility.Awaitility.waitAtMost; | ||
|
||
import java.io.IOException; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.util.Collections; | ||
import java.util.concurrent.TimeUnit; | ||
import org.apache.logging.log4j.core.config.Configuration; | ||
import org.apache.logging.log4j.core.config.ConfigurationSource; | ||
import org.apache.logging.log4j.core.config.Configurator; | ||
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder; | ||
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory; | ||
import org.apache.logging.log4j.core.config.properties.PropertiesConfiguration; | ||
import org.apache.logging.log4j.core.util.Source; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.io.CleanupMode; | ||
import org.junit.jupiter.api.io.TempDir; | ||
|
||
public class MonitorUriTest { | ||
|
||
private static final int MONITOR_INTERVAL = 3; | ||
|
||
@Test | ||
void testReconfigureOnChangeInMonitorUri(@TempDir(cleanup = CleanupMode.ON_SUCCESS) final Path tempDir) | ||
throws IOException { | ||
ConfigurationBuilder<PropertiesConfiguration> configBuilder = | ||
ConfigurationBuilderFactory.newConfigurationBuilder(PropertiesConfiguration.class); | ||
Path config = tempDir.resolve("config.xml"); | ||
Path monitorUri = tempDir.resolve("monitorUri.xml"); | ||
ConfigurationSource configSource = new ConfigurationSource(new Source(config), new byte[] {}, 0); | ||
Configuration configuration = configBuilder | ||
.setConfigurationSource(configSource) | ||
.setMonitorInterval(String.valueOf(MONITOR_INTERVAL)) | ||
.add(configBuilder.newMonitorUri(monitorUri.toUri().toString())) | ||
.build(); | ||
|
||
try (LoggerContext loggerContext = Configurator.initialize(configuration)) { | ||
Files.write(monitorUri, Collections.singletonList("a change")); | ||
waitAtMost(MONITOR_INTERVAL + 2, TimeUnit.SECONDS) | ||
.until(() -> loggerContext.getConfiguration() != configuration); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
import java.io.InputStream; | ||
import java.lang.ref.WeakReference; | ||
import java.net.URI; | ||
import java.net.URISyntaxException; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collection; | ||
|
@@ -132,6 +133,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement | |
private ConcurrentMap<String, Appender> appenders = new ConcurrentHashMap<>(); | ||
private ConcurrentMap<String, LoggerConfig> loggerConfigs = new ConcurrentHashMap<>(); | ||
private List<CustomLevelConfig> customLevels = Collections.emptyList(); | ||
private List<URI> uris = Collections.emptyList(); | ||
private final ConcurrentMap<String, String> propertyMap = new ConcurrentHashMap<>(); | ||
private final Interpolator tempLookup = new Interpolator(propertyMap); | ||
private final StrSubstitutor runtimeStrSubstitutor = new RuntimeStrSubstitutor(tempLookup); | ||
|
@@ -323,10 +325,19 @@ public void start() { | |
LOGGER.info("Starting configuration {}...", this); | ||
this.setStarting(); | ||
if (watchManager.getIntervalSeconds() >= 0) { | ||
LOGGER.info( | ||
"Start watching for changes to {} every {} seconds", | ||
getConfigurationSource(), | ||
watchManager.getIntervalSeconds()); | ||
if (uris != null && uris.size() > 0) { | ||
LOGGER.info( | ||
"Start watching for changes to {} and {} every {} seconds", | ||
getConfigurationSource(), | ||
uris, | ||
watchManager.getIntervalSeconds()); | ||
watchMonitorUris(); | ||
} else { | ||
LOGGER.info( | ||
"Start watching for changes to {} every {} seconds", | ||
getConfigurationSource(), | ||
watchManager.getIntervalSeconds()); | ||
} | ||
watchManager.start(); | ||
} | ||
if (hasAsyncLoggers()) { | ||
|
@@ -347,6 +358,17 @@ public void start() { | |
LOGGER.info("Configuration {} started.", this); | ||
} | ||
|
||
private void watchMonitorUris() { | ||
if (this instanceof Reconfigurable) { | ||
uris.stream().forEach(uri -> { | ||
Source source = new Source(uri); | ||
final ConfigurationFileWatcher watcher = new ConfigurationFileWatcher( | ||
this, (Reconfigurable) this, listeners, source.getFile().lastModified()); | ||
watchManager.watch(source, watcher); | ||
}); | ||
} | ||
} | ||
|
||
private boolean hasAsyncLoggers() { | ||
if (root instanceof AsyncLoggerConfig) { | ||
return true; | ||
|
@@ -729,9 +751,16 @@ protected void doConfigure() { | |
} else if (child.isInstanceOf(AsyncWaitStrategyFactoryConfig.class)) { | ||
final AsyncWaitStrategyFactoryConfig awsfc = child.getObject(AsyncWaitStrategyFactoryConfig.class); | ||
asyncWaitStrategyFactory = awsfc.createWaitStrategyFactory(); | ||
} else if (child.isInstanceOf(MonitorUris.class)) { | ||
uris = convertToJavaNetUris(child.getObject(MonitorUris.class).getUris()); | ||
} else { | ||
final List<String> expected = Arrays.asList( | ||
"\"Appenders\"", "\"Loggers\"", "\"Properties\"", "\"Scripts\"", "\"CustomLevels\""); | ||
"\"Appenders\"", | ||
"\"Loggers\"", | ||
"\"Properties\"", | ||
"\"Scripts\"", | ||
"\"CustomLevels\"", | ||
"\"MonitorUris\""); | ||
LOGGER.error( | ||
"Unknown object \"{}\" of type {} is ignored: try nesting it inside one of: {}.", | ||
child.getName(), | ||
|
@@ -767,6 +796,18 @@ protected void doConfigure() { | |
setParents(); | ||
} | ||
|
||
private List<URI> convertToJavaNetUris(final List<Uri> uris) { | ||
final List<URI> javaNetUris = new ArrayList<>(); | ||
for (Uri uri : uris) { | ||
try { | ||
javaNetUris.add(new URI(uri.getUri())); | ||
} catch (URISyntaxException e) { | ||
LOGGER.error("Error parsing monitor URI: " + uri, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure we want to proceed with (re)configuration obtained from an invalid configuration? I think we should propagate the exception and let it fail. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thinking here was to be consistent with other error scenarios, for example lines 726, 764, 790. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See this recent discussion on how to handle configuration failures. In short, configuration failures should result in the entire configuration to fail, instead of applying in partially. Existing code that doesn't adhere to this needs to be fixed, but that is another issue. |
||
} | ||
} | ||
return javaNetUris; | ||
} | ||
|
||
public static Level getDefaultLevel() { | ||
final String levelName = PropertiesUtil.getProperties() | ||
.getStringProperty(DefaultConfiguration.DEFAULT_LEVEL, Level.ERROR.name()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to you under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.logging.log4j.core.config; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import org.apache.logging.log4j.core.Core; | ||
import org.apache.logging.log4j.core.config.plugins.Plugin; | ||
import org.apache.logging.log4j.core.config.plugins.PluginElement; | ||
import org.apache.logging.log4j.core.config.plugins.PluginFactory; | ||
|
||
/** | ||
* Container for MonitorUri objects. | ||
*/ | ||
@Plugin(name = "MonitorUris", category = Core.CATEGORY_NAME, printObject = true) | ||
public final class MonitorUris { | ||
|
||
private final List<Uri> uris; | ||
|
||
private MonitorUris(final Uri[] uris) { | ||
this.uris = new ArrayList<>(Arrays.asList(uris)); | ||
} | ||
|
||
/** | ||
* Create a MonitorUris object to contain all the URIs to be monitored. | ||
* | ||
* @param uris An array of URIs. | ||
* @return A MonitorUris object. | ||
*/ | ||
@PluginFactory | ||
public static MonitorUris createMonitorUris( // | ||
@PluginElement("Uris") final Uri[] uris) { | ||
return new MonitorUris(uris == null ? Uri.EMPTY_ARRAY : uris); | ||
} | ||
|
||
/** | ||
* Returns a list of the {@code Uri} objects created during configuration. | ||
* @return the URIs to be monitored | ||
*/ | ||
public List<Uri> getUris() { | ||
return uris; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to you under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.logging.log4j.core.config; | ||
|
||
import java.util.Objects; | ||
import org.apache.logging.log4j.core.Core; | ||
import org.apache.logging.log4j.core.config.plugins.Plugin; | ||
import org.apache.logging.log4j.core.config.plugins.PluginFactory; | ||
import org.apache.logging.log4j.core.config.plugins.PluginValue; | ||
import org.apache.logging.log4j.status.StatusLogger; | ||
|
||
/** | ||
* Descriptor of a URI object that is created via configuration. | ||
*/ | ||
@Plugin(name = "Uri", category = Core.CATEGORY_NAME, printObject = true) | ||
public final class Uri { | ||
|
||
/** | ||
* The empty array. | ||
*/ | ||
static final Uri[] EMPTY_ARRAY = {}; | ||
|
||
private final String uri; | ||
|
||
private Uri(final String uri) { | ||
this.uri = Objects.requireNonNull(uri, "uri is null"); | ||
} | ||
|
||
/** | ||
* Creates a Uri object. | ||
* | ||
* @param uri the URI. | ||
* @return A Uri object. | ||
*/ | ||
@PluginFactory | ||
public static Uri createUri( // @formatter:off | ||
@PluginValue("uri") final String uri) { | ||
// @formatter:on | ||
|
||
StatusLogger.getLogger().debug("Creating Uri('{}')", uri); | ||
return new Uri(uri); | ||
} | ||
|
||
/** | ||
* Returns the URI. | ||
* | ||
* @return the URI | ||
*/ | ||
public String getUri() { | ||
return uri; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return uri.hashCode(); | ||
} | ||
|
||
@Override | ||
public boolean equals(final Object object) { | ||
if (this == object) { | ||
return true; | ||
} | ||
if (!(object instanceof Uri)) { | ||
return false; | ||
} | ||
final Uri other = (Uri) object; | ||
return this.uri == other.uri; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "Uri[" + uri + "]"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to you under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.logging.log4j.core.config.builder.api; | ||
|
||
/** | ||
* Assembler for constructing MonitorUri Components. | ||
*/ | ||
public interface MonitorUriComponentBuilder extends ComponentBuilder<MonitorUriComponentBuilder> {} |
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.
Can't we simplify this as follows? (Can
uris
be null at all?)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.
... and, this is the wrong place and time to do admit monitor URIs to the
WatchManager
. Would you move this admission operation (i.e., the logic inwatchMonitorUris
) toinitializeWatchers
, please?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.
Hi @vy
The reason I did not put it in initializeWatchers() is because initializeWatchers() is called before the elements are read. The top level attributes (including monitorInterval) have been read at this stage and can therefore be used, but the elements (including monitorUris) have not been.
Would it be ok to move invoking watchMonitorUris() to intialization() which is earlier than here, but after the elements have been read? This would avoid having to read the monitorUri element earlier and separately from the other elements.
I can move it to initializeWatchers() either if you still prefer that, and add code to read the monitorUri element separately to the other elements, but I think the above would be cleaner in the code.