You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2024/08/13 09:58:55 UTC
[PR] Improve plugin descriptor warnings (logging-log4j2)
ppkarwasz opened a new pull request, #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836
This improves status logger warnings about:
* missing `Log4j2Plugin.dat` plugin descriptors,
* usage of package scanning.
The warnings are reworded in a way that they are more useful to end-users. If package scanning is enabled, linkage errors (almost exclusively due to optional dependencies) are logged at a `DEBUG` level instead of `WARN`.
This addresses @vlsi suggestions from
https://github.com/apache/jmeter/issues/5937#issuecomment-2271160800 and fixes #2835.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715177626
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java:
##########
@@ -310,7 +314,7 @@ private void monitorSource(final Reconfigurable reconfigurable, final Configurat
@Override
public void start() {
// Preserve the prior behavior of initializing during start if not initialized.
- if (getState().equals(State.INITIALIZING)) {
+ if (getState() == State.INITIALIZING) {
Review Comment:
Why would `==` generate an NPE?
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715077408
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java:
##########
@@ -310,7 +314,7 @@ private void monitorSource(final Reconfigurable reconfigurable, final Configurat
@Override
public void start() {
// Preserve the prior behavior of initializing during start if not initialized.
- if (getState().equals(State.INITIALIZING)) {
+ if (getState() == State.INITIALIZING) {
Review Comment:
**Nit:** I am in favor of `enumConstant.equals(input)`, since this is the only NPE-resilient single-statement comparison.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginManager.java:
##########
@@ -35,6 +39,13 @@ public class PluginManager {
private static final CopyOnWriteArrayList<String> PACKAGES = new CopyOnWriteArrayList<>();
private static final String LOG4J_PACKAGES = "org.apache.logging.log4j.core";
+ private static final String DEPRECATION_WARNING =
+ "The use of package scanning to locate Log4j plugins is deprecated.\n"
+ + "Please remove the deprecated {} method call from {}.";
Review Comment:
```suggestion
+ "Please remove the deprecated `{}` method call from `{}`.";
```
##########
src/changelog/.2.x.x/2835_improve_plugin_descriptor_warnings.xml:
##########
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
Review Comment:
```suggestion
type="changed">
```
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginManager.java:
##########
@@ -138,28 +149,61 @@ public void collectPlugins(final List<String> packages) {
if (builtInPlugins.isEmpty()) {
// If we didn't find any plugins above, someone must have messed with the log4j-core.jar.
// Search the standard package in the hopes we can find our core plugins.
+ LOGGER.warn(
+ "No Log4j plugin descriptor was found in the classpath.\n"
+ + "Falling back to scanning the `{}` package.\n{}",
+ LOG4J_PACKAGES,
+ PLUGIN_DESCRIPTOR_DOC);
builtInPlugins = PluginRegistry.getInstance().loadFromPackage(LOG4J_PACKAGES);
}
- mergeByName(newPlugins, builtInPlugins.get(categoryLowerCase));
+ mergeByName(newPlugins, builtInPlugins.get(categoryLowerCase), null);
// Next, iterate any Log4j2Plugin.dat files from OSGi Bundles
for (final Map<String, List<PluginType<?>>> pluginsByCategory :
PluginRegistry.getInstance().getPluginsByCategoryByBundleId().values()) {
- mergeByName(newPlugins, pluginsByCategory.get(categoryLowerCase));
+ mergeByName(newPlugins, pluginsByCategory.get(categoryLowerCase), null);
}
+ final List<String> scannedPluginClassNames = new ArrayList<>();
// Next iterate any packages passed to the static addPackage method.
for (final String pkg : PACKAGES) {
mergeByName(
newPlugins,
- PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase));
+ PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase),
+ scannedPluginClassNames);
}
// Finally iterate any packages provided in the configuration (note these can be changed at runtime).
if (packages != null) {
for (final String pkg : packages) {
mergeByName(
newPlugins,
- PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase));
+ PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase),
+ scannedPluginClassNames);
+ }
+ }
+ // Report package scanning deprecation.
+ if (!scannedPluginClassNames.isEmpty()) {
+ final Predicate<String> customPluginPredicate = PluginManager::isCustomPlugin;
+ final String standardPlugins = scannedPluginClassNames.stream()
+ .filter(customPluginPredicate.negate())
+ .collect(Collectors.joining("\n\t"));
+ if (!standardPlugins.isEmpty()) {
+ LOGGER.warn(
+ "The plugin descriptors for the following '{}' plugins are missing:\n\t{}\n{}",
Review Comment:
```suggestion
"The Log4j plugin descriptors for the following `{}` plugins are missing:\n\t{}\n{}",
```
This message doesn't communicate an action users can take. Ideally, we should be able to answer users' _"Stuff is missing. So what shall I do?"_ thinking process.
##########
src/site/antora/modules/ROOT/pages/faq.adoc:
##########
@@ -214,3 +214,95 @@ When you are using Log4j with ProGuard/R8 enabled, you need to add the following
----
-keep,allowoptimization class org.apache.logging.log4j.** { *; }
----
+
+[#plugin-descriptors]
+== Why am I receiving warnings about missing plugin descriptors?
+
+Since Log4j 2.0
Review Comment:
```suggestion
```
##########
src/site/antora/modules/ROOT/pages/faq.adoc:
##########
@@ -214,3 +214,95 @@ When you are using Log4j with ProGuard/R8 enabled, you need to add the following
----
-keep,allowoptimization class org.apache.logging.log4j.** { *; }
----
+
+[#plugin-descriptors]
+== Why am I receiving warnings about missing plugin descriptors?
+
+Since Log4j 2.0
+xref:manual/plugins.adoc[Log4j Plugins]
+should be registered in a:
+
+[source]
+----
+META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat
+----
+
+file on the classpath of the Java applications.
+
+Since Log4j 2.24.0, you will receive a warning if a plugin is not registered properly.
+The common causes for improperly registered plugins are:
+
+* One of the xref:components.adoc[Log4j artifacts] might be corrupted.
+If you are using a single-JAR application, this might be caused by an improper shading configuration.
+See <<shading>> on how to properly shade Log4j artifacts.
+* A custom Log4j plugin has not been properly registered.
+See xref:manual/plugins.adoc#plugin-registry[Registering plugins] for more details.
+
+[#single-jar]
+== How to correctly create single-JAR applications?
Review Comment:
```suggestion
== How do I create a single-JAR application containing Log4j Core?
```
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginManager.java:
##########
@@ -138,28 +149,61 @@ public void collectPlugins(final List<String> packages) {
if (builtInPlugins.isEmpty()) {
// If we didn't find any plugins above, someone must have messed with the log4j-core.jar.
// Search the standard package in the hopes we can find our core plugins.
+ LOGGER.warn(
+ "No Log4j plugin descriptor was found in the classpath.\n"
+ + "Falling back to scanning the `{}` package.\n{}",
+ LOG4J_PACKAGES,
+ PLUGIN_DESCRIPTOR_DOC);
builtInPlugins = PluginRegistry.getInstance().loadFromPackage(LOG4J_PACKAGES);
}
- mergeByName(newPlugins, builtInPlugins.get(categoryLowerCase));
+ mergeByName(newPlugins, builtInPlugins.get(categoryLowerCase), null);
// Next, iterate any Log4j2Plugin.dat files from OSGi Bundles
for (final Map<String, List<PluginType<?>>> pluginsByCategory :
PluginRegistry.getInstance().getPluginsByCategoryByBundleId().values()) {
- mergeByName(newPlugins, pluginsByCategory.get(categoryLowerCase));
+ mergeByName(newPlugins, pluginsByCategory.get(categoryLowerCase), null);
}
+ final List<String> scannedPluginClassNames = new ArrayList<>();
// Next iterate any packages passed to the static addPackage method.
for (final String pkg : PACKAGES) {
mergeByName(
newPlugins,
- PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase));
+ PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase),
+ scannedPluginClassNames);
}
// Finally iterate any packages provided in the configuration (note these can be changed at runtime).
if (packages != null) {
for (final String pkg : packages) {
mergeByName(
newPlugins,
- PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase));
+ PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase),
+ scannedPluginClassNames);
+ }
+ }
+ // Report package scanning deprecation.
+ if (!scannedPluginClassNames.isEmpty()) {
+ final Predicate<String> customPluginPredicate = PluginManager::isCustomPlugin;
+ final String standardPlugins = scannedPluginClassNames.stream()
+ .filter(customPluginPredicate.negate())
+ .collect(Collectors.joining("\n\t"));
+ if (!standardPlugins.isEmpty()) {
+ LOGGER.warn(
+ "The plugin descriptors for the following '{}' plugins are missing:\n\t{}\n{}",
+ category,
+ standardPlugins,
+ PLUGIN_DESCRIPTOR_DOC);
+ }
+ final String customPlugins = scannedPluginClassNames.stream()
+ .filter(customPluginPredicate)
+ .collect(Collectors.joining("\n\t"));
+ if (!customPlugins.isEmpty()) {
+ LOGGER.warn(
+ "Some custom '{}' plugins are not properly registered:\n\t{}\n"
Review Comment:
```suggestion
"Some custom `{}` Log4j plugins are not properly registered:\n\t{}\n"
```
##########
src/site/antora/modules/ROOT/pages/faq.adoc:
##########
@@ -214,3 +214,95 @@ When you are using Log4j with ProGuard/R8 enabled, you need to add the following
----
-keep,allowoptimization class org.apache.logging.log4j.** { *; }
----
+
+[#plugin-descriptors]
+== Why am I receiving warnings about missing plugin descriptors?
+
+Since Log4j 2.0
+xref:manual/plugins.adoc[Log4j Plugins]
+should be registered in a:
+
+[source]
+----
+META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat
+----
+
+file on the classpath of the Java applications.
+
+Since Log4j 2.24.0, you will receive a warning if a plugin is not registered properly.
+The common causes for improperly registered plugins are:
+
+* One of the xref:components.adoc[Log4j artifacts] might be corrupted.
+If you are using a single-JAR application, this might be caused by an improper shading configuration.
+See <<shading>> on how to properly shade Log4j artifacts.
+* A custom Log4j plugin has not been properly registered.
+See xref:manual/plugins.adoc#plugin-registry[Registering plugins] for more details.
+
+[#single-jar]
+== How to correctly create single-JAR applications?
+
+There are two ways to create single-JAR applications: you can create
+<<jar-in-jar,JAR-in-JAR executable packages>>
+or
+<<shading,shaded JARs>>.
+See the sections below on how to properly use these techniques with Log4j Core.
+
+[#jar-in-jar]
+=== Jar-in-Jar
+
+The easiest and recommended way to create single-JAR applications based on Log4j Core is to include **unmodified** versions of Log4j artifacts inside the JAR file of your application.
Review Comment:
```suggestion
The easiest and recommended way to create single-JAR applications containing Log4j Core is to include **unmodified** versions of Log4j artifacts inside the JAR file of your application.
```
##########
src/site/antora/modules/ROOT/pages/faq.adoc:
##########
@@ -214,3 +214,95 @@ When you are using Log4j with ProGuard/R8 enabled, you need to add the following
----
-keep,allowoptimization class org.apache.logging.log4j.** { *; }
----
+
+[#plugin-descriptors]
+== Why am I receiving warnings about missing plugin descriptors?
+
+Since Log4j 2.0
+xref:manual/plugins.adoc[Log4j Plugins]
+should be registered in a:
+
+[source]
+----
+META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat
+----
+
+file on the classpath of the Java applications.
+
+Since Log4j 2.24.0, you will receive a warning if a plugin is not registered properly.
Review Comment:
```suggestion
Starting with Log4j version `2.24.0`, you will receive a warning if a plugin is not registered properly.
```
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/ResolverUtil.java:
##########
@@ -473,8 +469,10 @@ protected void addIfMatching(final Test test, final String fqn) {
resourceMatches.add(url.toURI());
}
}
- } catch (final Throwable t) {
- LOGGER.warn("Could not examine class {}", fqn, t);
+ } catch (final LinkageError e) {
+ LOGGER.debug("Could not scan class {}, since an optional dependency is missing.", fqn, e);
+ } catch (final Exception e) {
+ LOGGER.warn("Could not scan class {}.", fqn, e);
Review Comment:
```suggestion
} catch (final LinkageError error) {
LOGGER.debug("Could not scan class `{}`, since an optional dependency is missing.", fqn, error);
} catch (final Exception error) {
LOGGER.warn("Could not scan class `{}`.", fqn, error);
```
This will still log `ClassNotFoundException` at `WARN`, won't it? Shall we instead have a single `catch` block and check for the existence of `ClassNotFoundException` in the causal chain?
I am actually in favor of **not** creating a whitelist here and instead setting the logging level to `DEBUG` for all `Exception`s.
##########
src/site/antora/modules/ROOT/pages/faq.adoc:
##########
@@ -214,3 +214,95 @@ When you are using Log4j with ProGuard/R8 enabled, you need to add the following
----
-keep,allowoptimization class org.apache.logging.log4j.** { *; }
----
+
+[#plugin-descriptors]
+== Why am I receiving warnings about missing plugin descriptors?
+
+Since Log4j 2.0
+xref:manual/plugins.adoc[Log4j Plugins]
+should be registered in a:
+
+[source]
+----
+META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat
+----
+
+file on the classpath of the Java applications.
+
+Since Log4j 2.24.0, you will receive a warning if a plugin is not registered properly.
+The common causes for improperly registered plugins are:
+
+* One of the xref:components.adoc[Log4j artifacts] might be corrupted.
+If you are using a single-JAR application, this might be caused by an improper shading configuration.
+See <<shading>> on how to properly shade Log4j artifacts.
+* A custom Log4j plugin has not been properly registered.
+See xref:manual/plugins.adoc#plugin-registry[Registering plugins] for more details.
+
+[#single-jar]
+== How to correctly create single-JAR applications?
+
+There are two ways to create single-JAR applications: you can create
+<<jar-in-jar,JAR-in-JAR executable packages>>
+or
+<<shading,shaded JARs>>.
+See the sections below on how to properly use these techniques with Log4j Core.
+
+[#jar-in-jar]
+=== Jar-in-Jar
+
+The easiest and recommended way to create single-JAR applications based on Log4j Core is to include **unmodified** versions of Log4j artifacts inside the JAR file of your application.
+You can do it with the following build tool plugins:
+
+* https://docs.spring.io/spring-boot/maven-plugin/packaging.html[Spring Boot Maven Plugin],
+* https://docs.spring.io/spring-boot/gradle-plugin/packaging.html[Spring Boot Gradle Plugin],
+* https://github.com/sbt/sbt-onejar[sbt-onejar plugin].
+
+[#shading]
+=== Shading/Shadowing
+
+The shading process unwraps all the files contained in dependency JARs and copies them to the JAR containing the application.
+Since multiple JARs can contain files with the same name, you need to properly resolve the file naming conflicts that arise.
+
+If your application uses Log4j Core, the following conflicts must be resolved:
+
+`module-info.class`::
+If you are writing a JPMS application, you need to propertly merge these files.
+Otherwise, they should be discarded.
+
+`META-INF/MANIFEST.MF`::
++
+If you are writing a Java SE application, you only need to add:
++
+[source,manifest]
+----
+Multi-Release: true
+----
++
+to your application manifest.
++
+If you are writing an OSGi application, you need to properly merge the OSGi headers.
+
+`META-INF/DEPENDENCIES`::
+`META-INF/LICENSE`::
+`META-INF/NOTICE`::
+Ask your legal department on how to handle these files.
+
+`META-INF/services/*`::
+These files contain
+https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html[`ServiceLoader` descriptors].
+You need to properly merge them by concatenating conflicting files.
Review Comment:
**Tip:** You can consider moving this to a `collapsible`.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715191710
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginManager.java:
##########
@@ -35,6 +39,13 @@ public class PluginManager {
private static final CopyOnWriteArrayList<String> PACKAGES = new CopyOnWriteArrayList<>();
private static final String LOG4J_PACKAGES = "org.apache.logging.log4j.core";
+ private static final String DEPRECATION_WARNING =
+ "The use of package scanning to locate Log4j plugins is deprecated.\n"
+ + "Please remove the deprecated {} method call from {}.";
Review Comment:
Fixed in https://github.com/apache/logging-log4j2/pull/2836/commits/939de777cd047753b393365506cbe54e34a7af9b
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715294845
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
Review Comment:
Great job! :star_struck:
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.plugins.util;
+
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Collections.enumeration;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Enumeration;
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
+import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.test.Compiler;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class PluginManagerTest {
+
+ private static final String PLUGIN_DESCRIPTOR =
+ "META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat";
+
+ @BeforeEach
+ void setUp() {
+ PluginRegistry.getInstance().clear();
+ }
+
+ @Test
+ @UsingStatusListener
+ @SuppressWarnings("deprecation")
+ void calling_addPackage_issues_warning(final ListStatusListener listener) {
+ try {
+ PluginManager.addPackage("com.example");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+
+ PluginManager.addPackages(singletonList("com.example"));
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+ } finally {
+ PluginManager.clearPackages();
+ }
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_issues_warning(final ListStatusListener listener) {
+ final ConfigurationSource source = ConfigurationSource.fromResource(
+ "customplugin/log4j2-741.xml", getClass().getClassLoader());
+ assertThat(source).as("Configuration source").isNotNull();
+ final Configuration configuration = new XmlConfigurationFactory().getConfiguration(null, source);
+ assertThat(configuration).as("Configuration").isNotNull();
+ configuration.initialize();
+ assertThat(configuration.getPluginPackages()).as("Plugin packages").contains("customplugin");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_finds_custom_plugin(final ListStatusListener listener) throws Exception {
+ // To ensure our custom plugin is NOT included in the log4j plugin metadata file,
+ // we make sure the class does not exist until after the build is finished.
+ // So we don't create the custom plugin class until this test is run.
+ final URL resource = PluginManagerTest.class.getResource("/customplugin/FixedStringLayout.java.source");
+ assertThat(resource).isNotNull();
+ final File orig = new File(resource.toURI());
+ final File f = new File(orig.getParentFile(), "FixedStringLayout.java");
+ assertDoesNotThrow(() -> FileUtils.copyFile(orig, f));
+ // compile generated source
+ // (switch off annotation processing: no need to create Log4j2Plugins.dat)
+ Compiler.compile(f, "-proc:none");
+ assertDoesNotThrow(() -> FileUtils.delete(f));
+ // load the compiled class
+ Class.forName("customplugin.FixedStringLayout");
+
+ final PluginManager manager = new PluginManager(Node.CATEGORY);
+ manager.collectPlugins(singletonList("customplugin"));
+ assertThat(manager.getPluginType("FixedString"))
+ .as("Custom unregistered plugin")
+ .isNotNull();
+ assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
+ assertThat(message.getLevel()).isEqualTo(Level.WARN);
+ assertThat(message.getMessage().getFormattedMessage())
+ .as("Status logger message")
+ // The message specifies which plugin is not registered
+ .contains("customplugin.FixedStringLayout")
+ // The message provides a link to the registration instructions
+ .contains("https://logging.apache.org/log4j/2.x/manual/plugins.html#plugin-registry");
+ });
+ }
+
+ @Test
+ @UsingStatusListener
+ void missing_plugin_descriptor_issues_warning(final ListStatusListener listener) {
+ final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+ try {
+ Thread.currentThread()
+ .setContextClassLoader(new FilteringClassLoader(getClass().getClassLoader(), null));
+ assertThat(LoaderUtil.getClassLoader().getResource(PLUGIN_DESCRIPTOR))
+ .isNull();
+ final PluginManager manager = new PluginManager(Node.CATEGORY);
+ manager.collectPlugins(null);
+ assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
+ assertThat(message.getLevel()).isEqualTo(Level.WARN);
+ assertThat(message.getMessage().getFormattedMessage())
+ .as("Status logger message")
+ // The message provides a link to plugin descriptor troubleshooting.
+ .contains("https://logging.apache.org/log4j/2.x/faq.html#plugin-descriptors");
+ });
+ } finally {
+ Thread.currentThread().setContextClassLoader(tccl);
+ }
+ }
+
+ /**
+ * Tests if removing descriptors of standard Log4j artifacts and replacing them with a custom one issues warnings.
+ * <p>
+ * This often happens in shading scenarios.
+ * </p>
+ */
+ @Test
+ @UsingStatusListener
+ void corrupted_plugin_descriptor_issues_warning(final ListStatusListener listener) throws IOException {
+ // Create empty descriptor
+ final Path customDescriptor = Files.createTempFile("Log4j2Plugins", ".dat");
+ final URL descriptorUrl = customDescriptor.toUri().toURL();
+ final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+ try {
+ // Generate single plugin
+ final PluginEntry entry = new PluginEntry();
+ entry.setKey("CustomPlugin");
+ entry.setClassName("com.example.CustomPlugin");
+ entry.setName("CustomPlugin");
+ entry.setCategory(Node.CATEGORY);
+ // Generate descriptor with single plugin
+ final PluginCache customCache = new PluginCache();
+ customCache.getCategory(Node.CATEGORY).put("CustomPlugin", entry);
+ customCache.writeCache(Files.newOutputStream(customDescriptor));
Review Comment:
`CustomPlugin` should not exist right? If so, would you mind renaming it to `com.example.NotExistingPlugin`, please? (The case is not self-explanatory to those unfamiliar with the test.)
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.plugins.util;
+
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Collections.enumeration;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Enumeration;
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
+import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.test.Compiler;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class PluginManagerTest {
+
+ private static final String PLUGIN_DESCRIPTOR =
+ "META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat";
+
+ @BeforeEach
+ void setUp() {
+ PluginRegistry.getInstance().clear();
+ }
+
+ @Test
+ @UsingStatusListener
+ @SuppressWarnings("deprecation")
+ void calling_addPackage_issues_warning(final ListStatusListener listener) {
+ try {
+ PluginManager.addPackage("com.example");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+
+ PluginManager.addPackages(singletonList("com.example"));
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+ } finally {
+ PluginManager.clearPackages();
+ }
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_issues_warning(final ListStatusListener listener) {
+ final ConfigurationSource source = ConfigurationSource.fromResource(
+ "customplugin/log4j2-741.xml", getClass().getClassLoader());
+ assertThat(source).as("Configuration source").isNotNull();
+ final Configuration configuration = new XmlConfigurationFactory().getConfiguration(null, source);
+ assertThat(configuration).as("Configuration").isNotNull();
+ configuration.initialize();
+ assertThat(configuration.getPluginPackages()).as("Plugin packages").contains("customplugin");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_finds_custom_plugin(final ListStatusListener listener) throws Exception {
+ // To ensure our custom plugin is NOT included in the log4j plugin metadata file,
+ // we make sure the class does not exist until after the build is finished.
+ // So we don't create the custom plugin class until this test is run.
Review Comment:
Would you mind also documenting `FixedStringLayout.java.source` with similar 3-4 sentences to explain its raison d'être, please?
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.plugins.util;
+
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Collections.enumeration;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Enumeration;
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
+import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.test.Compiler;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class PluginManagerTest {
+
+ private static final String PLUGIN_DESCRIPTOR =
Review Comment:
Why do we not use `PluginProcessor.PLUGIN_CACHE_FILE` instead?
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715380178
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.plugins.util;
+
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Collections.enumeration;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Enumeration;
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
+import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.test.Compiler;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class PluginManagerTest {
+
+ private static final String PLUGIN_DESCRIPTOR =
Review Comment:
Fixed in https://github.com/apache/logging-log4j2/pull/2836/commits/d2bdd02ca9add9cc155d7c8a65f1335bf8d5dd70
BTW: I verified that BND Baseline will complain if the value of `PluginProcessor.PLUGIN_CACHE_FILE` unintentionally changes.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715181894
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java:
##########
@@ -310,7 +314,7 @@ private void monitorSource(final Reconfigurable reconfigurable, final Configurat
@Override
public void start() {
// Preserve the prior behavior of initializing during start if not initialized.
- if (getState().equals(State.INITIALIZING)) {
+ if (getState() == State.INITIALIZING) {
Review Comment:
You're right, nevermind.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715191317
##########
src/site/antora/modules/ROOT/pages/faq.adoc:
##########
@@ -214,3 +214,95 @@ When you are using Log4j with ProGuard/R8 enabled, you need to add the following
----
-keep,allowoptimization class org.apache.logging.log4j.** { *; }
----
+
+[#plugin-descriptors]
+== Why am I receiving warnings about missing plugin descriptors?
+
+Since Log4j 2.0
+xref:manual/plugins.adoc[Log4j Plugins]
+should be registered in a:
+
+[source]
+----
+META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat
+----
+
+file on the classpath of the Java applications.
+
+Since Log4j 2.24.0, you will receive a warning if a plugin is not registered properly.
+The common causes for improperly registered plugins are:
+
+* One of the xref:components.adoc[Log4j artifacts] might be corrupted.
+If you are using a single-JAR application, this might be caused by an improper shading configuration.
+See <<shading>> on how to properly shade Log4j artifacts.
+* A custom Log4j plugin has not been properly registered.
+See xref:manual/plugins.adoc#plugin-registry[Registering plugins] for more details.
+
+[#single-jar]
+== How to correctly create single-JAR applications?
+
+There are two ways to create single-JAR applications: you can create
+<<jar-in-jar,JAR-in-JAR executable packages>>
+or
+<<shading,shaded JARs>>.
+See the sections below on how to properly use these techniques with Log4j Core.
+
+[#jar-in-jar]
+=== Jar-in-Jar
+
+The easiest and recommended way to create single-JAR applications based on Log4j Core is to include **unmodified** versions of Log4j artifacts inside the JAR file of your application.
+You can do it with the following build tool plugins:
+
+* https://docs.spring.io/spring-boot/maven-plugin/packaging.html[Spring Boot Maven Plugin],
+* https://docs.spring.io/spring-boot/gradle-plugin/packaging.html[Spring Boot Gradle Plugin],
+* https://github.com/sbt/sbt-onejar[sbt-onejar plugin].
+
+[#shading]
+=== Shading/Shadowing
+
+The shading process unwraps all the files contained in dependency JARs and copies them to the JAR containing the application.
+Since multiple JARs can contain files with the same name, you need to properly resolve the file naming conflicts that arise.
+
+If your application uses Log4j Core, the following conflicts must be resolved:
+
+`module-info.class`::
+If you are writing a JPMS application, you need to propertly merge these files.
+Otherwise, they should be discarded.
+
+`META-INF/MANIFEST.MF`::
++
+If you are writing a Java SE application, you only need to add:
++
+[source,manifest]
+----
+Multi-Release: true
+----
++
+to your application manifest.
++
+If you are writing an OSGi application, you need to properly merge the OSGi headers.
+
+`META-INF/DEPENDENCIES`::
+`META-INF/LICENSE`::
+`META-INF/NOTICE`::
+Ask your legal department on how to handle these files.
+
+`META-INF/services/*`::
+These files contain
+https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html[`ServiceLoader` descriptors].
+You need to properly merge them by concatenating conflicting files.
Review Comment:
Fixed in https://github.com/apache/logging-log4j2/pull/2836/commits/939de777cd047753b393365506cbe54e34a7af9b
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715192319
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/ResolverUtil.java:
##########
@@ -473,8 +469,10 @@ protected void addIfMatching(final Test test, final String fqn) {
resourceMatches.add(url.toURI());
}
}
- } catch (final Throwable t) {
- LOGGER.warn("Could not examine class {}", fqn, t);
+ } catch (final LinkageError e) {
+ LOGGER.debug("Could not scan class {}, since an optional dependency is missing.", fqn, e);
+ } catch (final Exception e) {
+ LOGGER.warn("Could not scan class {}.", fqn, e);
Review Comment:
Fixed in https://github.com/apache/logging-log4j2/pull/2836/commits/939de777cd047753b393365506cbe54e34a7af9b
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715382142
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.plugins.util;
+
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Collections.enumeration;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Enumeration;
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
+import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.test.Compiler;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class PluginManagerTest {
+
+ private static final String PLUGIN_DESCRIPTOR =
+ "META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat";
+
+ @BeforeEach
+ void setUp() {
+ PluginRegistry.getInstance().clear();
+ }
+
+ @Test
+ @UsingStatusListener
+ @SuppressWarnings("deprecation")
+ void calling_addPackage_issues_warning(final ListStatusListener listener) {
+ try {
+ PluginManager.addPackage("com.example");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+
+ PluginManager.addPackages(singletonList("com.example"));
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+ } finally {
+ PluginManager.clearPackages();
+ }
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_issues_warning(final ListStatusListener listener) {
+ final ConfigurationSource source = ConfigurationSource.fromResource(
+ "customplugin/log4j2-741.xml", getClass().getClassLoader());
+ assertThat(source).as("Configuration source").isNotNull();
+ final Configuration configuration = new XmlConfigurationFactory().getConfiguration(null, source);
+ assertThat(configuration).as("Configuration").isNotNull();
+ configuration.initialize();
+ assertThat(configuration.getPluginPackages()).as("Plugin packages").contains("customplugin");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_finds_custom_plugin(final ListStatusListener listener) throws Exception {
+ // To ensure our custom plugin is NOT included in the log4j plugin metadata file,
+ // we make sure the class does not exist until after the build is finished.
+ // So we don't create the custom plugin class until this test is run.
+ final URL resource = PluginManagerTest.class.getResource("/customplugin/FixedStringLayout.java.source");
+ assertThat(resource).isNotNull();
+ final File orig = new File(resource.toURI());
+ final File f = new File(orig.getParentFile(), "FixedStringLayout.java");
+ assertDoesNotThrow(() -> FileUtils.copyFile(orig, f));
+ // compile generated source
+ // (switch off annotation processing: no need to create Log4j2Plugins.dat)
+ Compiler.compile(f, "-proc:none");
+ assertDoesNotThrow(() -> FileUtils.delete(f));
+ // load the compiled class
+ Class.forName("customplugin.FixedStringLayout");
+
+ final PluginManager manager = new PluginManager(Node.CATEGORY);
+ manager.collectPlugins(singletonList("customplugin"));
+ assertThat(manager.getPluginType("FixedString"))
+ .as("Custom unregistered plugin")
+ .isNotNull();
+ assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
+ assertThat(message.getLevel()).isEqualTo(Level.WARN);
+ assertThat(message.getMessage().getFormattedMessage())
+ .as("Status logger message")
+ // The message specifies which plugin is not registered
+ .contains("customplugin.FixedStringLayout")
+ // The message provides a link to the registration instructions
+ .contains("https://logging.apache.org/log4j/2.x/manual/plugins.html#plugin-registry");
+ });
+ }
+
+ @Test
+ @UsingStatusListener
+ void missing_plugin_descriptor_issues_warning(final ListStatusListener listener) {
+ final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+ try {
+ Thread.currentThread()
+ .setContextClassLoader(new FilteringClassLoader(getClass().getClassLoader(), null));
+ assertThat(LoaderUtil.getClassLoader().getResource(PLUGIN_DESCRIPTOR))
+ .isNull();
+ final PluginManager manager = new PluginManager(Node.CATEGORY);
+ manager.collectPlugins(null);
+ assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
+ assertThat(message.getLevel()).isEqualTo(Level.WARN);
+ assertThat(message.getMessage().getFormattedMessage())
+ .as("Status logger message")
+ // The message provides a link to plugin descriptor troubleshooting.
+ .contains("https://logging.apache.org/log4j/2.x/faq.html#plugin-descriptors");
+ });
+ } finally {
+ Thread.currentThread().setContextClassLoader(tccl);
+ }
+ }
+
+ /**
+ * Tests if removing descriptors of standard Log4j artifacts and replacing them with a custom one issues warnings.
+ * <p>
+ * This often happens in shading scenarios.
+ * </p>
+ */
+ @Test
+ @UsingStatusListener
+ void corrupted_plugin_descriptor_issues_warning(final ListStatusListener listener) throws IOException {
+ // Create empty descriptor
+ final Path customDescriptor = Files.createTempFile("Log4j2Plugins", ".dat");
+ final URL descriptorUrl = customDescriptor.toUri().toURL();
+ final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+ try {
+ // Generate single plugin
+ final PluginEntry entry = new PluginEntry();
+ entry.setKey("CustomPlugin");
+ entry.setClassName("com.example.CustomPlugin");
+ entry.setName("CustomPlugin");
+ entry.setCategory(Node.CATEGORY);
+ // Generate descriptor with single plugin
+ final PluginCache customCache = new PluginCache();
+ customCache.getCategory(Node.CATEGORY).put("CustomPlugin", entry);
+ customCache.writeCache(Files.newOutputStream(customDescriptor));
Review Comment:
It doesn't really matter if the class exists or not.
I added a comment in https://github.com/apache/logging-log4j2/pull/2836/commits/d2bdd02ca9add9cc155d7c8a65f1335bf8d5dd70.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#issuecomment-2286213351
@vy,
I added some test cases to check for the presence of an appropriate warning message.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715181097
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginManager.java:
##########
@@ -138,28 +149,61 @@ public void collectPlugins(final List<String> packages) {
if (builtInPlugins.isEmpty()) {
// If we didn't find any plugins above, someone must have messed with the log4j-core.jar.
// Search the standard package in the hopes we can find our core plugins.
+ LOGGER.warn(
+ "No Log4j plugin descriptor was found in the classpath.\n"
+ + "Falling back to scanning the `{}` package.\n{}",
+ LOG4J_PACKAGES,
+ PLUGIN_DESCRIPTOR_DOC);
builtInPlugins = PluginRegistry.getInstance().loadFromPackage(LOG4J_PACKAGES);
}
- mergeByName(newPlugins, builtInPlugins.get(categoryLowerCase));
+ mergeByName(newPlugins, builtInPlugins.get(categoryLowerCase), null);
// Next, iterate any Log4j2Plugin.dat files from OSGi Bundles
for (final Map<String, List<PluginType<?>>> pluginsByCategory :
PluginRegistry.getInstance().getPluginsByCategoryByBundleId().values()) {
- mergeByName(newPlugins, pluginsByCategory.get(categoryLowerCase));
+ mergeByName(newPlugins, pluginsByCategory.get(categoryLowerCase), null);
}
+ final List<String> scannedPluginClassNames = new ArrayList<>();
// Next iterate any packages passed to the static addPackage method.
for (final String pkg : PACKAGES) {
mergeByName(
newPlugins,
- PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase));
+ PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase),
+ scannedPluginClassNames);
}
// Finally iterate any packages provided in the configuration (note these can be changed at runtime).
if (packages != null) {
for (final String pkg : packages) {
mergeByName(
newPlugins,
- PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase));
+ PluginRegistry.getInstance().loadFromPackage(pkg).get(categoryLowerCase),
+ scannedPluginClassNames);
+ }
+ }
+ // Report package scanning deprecation.
+ if (!scannedPluginClassNames.isEmpty()) {
+ final Predicate<String> customPluginPredicate = PluginManager::isCustomPlugin;
+ final String standardPlugins = scannedPluginClassNames.stream()
+ .filter(customPluginPredicate.negate())
+ .collect(Collectors.joining("\n\t"));
+ if (!standardPlugins.isEmpty()) {
+ LOGGER.warn(
+ "The plugin descriptors for the following '{}' plugins are missing:\n\t{}\n{}",
Review Comment:
The last parameter contains the URL to a `faq.adoc` entry.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836#discussion_r1715384135
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.plugins.util;
+
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Collections.enumeration;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Enumeration;
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
+import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
+import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.test.Compiler;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.apache.logging.log4j.util.LoaderUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class PluginManagerTest {
+
+ private static final String PLUGIN_DESCRIPTOR =
+ "META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat";
+
+ @BeforeEach
+ void setUp() {
+ PluginRegistry.getInstance().clear();
+ }
+
+ @Test
+ @UsingStatusListener
+ @SuppressWarnings("deprecation")
+ void calling_addPackage_issues_warning(final ListStatusListener listener) {
+ try {
+ PluginManager.addPackage("com.example");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+
+ PluginManager.addPackages(singletonList("com.example"));
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ listener.clear();
+ } finally {
+ PluginManager.clearPackages();
+ }
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_issues_warning(final ListStatusListener listener) {
+ final ConfigurationSource source = ConfigurationSource.fromResource(
+ "customplugin/log4j2-741.xml", getClass().getClassLoader());
+ assertThat(source).as("Configuration source").isNotNull();
+ final Configuration configuration = new XmlConfigurationFactory().getConfiguration(null, source);
+ assertThat(configuration).as("Configuration").isNotNull();
+ configuration.initialize();
+ assertThat(configuration.getPluginPackages()).as("Plugin packages").contains("customplugin");
+ assertThat(listener.findStatusData(Level.WARN))
+ .anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
+ }
+
+ @Test
+ @UsingStatusListener
+ void using_packages_finds_custom_plugin(final ListStatusListener listener) throws Exception {
+ // To ensure our custom plugin is NOT included in the log4j plugin metadata file,
+ // we make sure the class does not exist until after the build is finished.
+ // So we don't create the custom plugin class until this test is run.
Review Comment:
Fixed in https://github.com/apache/logging-log4j2/pull/2836/commits/d2bdd02ca9add9cc155d7c8a65f1335bf8d5dd70.
The `FixedStringLayout.java.source` file is also used by `ResolverUtilTest` to compile multiple versions of the same class.
--
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] Improve plugin descriptor warnings (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #2836:
URL: https://github.com/apache/logging-log4j2/pull/2836
--
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