Skip to content

Commit

Permalink
Deprecate StatusConsoleListener filters (#2226)
Browse files Browse the repository at this point in the history
The configuration attribute `verbose` (i.e., `<Configuration verbose="..."`) and
`StatusConsoleListener` filters are deprecated. This is part of an ongoing effort
to simplify the `StatusLogger`.

`verbose` attribute is deprecated, because it was only effective over
`verboseClasses` and that was hardcoded to a list only containing
`ResourceUtil`. `ResourceUtil` is slightly updated and its `logger.info()`
calls are replaced with `logger.debug()`.
  • Loading branch information
vy committed Jan 22, 2024
1 parent 5d47e93 commit 6d84367
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,29 +114,6 @@ void level_and_stream_should_be_honored() throws Exception {
.doesNotContain(discardedMessage.getFormattedMessage());
}

@Test
void filters_should_be_honored() throws Exception {

// Create the listener.
final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
final String encoding = "UTF-8";
final PrintStream printStream = new PrintStream(outputStream, false, encoding);
final StatusConsoleListener listener = new StatusConsoleListener(Level.TRACE, printStream);

// Set the filter.
final StackTraceElement caller = new StackTraceElement("callerClass", "callerMethod", "callerFile", 1);
listener.setFilters(caller.getClassName());

// Log the message to be filtered.
final Message message = MESSAGE_FACTORY.newMessage("foo");
listener.log(new StatusData(caller, Level.TRACE, message, null, null)); // as set by `StatusLogger` itself

// Verify the filtering.
printStream.flush();
final String output = outputStream.toString(encoding);
Assertions.assertThat(output).isEmpty();
}

@Test
void non_system_streams_should_be_closed() throws Exception {
final PrintStream stream = Mockito.mock(PrintStream.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ public class StatusConsoleListener implements StatusListener {

private Level level;

private String[] filters;

private final PrintStream stream;

private final Logger logger;
Expand Down Expand Up @@ -92,37 +90,21 @@ public Level getStatusLevel() {
*/
@Override
public void log(final StatusData data) {
final boolean filtered = filtered(data);
if (!filtered) {
logger
// Logging using _only_ the following 4 fields set by `StatusLogger#logMessage()`:
.atLevel(data.getLevel())
.withThrowable(data.getThrowable())
.withLocation(data.getStackTraceElement())
.log(data.getMessage());
}
logger
// Logging using _only_ the following 4 fields set by `StatusLogger#logMessage()`:
.atLevel(data.getLevel())
.withThrowable(data.getThrowable())
.withLocation(data.getStackTraceElement())
.log(data.getMessage());
}

/**
* Adds package name filters to exclude.
* @param filters An array of package names to exclude.
* @deprecated This method is ineffective and only kept for binary backward compatibility.
*/
public void setFilters(final String... filters) {
this.filters = filters;
}

private boolean filtered(final StatusData data) {
if (filters == null) {
return false;
}
final String caller = data.getStackTraceElement().getClassName();
for (final String filter : filters) {
if (caller.startsWith(filter)) {
return true;
}
}
return false;
}
@Deprecated
public void setFilters(final String... filters) {}

@Override
public void close() throws IOException {
Expand Down
5 changes: 0 additions & 5 deletions log4j-core-test/src/main/resources/Log4j-config.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,6 @@
<documentation>Enables the use of the strict XML format. Not supported in JSON configurations.</documentation>
</annotation>
</attribute>
<attribute name="verbose" type="tns:BooleanType" default="false">
<annotation>
<documentation>Enables diagnostic information while loading plugins.</documentation>
</annotation>
</attribute>
</complexType>
</element>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Configuration status="WARN" verbose="true">
<Configuration status="WARN">
<CustomLevels>
<CustomLevel name="INFOM1" intLevel="399" />
<CustomLevel name="INFOP1" intLevel="401" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.apache.logging.log4j.core.config.builder.api.Component;
import org.apache.logging.log4j.core.config.plugins.util.PluginManager;
import org.apache.logging.log4j.core.config.plugins.util.PluginType;
import org.apache.logging.log4j.core.config.plugins.util.ResolverUtil;
import org.apache.logging.log4j.core.config.status.StatusConfiguration;
import org.apache.logging.log4j.core.util.Patterns;

Expand All @@ -39,7 +38,6 @@
* @since 2.4
*/
public class BuiltConfiguration extends AbstractConfiguration {
private static final String[] VERBOSE_CLASSES = new String[] {ResolverUtil.class.getName()};
private final StatusConfiguration statusConfig;
protected Component rootComponent;
private Component loggersComponent;
Expand All @@ -53,8 +51,7 @@ public class BuiltConfiguration extends AbstractConfiguration {
public BuiltConfiguration(
final LoggerContext loggerContext, final ConfigurationSource source, final Component rootComponent) {
super(loggerContext, source);
statusConfig =
new StatusConfiguration().withVerboseClasses(VERBOSE_CLASSES).withStatus(getDefaultStatus());
statusConfig = new StatusConfiguration().withStatus(getDefaultStatus());
for (final Component component : rootComponent.getComponents()) {
switch (component.getPluginType()) {
case "Scripts": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ public class DefaultConfigurationBuilder<T extends BuiltConfiguration> implement
private ConfigurationSource source;
private int monitorInterval;
private Level level;
private String verbosity;
private String destination;
private String packages;
private String shutdownFlag;
Expand Down Expand Up @@ -200,9 +199,6 @@ public T build(final boolean initialize) {
if (level != null) {
configuration.getStatusConfiguration().withStatus(level);
}
if (verbosity != null) {
configuration.getStatusConfiguration().withVerbosity(verbosity);
}
if (destination != null) {
configuration.getStatusConfiguration().withDestination(destination);
}
Expand Down Expand Up @@ -273,9 +269,6 @@ private void writeXmlConfiguration(final XMLStreamWriter xmlWriter) throws XMLSt
if (level != null) {
xmlWriter.writeAttribute("status", level.name());
}
if (verbosity != null) {
xmlWriter.writeAttribute("verbose", verbosity);
}
if (destination != null) {
xmlWriter.writeAttribute("dest", destination);
}
Expand Down Expand Up @@ -596,9 +589,12 @@ public ConfigurationBuilder<T> setStatusLevel(final Level level) {
return this;
}

/**
* @deprecated This method is ineffective and only kept for binary backward compatibility.
*/
@Override
@Deprecated
public ConfigurationBuilder<T> setVerbosity(final String verbosity) {
this.verbosity = verbosity;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.apache.logging.log4j.core.config.ConfigurationSource;
import org.apache.logging.log4j.core.config.Node;
import org.apache.logging.log4j.core.config.Reconfigurable;
import org.apache.logging.log4j.core.config.plugins.util.ResolverUtil;
import org.apache.logging.log4j.core.config.status.StatusConfiguration;
import org.apache.logging.log4j.core.util.Loader;
import org.apache.logging.log4j.core.util.Patterns;
Expand All @@ -50,8 +49,6 @@ public class CompositeConfiguration extends AbstractConfiguration implements Rec
*/
public static final String MERGE_STRATEGY_PROPERTY = "log4j.mergeStrategy";

private static final String[] VERBOSE_CLASSES = new String[] {ResolverUtil.class.getName()};

private final List<? extends AbstractConfiguration> configurations;

private MergeStrategy mergeStrategy;
Expand Down Expand Up @@ -79,8 +76,7 @@ public CompositeConfiguration(final List<? extends AbstractConfiguration> config
for (final AbstractConfiguration config : configurations) {
mergeStrategy.mergeRootProperties(rootNode, config);
}
final StatusConfiguration statusConfig =
new StatusConfiguration().withVerboseClasses(VERBOSE_CLASSES).withStatus(getDefaultStatus());
final StatusConfiguration statusConfig = new StatusConfiguration().withStatus(getDefaultStatus());
for (final Map.Entry<String, String> entry : rootNode.getAttributes().entrySet()) {
final String key = entry.getKey();
final String value = getConfigurationStrSubstitutor().replace(entry.getValue());
Expand All @@ -92,8 +88,6 @@ public CompositeConfiguration(final List<? extends AbstractConfiguration> config
isShutdownHookEnabled = !"disable".equalsIgnoreCase(value);
} else if ("shutdownTimeout".equalsIgnoreCase(key)) {
shutdownTimeoutMillis = Long.parseLong(value);
} else if ("verbose".equalsIgnoreCase(key)) {
statusConfig.withVerbosity(value);
} else if ("packages".equalsIgnoreCase(key)) {
pluginPackages.addAll(Arrays.asList(value.split(Patterns.COMMA_SEPARATOR)));
} else if ("name".equalsIgnoreCase(key)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.apache.logging.log4j.core.config.Node;
import org.apache.logging.log4j.core.config.Reconfigurable;
import org.apache.logging.log4j.core.config.plugins.util.PluginType;
import org.apache.logging.log4j.core.config.plugins.util.ResolverUtil;
import org.apache.logging.log4j.core.config.status.StatusConfiguration;
import org.apache.logging.log4j.core.util.Integers;
import org.apache.logging.log4j.core.util.Patterns;
Expand All @@ -46,7 +45,6 @@
*/
public class JsonConfiguration extends AbstractConfiguration implements Reconfigurable {

private static final String[] VERBOSE_CLASSES = new String[] {ResolverUtil.class.getName()};
private final List<Status> status = new ArrayList<>();
private JsonNode root;

Expand All @@ -66,9 +64,7 @@ public JsonConfiguration(final LoggerContext loggerContext, final ConfigurationS
}
}
processAttributes(rootNode, root);
final StatusConfiguration statusConfig = new StatusConfiguration()
.withVerboseClasses(VERBOSE_CLASSES)
.withStatus(getDefaultStatus());
final StatusConfiguration statusConfig = new StatusConfiguration().withStatus(getDefaultStatus());
int monitorIntervalSeconds = 0;
for (final Map.Entry<String, String> entry :
rootNode.getAttributes().entrySet()) {
Expand All @@ -83,8 +79,6 @@ public JsonConfiguration(final LoggerContext loggerContext, final ConfigurationS
isShutdownHookEnabled = !"disable".equalsIgnoreCase(value);
} else if ("shutdownTimeout".equalsIgnoreCase(key)) {
shutdownTimeoutMillis = Long.parseLong(value);
} else if ("verbose".equalsIgnoreCase(entry.getKey())) {
statusConfig.withVerbosity(value);
} else if ("packages".equalsIgnoreCase(key)) {
pluginPackages.addAll(Arrays.asList(value.split(Patterns.COMMA_SEPARATOR)));
} else if ("name".equalsIgnoreCase(key)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public void findInPackage(final Test test, String packageName) {
final URL url = urls.nextElement();
final String urlPath = extractPath(url);

LOGGER.info("Scanning for classes in '{}' matching criteria {}", urlPath, test);
LOGGER.debug("Scanning for classes in '{}' matching criteria {}", urlPath, test);
// Check for a jar in a war in JBoss
if (VFSZIP.equals(url.getProtocol())) {
final String path = urlPath.substring(0, urlPath.length() - packageName.length() - 2);
Expand Down Expand Up @@ -457,9 +457,7 @@ protected void addIfMatching(final Test test, final String fqn) {
final ClassLoader loader = getClassLoader();
if (test.doesMatchClass()) {
final String externalName = fqn.substring(0, fqn.indexOf('.')).replace('/', '.');
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Checking to see if class {} matches criteria {}", externalName, test);
}
LOGGER.debug("Checking to see if class {} matches criteria {}", externalName, test);

final Class<?> type = loader.loadClass(externalName);
if (test.matches(type)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class StatusConfiguration {
private static final PrintStream DEFAULT_STREAM = System.out;

private static final Level DEFAULT_STATUS = Level.ERROR;
private static final Verbosity DEFAULT_VERBOSITY = Verbosity.QUIET;

private final Collection<String> errorMessages = new LinkedBlockingQueue<>();
private final StatusLogger logger = StatusLogger.getLogger();
Expand All @@ -49,12 +48,12 @@ public class StatusConfiguration {

private PrintStream destination = DEFAULT_STREAM;
private Level status = DEFAULT_STATUS;
private Verbosity verbosity = DEFAULT_VERBOSITY;
private String[] verboseClasses;

/**
* Specifies how verbose the StatusLogger should be.
* @deprecated This class is not used anymore and only kept for binary backward compatibility.
*/
@Deprecated
public enum Verbosity {
QUIET,
VERBOSE;
Expand All @@ -64,7 +63,9 @@ public enum Verbosity {
*
* @param value property value to parse.
* @return enum corresponding to value, or QUIET by default.
* @deprecated This class is not used anymore and only kept for binary backward compatibility.
*/
@Deprecated
public static Verbosity toVerbosity(final String value) {
return Boolean.parseBoolean(value) ? VERBOSE : QUIET;
}
Expand Down Expand Up @@ -156,9 +157,10 @@ public StatusConfiguration withStatus(final Level status) {
*
* @param verbosity basic filter for status logger messages.
* @return {@code this}
* @deprecated This method is ineffective and only kept for binary backward compatibility.
*/
@Deprecated
public StatusConfiguration withVerbosity(final String verbosity) {
this.verbosity = Verbosity.toVerbosity(verbosity);
return this;
}

Expand All @@ -167,9 +169,10 @@ public StatusConfiguration withVerbosity(final String verbosity) {
*
* @param verboseClasses names of classes to filter if not using VERBOSE.
* @return {@code this}
* @deprecated This method is ineffective and only kept for binary backward compatibility.
*/
@Deprecated
public StatusConfiguration withVerboseClasses(final String... verboseClasses) {
this.verboseClasses = verboseClasses;
return this;
}

Expand All @@ -183,7 +186,8 @@ public void initialize() {
} else {
final boolean configured = configureExistingStatusConsoleListener();
if (!configured) {
registerNewStatusConsoleListener();
final StatusConsoleListener listener = new StatusConsoleListener(this.status, this.destination);
this.logger.registerListener(listener);
}
migrateSavedLogMessages();
}
Expand All @@ -197,23 +201,12 @@ private boolean configureExistingStatusConsoleListener() {
final StatusConsoleListener listener = (StatusConsoleListener) statusListener;
listener.setLevel(this.status);
this.logger.updateListenerLevel(this.status);
if (this.verbosity == Verbosity.QUIET) {
listener.setFilters(this.verboseClasses);
}
configured = true;
}
}
return configured;
}

private void registerNewStatusConsoleListener() {
final StatusConsoleListener listener = new StatusConsoleListener(this.status, this.destination);
if (this.verbosity == Verbosity.QUIET) {
listener.setFilters(this.verboseClasses);
}
this.logger.registerListener(listener);
}

private void migrateSavedLogMessages() {
for (final String message : this.errorMessages) {
this.logger.error(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.apache.logging.log4j.core.config.Node;
import org.apache.logging.log4j.core.config.Reconfigurable;
import org.apache.logging.log4j.core.config.plugins.util.PluginType;
import org.apache.logging.log4j.core.config.plugins.util.ResolverUtil;
import org.apache.logging.log4j.core.config.status.StatusConfiguration;
import org.apache.logging.log4j.core.util.Closer;
import org.apache.logging.log4j.core.util.Integers;
Expand All @@ -63,7 +62,6 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu

private static final String XINCLUDE_FIXUP_LANGUAGE = "http://apache.org/xml/features/xinclude/fixup-language";
private static final String XINCLUDE_FIXUP_BASE_URIS = "http://apache.org/xml/features/xinclude/fixup-base-uris";
private static final String[] VERBOSE_CLASSES = new String[] {ResolverUtil.class.getName()};
private static final String LOG4J_XSD = "Log4j-config.xsd";

private final List<Status> status = new ArrayList<>();
Expand Down Expand Up @@ -108,9 +106,7 @@ public XmlConfiguration(final LoggerContext loggerContext, final ConfigurationSo
}
rootElement = document.getDocumentElement();
final Map<String, String> attrs = processAttributes(rootNode, rootElement);
final StatusConfiguration statusConfig = new StatusConfiguration()
.withVerboseClasses(VERBOSE_CLASSES)
.withStatus(getDefaultStatus());
final StatusConfiguration statusConfig = new StatusConfiguration().withStatus(getDefaultStatus());
int monitorIntervalSeconds = 0;
for (final Map.Entry<String, String> entry : attrs.entrySet()) {
final String key = entry.getKey();
Expand All @@ -123,8 +119,6 @@ public XmlConfiguration(final LoggerContext loggerContext, final ConfigurationSo
isShutdownHookEnabled = !"disable".equalsIgnoreCase(value);
} else if ("shutdownTimeout".equalsIgnoreCase(key)) {
shutdownTimeoutMillis = Long.parseLong(value);
} else if ("verbose".equalsIgnoreCase(key)) {
statusConfig.withVerbosity(value);
} else if ("packages".equalsIgnoreCase(key)) {
pluginPackages.addAll(Arrays.asList(value.split(Patterns.COMMA_SEPARATOR)));
} else if ("name".equalsIgnoreCase(key)) {
Expand Down

0 comments on commit 6d84367

Please sign in to comment.