-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated log4j from version 1.2.17 to version 2.16.0 #6390
Conversation
The TR unit tests action had some errors: Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project traffic_router_shared: Compilation failure: Compilation failure:
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/Pkcs8.java:[18,32] package org.apache.logging.log4j does not exist
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/Pkcs8.java:[27,30] cannot find symbol
Error: symbol: class Logger
Error: location: class org.apache.traffic_control.traffic_router.secure.Pkcs8
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/BindPrivateKey.java:[18,32] package org.apache.logging.log4j does not exist
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/BindPrivateKey.java:[32,30] cannot find symbol
Error: symbol: class Logger
Error: location: class org.apache.traffic_control.traffic_router.secure.BindPrivateKey
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/Pkcs1KeySpecDecoder.java:[18,32] package org.apache.logging.log4j does not exist
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/Pkcs1KeySpecDecoder.java:[40,30] cannot find symbol
Error: symbol: class Logger
Error: location: class org.apache.traffic_control.traffic_router.secure.Pkcs1KeySpecDecoder
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/Pkcs8.java:[27,46] cannot find symbol
Error: symbol: variable Logger
Error: location: class org.apache.traffic_control.traffic_router.secure.Pkcs8
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/BindPrivateKey.java:[32,46] cannot find symbol
Error: symbol: variable Logger
Error: location: class org.apache.traffic_control.traffic_router.secure.BindPrivateKey
Error: /github/workspace/traffic_router/shared/src/main/java/org/apache/traffic_control/traffic_router/secure/Pkcs1KeySpecDecoder.java:[40,46] cannot find symbol
Error: symbol: variable Logger
Error: location: class org.apache.traffic_control.traffic_router.secure.Pkcs1KeySpecDecoder |
Weird.. i checked it locally and gave me no errors.. let me fix it |
Also added LogManager class.
.../core/src/main/java/org/apache/traffic_control/traffic_router/core/config/ConfigHandler.java
Outdated
Show resolved
Hide resolved
.../core/src/main/java/org/apache/traffic_control/traffic_router/core/config/ConfigHandler.java
Outdated
Show resolved
Hide resolved
.../core/src/main/java/org/apache/traffic_control/traffic_router/core/config/ConfigHandler.java
Outdated
Show resolved
Hide resolved
This reverts commit 22289f1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR should remove slf4j-log4j12
, since it log4j 1.2.17:
trafficcontrol/traffic_router/shared/pom.xml
Lines 114 to 118 in 411bb52
<dependency> | |
<groupId>org.slf4j</groupId> | |
<artifactId>slf4j-log4j12</artifactId> | |
<version>1.7.5</version> | |
</dependency> |
...router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/NameServer.java
Outdated
Show resolved
Hide resolved
.../core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/SignatureManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating log4j in #6390 seems to cause java.lang.LinkageError
in some tests:
java.lang.LinkageError: loader constraint violation: loader (instance of org/powermock/core/classloader/javassist/JavassistMockClassLoader) previously initiated loading for a different type with name "javax/management/MBeanServer"
Full stack trace for BindPrivateKeyTest (click to expand)
Running secure.BindPrivateKeyTest
ScriptEngineManager providers.next(): javax.script.ScriptEngineFactory: Provider jdk.nashorn.api.scripting.NashornScriptEngineFactory not a subtype
ERROR StatusLogger Could not reconfigure JMX
java.lang.LinkageError: loader constraint violation: loader (instance of org/powermock/core/classloader/javassist/JavassistMockClassLoader) previously initiated loading for a different type with name "javax/management/MBeanServer"
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClass(ClassLoader.java:757)
at org.powermock.core.classloader.javassist.JavassistMockClassLoader.loadUnmockedClass(JavassistMockClassLoader.java:90)
at org.powermock.core.classloader.MockClassLoader.loadClassByThisClassLoader(MockClassLoader.java:104)
at org.powermock.core.classloader.DeferSupportingClassLoader.loadClass1(DeferSupportingClassLoader.java:147)
at org.powermock.core.classloader.DeferSupportingClassLoader.loadClass(DeferSupportingClassLoader.java:98)
at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
at org.apache.logging.log4j.core.jmx.Server.unregisterAllMatching(Server.java:337)
at org.apache.logging.log4j.core.jmx.Server.unregisterLoggerContext(Server.java:261)
at org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:165)
at org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:141)
at org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:638)
at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:700)
at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:717)
at org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:272)
at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:155)
at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:47)
at org.apache.logging.log4j.LogManager.getContext(LogManager.java:196)
at org.apache.logging.log4j.LogManager.getLogger(LogManager.java:599)
at org.apache.traffic_control.traffic_router.secure.BindPrivateKey.<clinit>(BindPrivateKey.java:33)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:264)
at javassist.runtime.Desc.getClassObject(Desc.java:72)
at javassist.runtime.Desc.getClassType(Desc.java:181)
at javassist.runtime.Desc.getType(Desc.java:151)
at javassist.runtime.Desc.getType(Desc.java:107)
at secure.BindPrivateKeyTest.itDecodesPrivateKeyString(BindPrivateKeyTest.java:98)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:326)
at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:89)
at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:97)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:310)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:298)
at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87)
at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:218)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:160)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:134)
at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34)
at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:136)
at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:117)
at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:57)
at org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.271 sec
It looks like other people have encountered this, too, and adding
@PowerMockIgnore("javax.management.*")
to the test class seems to fix it. That fix would involve adding it to these classes:
public class KeyManagerTest { |
trafficcontrol/traffic_router/connector/src/test/java/secure/TomcatLifecycleListenerTest.java
Line 42 in 411bb52
public class TomcatLifecycleListenerTest { |
Line 52 in 411bb52
public class NameServerTest { |
Line 58 in 411bb52
public class TCPTest { |
Line 57 in 411bb52
public class UDPTest { |
Line 56 in 411bb52
public class ZoneManagerUnitTest { |
Line 53 in 411bb52
public class ZoneSignerImplTest { |
Line 36 in 411bb52
public class DeliveryServiceMatcherTest { |
Line 43 in 411bb52
public class CoverageZoneTest { |
Line 47 in 411bb52
public class AbstractServiceUpdaterTest { |
Line 49 in 411bb52
public class CoverageZoneTest { |
Line 41 in 411bb52
public class MaxmindGeolocationServiceTest { |
Line 47 in 411bb52
public class DNSRoutingMissesTest { |
Line 47 in 411bb52
public class GeoSortSteeringResultsTest { |
Lines 39 to 40 in 411bb52
@PowerMockIgnore("javax.net.ssl.*") | |
public class FetcherTest { |
trafficcontrol/traffic_router/shared/src/test/java/secure/BindPrivateKeyTest.java
Line 46 in 411bb52
public class BindPrivateKeyTest { |
…ing_log4j_module � Conflicts: � traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/util/Fetcher.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like java.lang.LinkageError
is fixed in most of the tests. Getting java.lang.LinkageError
in CertificateDataListenerTest
:
Running secure.CertificateDataListenerTest
ERROR StatusLogger Could not reconfigure JMX
java.lang.LinkageError: loader constraint violation: loader org.powermock.core.classloader.javassist.JavassistMockClassLoader @2447940d wants to load interface javax.management.MBeanServer. A different interface with the same name was previously loaded by 'bootstrap'. (javax.management.MBeanServer is in module java.management of loader 'bootstrap')
traffic_router/neustar/src/test/java/data/NeustarDatabaseUpdaterTest.java
Outdated
Show resolved
Hide resolved
traffic_router/neustar/src/test/java/data/TarExtractorTest.java
Outdated
Show resolved
Hide resolved
infrastructure/ansible/roles/traffic-router/templates/conf/log4j.xml.j2
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the content of both access.log
traffic_router.log
are also being outputted to the standard output of /opt/tomcat/bin/catalina.sh
:
Entered processConfig
Exiting processConfig: No json data to process
INFO 2021-12-15T21:49:11.445 [AsyncHttpClient-3-1] org.apache.traffic_control.traffic_router.core.config.ConfigHandler - Entered processConfig
INFO 2021-12-15T21:49:11.445 [AsyncHttpClient-3-1] org.apache.traffic_control.traffic_router.core.config.ConfigHandler - Exiting processConfig: No json data to process
Entered processConfig
Exiting processConfig: No json data to process
INFO 2021-12-15T21:49:16.441 [AsyncHttpClient-3-1] org.apache.traffic_control.traffic_router.core.config.ConfigHandler - Entered processConfig
INFO 2021-12-15T21:49:16.442 [AsyncHttpClient-3-1] org.apache.traffic_control.traffic_router.core.config.ConfigHandler - Exiting processConfig: No json data to process
To reproduce, open
trafficcontrol/infrastructure/cdn-in-a-box/traffic_router/run.sh
Lines 134 to 137 in a14f1e6
exec /opt/tomcat/bin/catalina.sh run & | |
fi; | |
tail -F $CATALINA_OUT $CATALINA_LOG $LOGFILE $ACCESSLOG |
tail -F
line with tail -F /dev/null
so you get nothing but process output.
With that tail -f /dev/null
line, the output looks like this:
Entered processConfig
Exiting processConfig: No json data to process
Entered processConfig
Exiting processConfig: No json data to process
<groupId>org.slf4j</groupId> | ||
<artifactId>slf4j-log4j12</artifactId> | ||
<version>1.7.5</version> | ||
<artifactId>slf4j-simple</artifactId> | ||
<version>1.7.32</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slf4j-log4j12
should be replaced with log4j-slf4j-impl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with slf4j-simple
? why does it need to be replaced with log4j-slf4j-impl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log4j-slf4j-impl
is what to use for log4j2
. From https://logging.apache.org/log4j/2.x/log4j-slf4j-impl:
The Log4j 2 SLF4J Binding allows applications coded to the SLF4J API to use Log4j 2 as the implementation.
[...]
log4j-slf4j-impl should be used with SLF4J 1.7.x releases or older.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not breaking anything to leave it as-is, though. This doesn't hold up the PR, maybe just something to do in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About log4j-slf4j-impl
:
- Its version should match the
log4j2
. version. log4j-slf4j-impl
depends onslf4j-api
, butasync-http-client
also depends onslf4j-api
:trafficcontrol/traffic_router/core/pom.xml
Lines 314 to 318 in ae390b1
<dependency> <groupId>org.asynchttpclient</groupId> <artifactId>async-http-client</artifactId> <version>2.12.1</version> </dependency>
As a result, if the versions ofslf4j-api
thatlog4j-slf4j-impl
andasync-http-client
depend on are different, TR will include two separate versions ofslf4j-api
. IMO,async-http-client
should be downgraded so that its requiredslf4j-api
version matches theslf4j-api
version thatlog4j-slf4j-impl
requires, meaningslf4j-api
only gets pulled in once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- log4j is upgraded to 2.16
- logs show up in the format that they used to.
- TR logs are logged to
traffic_router.log
and access logs are logged toaccess.log
- tests pass. There are some additional exceptions shown when running the TR unit tests, but it turns out that these exceptions were occurring before anyway, this is just the first time that they show up in the test logs.
Looks good to me!
* updated log4j from version 1 to 2 * Updated pom.xml to include pom type tag * Updated pom.xml to remove pom type tag and use log4j-core artifactID. Also added LogManager class. * Updated java files to use LogManager class instead of logger class for ver 2.14.1 * Updated java files to use LogManager class instead of logger class for ver 2.14.1-1 * Un-optimized import statements * Updated setLevel class for getLogger and getRootLogger. * Updated setLevel class for getLogger and getRootLogger-1 * Imported Record class from Maven to remove ambiguity. * Revert "Imported Record class from Maven to remove ambiguity." This reverts commit 22289f1. * Fixed when clause with LogManager. * Fixed when clause with LogManager-1 * updated CHANGELOG.md * Fixed based on review comments * Fixed based on review comments - 1 * updated import path for ConsoleAppender, PatternLayout, RollingFileAppender classes. * updated log4j version to 2.15.0 * Added slf4j-simple dependency. * removed file:// to log INFO level messages. * removed file:// to log INFO level messages-1 * Migrated log4j properties to log4j2.xml * Updated log4j to version 2.16.0 * added fileName property * Updated date format based on review comments. * Added new line for xml files. * Added AppenderRef for traffic_router.log, access.log and console. * Removed typo character and fixed xml tags. * Updated log4j in ansible. * Updated based on review comments. * Added threshold level to main.yml and log4j2.xml * Added PatternLayout level to main.yml * Added additivity="false" to avoid printing logs twice (cherry picked from commit 070df30)
* updated log4j from version 1 to 2 * Updated pom.xml to include pom type tag * Updated pom.xml to remove pom type tag and use log4j-core artifactID. Also added LogManager class. * Updated java files to use LogManager class instead of logger class for ver 2.14.1 * Updated java files to use LogManager class instead of logger class for ver 2.14.1-1 * Un-optimized import statements * Updated setLevel class for getLogger and getRootLogger. * Updated setLevel class for getLogger and getRootLogger-1 * Imported Record class from Maven to remove ambiguity. * Revert "Imported Record class from Maven to remove ambiguity." This reverts commit 22289f1. * Fixed when clause with LogManager. * Fixed when clause with LogManager-1 * updated CHANGELOG.md * Fixed based on review comments * Fixed based on review comments - 1 * updated import path for ConsoleAppender, PatternLayout, RollingFileAppender classes. * updated log4j version to 2.15.0 * Added slf4j-simple dependency. * removed file:// to log INFO level messages. * removed file:// to log INFO level messages-1 * Migrated log4j properties to log4j2.xml * Updated log4j to version 2.16.0 * added fileName property * Updated date format based on review comments. * Added new line for xml files. * Added AppenderRef for traffic_router.log, access.log and console. * Removed typo character and fixed xml tags. * Updated log4j in ansible. * Updated based on review comments. * Added threshold level to main.yml and log4j2.xml * Added PatternLayout level to main.yml * Added additivity="false" to avoid printing logs twice (cherry picked from commit 070df30)
* updated log4j from version 1 to 2 * Updated pom.xml to include pom type tag * Updated pom.xml to remove pom type tag and use log4j-core artifactID. Also added LogManager class. * Updated java files to use LogManager class instead of logger class for ver 2.14.1 * Updated java files to use LogManager class instead of logger class for ver 2.14.1-1 * Un-optimized import statements * Updated setLevel class for getLogger and getRootLogger. * Updated setLevel class for getLogger and getRootLogger-1 * Imported Record class from Maven to remove ambiguity. * Revert "Imported Record class from Maven to remove ambiguity." This reverts commit 22289f1. * Fixed when clause with LogManager. * Fixed when clause with LogManager-1 * updated CHANGELOG.md * Fixed based on review comments * Fixed based on review comments - 1 * updated import path for ConsoleAppender, PatternLayout, RollingFileAppender classes. * updated log4j version to 2.15.0 * Added slf4j-simple dependency. * removed file:// to log INFO level messages. * removed file:// to log INFO level messages-1 * Migrated log4j properties to log4j2.xml * Updated log4j to version 2.16.0 * added fileName property * Updated date format based on review comments. * Added new line for xml files. * Added AppenderRef for traffic_router.log, access.log and console. * Removed typo character and fixed xml tags. * Updated log4j in ansible. * Updated based on review comments. * Added threshold level to main.yml and log4j2.xml * Added PatternLayout level to main.yml * Added additivity="false" to avoid printing logs twice (cherry picked from commit 070df30)
Closes: #5782
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
Run unit tests for Traffic Router
If this is a bugfix, which Traffic Control versions contained the bug?
Not a bugfix
PR submission checklist
There hasn't been any upgrade to TR logic, hence no new tests have been added or documentation have been updated.