Skip to content

Commit 715c97a

Browse files
ppkarwaszvy
ppkarwasz
and
vy
committed
Improve plugin descriptor warnings
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`. The `PluginManager` tests are revamped to: - Look for a status logger warning if a deprecated plugin scanning feature is used. - Look for a status logger warning if no plugin descriptor is available. This addresses @vlsi suggestions from apache/jmeter#5937 (comment) and fixes #2835. Co-authored-by: Volkan Yazıcı <[email protected]>
1 parent 505a28e commit 715c97a

File tree

12 files changed

+458
-140
lines changed

12 files changed

+458
-140
lines changed

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerPackagesTest.java

Lines changed: 0 additions & 88 deletions
This file was deleted.

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/PluginManagerTest.java

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.config.plugins.util;
18+
19+
import static java.util.Collections.emptyEnumeration;
20+
import static java.util.Collections.enumeration;
21+
import static java.util.Collections.singletonList;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
24+
25+
import java.io.File;
26+
import java.io.IOException;
27+
import java.net.URL;
28+
import java.nio.file.Files;
29+
import java.nio.file.Path;
30+
import java.util.Enumeration;
31+
import org.apache.commons.io.FileUtils;
32+
import org.apache.logging.log4j.Level;
33+
import org.apache.logging.log4j.core.config.Configuration;
34+
import org.apache.logging.log4j.core.config.ConfigurationSource;
35+
import org.apache.logging.log4j.core.config.Node;
36+
import org.apache.logging.log4j.core.config.plugins.processor.PluginCache;
37+
import org.apache.logging.log4j.core.config.plugins.processor.PluginEntry;
38+
import org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor;
39+
import org.apache.logging.log4j.core.config.xml.XmlConfigurationFactory;
40+
import org.apache.logging.log4j.core.test.Compiler;
41+
import org.apache.logging.log4j.status.StatusData;
42+
import org.apache.logging.log4j.test.ListStatusListener;
43+
import org.apache.logging.log4j.test.junit.UsingStatusListener;
44+
import org.apache.logging.log4j.util.LoaderUtil;
45+
import org.junit.jupiter.api.BeforeEach;
46+
import org.junit.jupiter.api.Test;
47+
48+
class PluginManagerTest {
49+
50+
@BeforeEach
51+
void setUp() {
52+
PluginRegistry.getInstance().clear();
53+
}
54+
55+
@Test
56+
@UsingStatusListener
57+
@SuppressWarnings("deprecation")
58+
void calling_addPackage_issues_warning(final ListStatusListener listener) {
59+
try {
60+
PluginManager.addPackage("com.example");
61+
assertThat(listener.findStatusData(Level.WARN))
62+
.anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
63+
listener.clear();
64+
65+
PluginManager.addPackages(singletonList("com.example"));
66+
assertThat(listener.findStatusData(Level.WARN))
67+
.anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
68+
listener.clear();
69+
} finally {
70+
PluginManager.clearPackages();
71+
}
72+
}
73+
74+
@Test
75+
@UsingStatusListener
76+
void using_packages_issues_warning(final ListStatusListener listener) {
77+
final ConfigurationSource source = ConfigurationSource.fromResource(
78+
"customplugin/log4j2-741.xml", getClass().getClassLoader());
79+
assertThat(source).as("Configuration source").isNotNull();
80+
final Configuration configuration = new XmlConfigurationFactory().getConfiguration(null, source);
81+
assertThat(configuration).as("Configuration").isNotNull();
82+
configuration.initialize();
83+
assertThat(configuration.getPluginPackages()).as("Plugin packages").contains("customplugin");
84+
assertThat(listener.findStatusData(Level.WARN))
85+
.anySatisfy(PluginManagerTest::assertContainsPackageScanningLink);
86+
}
87+
88+
@Test
89+
@UsingStatusListener
90+
void using_packages_finds_custom_plugin(final ListStatusListener listener) throws Exception {
91+
// To ensure our custom plugin is NOT included in the log4j plugin metadata file,
92+
// we make sure the class does not exist until after the build is finished.
93+
// So we don't create the custom plugin class until this test is run.
94+
final URL resource = PluginManagerTest.class.getResource("/customplugin/FixedStringLayout.java.source");
95+
assertThat(resource).isNotNull();
96+
final File orig = new File(resource.toURI());
97+
final File f = new File(orig.getParentFile(), "FixedStringLayout.java");
98+
assertDoesNotThrow(() -> FileUtils.copyFile(orig, f));
99+
// compile generated source
100+
// (switch off annotation processing: no need to create Log4j2Plugins.dat)
101+
Compiler.compile(f, "-proc:none");
102+
assertDoesNotThrow(() -> FileUtils.delete(f));
103+
// load the compiled class
104+
Class.forName("customplugin.FixedStringLayout");
105+
106+
final PluginManager manager = new PluginManager(Node.CATEGORY);
107+
manager.collectPlugins(singletonList("customplugin"));
108+
assertThat(manager.getPluginType("FixedString"))
109+
.as("Custom unregistered plugin")
110+
.isNotNull();
111+
assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
112+
assertThat(message.getLevel()).isEqualTo(Level.WARN);
113+
assertThat(message.getMessage().getFormattedMessage())
114+
.as("Status logger message")
115+
// The message specifies which plugin is not registered
116+
.contains("customplugin.FixedStringLayout")
117+
// The message provides a link to the registration instructions
118+
.contains("https://logging.apache.org/log4j/2.x/manual/plugins.html#plugin-registry");
119+
});
120+
}
121+
122+
@Test
123+
@UsingStatusListener
124+
void missing_plugin_descriptor_issues_warning(final ListStatusListener listener) {
125+
final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
126+
try {
127+
Thread.currentThread()
128+
.setContextClassLoader(new FilteringClassLoader(getClass().getClassLoader(), null));
129+
assertThat(LoaderUtil.getClassLoader().getResource(PluginProcessor.PLUGIN_CACHE_FILE))
130+
.isNull();
131+
final PluginManager manager = new PluginManager(Node.CATEGORY);
132+
manager.collectPlugins(null);
133+
assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
134+
assertThat(message.getLevel()).isEqualTo(Level.WARN);
135+
assertThat(message.getMessage().getFormattedMessage())
136+
.as("Status logger message")
137+
// The message provides a link to plugin descriptor troubleshooting.
138+
.contains("https://logging.apache.org/log4j/2.x/faq.html#plugin-descriptors");
139+
});
140+
} finally {
141+
Thread.currentThread().setContextClassLoader(tccl);
142+
}
143+
}
144+
145+
/**
146+
* Tests if removing descriptors of standard Log4j artifacts and replacing them with a custom one issues warnings.
147+
* <p>
148+
* This often happens in shading scenarios.
149+
* </p>
150+
*/
151+
@Test
152+
@UsingStatusListener
153+
void corrupted_plugin_descriptor_issues_warning(final ListStatusListener listener) throws IOException {
154+
// Create empty descriptor
155+
final Path customDescriptor = Files.createTempFile("Log4j2Plugins", ".dat");
156+
final URL descriptorUrl = customDescriptor.toUri().toURL();
157+
final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
158+
try {
159+
// Generate a plugin entry to have a non-empty plugin descriptor
160+
// For this test case it doesn't really matter if the class exists or not.
161+
// This test will not instantiate the plugin.
162+
final PluginEntry entry = new PluginEntry();
163+
entry.setKey("CustomPlugin");
164+
entry.setClassName("com.example.NotExistingClass");
165+
entry.setName("CustomPlugin");
166+
entry.setCategory(Node.CATEGORY);
167+
// Generate descriptor with single plugin
168+
final PluginCache customCache = new PluginCache();
169+
customCache.getCategory(Node.CATEGORY).put("CustomPlugin", entry);
170+
customCache.writeCache(Files.newOutputStream(customDescriptor));
171+
172+
Thread.currentThread()
173+
.setContextClassLoader(new FilteringClassLoader(getClass().getClassLoader(), descriptorUrl));
174+
175+
assertThat(LoaderUtil.getClassLoader().getResource(PluginProcessor.PLUGIN_CACHE_FILE))
176+
.isEqualTo(descriptorUrl);
177+
final PluginManager manager = new PluginManager(Node.CATEGORY);
178+
// Try loading some standard Log4j plugins
179+
manager.collectPlugins(singletonList("org.apache.logging.log4j"));
180+
assertThat(manager.getPluginType("Console")).isNotNull();
181+
assertThat(listener.findStatusData(Level.WARN)).anySatisfy(message -> {
182+
assertThat(message.getLevel()).isEqualTo(Level.WARN);
183+
assertThat(message.getMessage().getFormattedMessage())
184+
.as("Status logger message")
185+
// The message specifies which standard plugin is not registered
186+
.contains("org.apache.logging.log4j.core.appender.ConsoleAppender")
187+
// The message provides a link to plugin descriptor troubleshooting.
188+
.contains("https://logging.apache.org/log4j/2.x/faq.html#plugin-descriptors");
189+
});
190+
} finally {
191+
Thread.currentThread().setContextClassLoader(tccl);
192+
Files.delete(customDescriptor);
193+
}
194+
}
195+
196+
private static void assertContainsPackageScanningLink(final StatusData message) {
197+
assertThat(message.getLevel()).isEqualTo(Level.WARN);
198+
assertThat(message.getMessage().getFormattedMessage())
199+
// The message specifies
200+
.contains("https://logging.apache.org/log4j/2.x/faq.html#package-scanning");
201+
}
202+
203+
private static final class FilteringClassLoader extends ClassLoader {
204+
205+
private final URL descriptorUrl;
206+
207+
private FilteringClassLoader(final ClassLoader parent, final URL descriptorUrl) {
208+
super(parent);
209+
this.descriptorUrl = descriptorUrl;
210+
}
211+
212+
@Override
213+
public URL getResource(final String name) {
214+
return PluginProcessor.PLUGIN_CACHE_FILE.equals(name) ? descriptorUrl : super.getResource(name);
215+
}
216+
217+
@Override
218+
public Enumeration<URL> getResources(final String name) throws IOException {
219+
return PluginProcessor.PLUGIN_CACHE_FILE.equals(name)
220+
? descriptorUrl != null ? enumeration(singletonList(descriptorUrl)) : emptyEnumeration()
221+
: super.getResources(name);
222+
}
223+
}
224+
}

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/util/ResolverUtilTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Map;
3737
import java.util.stream.Stream;
3838
import org.apache.logging.log4j.core.config.plugins.util.PluginRegistry.PluginTest;
39+
import org.apache.logging.log4j.core.test.Compiler;
3940
import org.junit.jupiter.api.Test;
4041
import org.junit.jupiter.api.io.TempDir;
4142
import org.junit.jupiter.params.ParameterizedTest;
@@ -90,7 +91,7 @@ public void testExtractPathFromJarUrlNotDecodedIfFileExists() throws Exception {
9091
private void testExtractPathFromJarUrlNotDecodedIfFileExists(final String existingFile)
9192
throws MalformedURLException, UnsupportedEncodingException, URISyntaxException {
9293
URL url = ResolverUtilTest.class.getResource(existingFile);
93-
if (!url.getProtocol().equals("jar")) {
94+
if (!"jar".equals(url.getProtocol())) {
9495
// create fake jar: URL that resolves to existing file
9596
url = new URL("jar:" + url.toExternalForm() + "!/some/entry");
9697
}
@@ -170,7 +171,9 @@ static void compile(final File tmpDir, final String suffix) throws Exception {
170171
.replaceAll("customplugin", "customplugin" + suffix);
171172
Files.write(f.toPath(), content.getBytes());
172173

173-
PluginManagerPackagesTest.compile(f);
174+
// compile generated source
175+
// (switch off annotation processing: no need to create Log4j2Plugins.dat)
176+
Compiler.compile(f, "-proc:none");
174177
}
175178

176179
static void createJar(final URI jarURI, final File workDir, final File f) throws Exception {

‎log4j-core-test/src/test/resources/customplugin/FixedStringLayout.java.source

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
2626
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
2727
import org.apache.logging.log4j.core.layout.AbstractStringLayout;
2828

29+
/**
30+
* A plugin template used in PluginManagerTest and ResolverUtilTest.
31+
*
32+
* The tests modify and compile this class themselves.
33+
*/
2934
@Plugin(name = "FixedString", category = "Core", elementType = "layout", printObject = true)
3035
public class FixedStringLayout extends AbstractStringLayout {
3136

‎log4j-core-test/src/test/resources/customplugin/log4j2-741.xml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,13 @@
1515
~ See the License for the specific language governing permissions and
1616
~ limitations under the License.
1717
-->
18-
<Configuration status="off" packages="customplugin">
18+
<Configuration packages="customplugin">
1919
<Appenders>
20-
<List name="List">
21-
<FixedString fixedString="abc123XYZ" />
22-
</List>
20+
<Console name="CONSOLE"/>
2321
</Appenders>
2422
<Loggers>
25-
<Root level="trace">
26-
<AppenderRef ref="List" />
23+
<Root level="INFO">
24+
<AppenderRef ref="CONSOLE" />
2725
</Root>
2826
</Loggers>
2927
</Configuration>

0 commit comments

Comments
 (0)