add log4j-core for test logging

[EdColeman]

Updates to log4j versions require log4j-core to be present for test logging.

[dlmarion]

If you put this in the parent pom, in the dependencies section (not dependencyManagement), then I believe that each module will automatically inherit it. Just thinking that we could make the change in one place.

[EdColeman]

Would the proper scope still be test - or is provided more appropriate - it needs to make it to the class paths for tests - not sure what is required for "normal" logging

[dlmarion]

I tested with the test scope, and that failed. The monitor needs it apparently. I'm testing now with the provided scope.

[dlmarion]

So, I added the following to the parent pom:

@@ -695,6 +695,13 @@
       <artifactId>spotbugs-annotations</artifactId>
       <optional>true</optional>
     </dependency>
+    <dependency>
+      <!-- needed because https://issues.apache.org/jira/browse/LOG4J2-3601 -->
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-core</artifactId>
+      <scope>provided</scope>
+      <optional>true</optional>
+    </dependency>

and removed the dependency declaration in assemble/pom.xml, core/pom.xml, and server/monitor/pom.xml. mvn clean package completed successfully, but log4j-core is not in the assembly (I assume it's supposed to be there for the monitor). So, the declaration in assemble/pom.xml may need to remain. Further tests are likely needed with this approach.

[EdColeman]

What I first noticed was when running a test in an ide - the warning about it missing and using SimpleLogger was printed - but no logs showed up in the ide run pane

[dlmarion]

What I first noticed was when running a test in an ide - the warning about it missing and using SimpleLogger was printed - but no logs showed up in the ide run pane

Yes, I had the same issue in accumulo-classloaders yesterday. Adding log4j-core resolved the issue.

[EdColeman]

Moving the declaration to main pom.xml with test scope seems to work for logging in the IDE and mvn clean package passed for me. To confirm the logging I used a single IT test - so not really comprehensive, but this seem to move us closer.

[dlmarion]

Did you mean to add log4j-slf4j2-impl to the main pom? I thought you were adding log4j-core ?

[EdColeman]

log4j-slf4j2-impl was inadvertent - running a mvn clean now to see if that changes things.

[ctubbsii]

This should already be on the class path. It's brought in transitively through accumulo-monitor, and we shouldn't have any direct dependencies on it, only transitive runtime dependencies on it. The test module has a direct dependency on the accumulo-monitor.

Ultimately, this should be fixed by 2.19.1, which should re-add the transitive dependency for log4j-slf4j2-impl (https://issues.apache.org/jira/browse/LOG4J2-3601), but it hasn't been done yet, and some of the devs seemed confused about why they needed to fix that. So, I don't know when it will happen.

If this helps fix something during testing in an IDE, I'm fine with adding it. It's not going to hurt.

[cshannon]

The current version of this change doesn't actually solve anything because as @ctubbsii pointed out the dependency is already transitive from accumulo-monitor. I can see the dependency is pulled in transitively both in Intellij and the command line and running tests from the test module from Intellij logging is working fine in the test module.

However, where I do see a problem is other modules. For example, running BulkImporterTest doesn't have logging as there is not longer a transitive dependency on log4j-core for server/base module.

I will go through and update the PR and push a commit to fix this.

[cshannon]

This has been fixed with my latest commit. Essentially, due to the bug, we have to add a test scope dependency for log4j-core everywhere we already have a dependency for log4j-slf4j2-impl unless there is already a dependency on accumulo-monitor which pulls in log4j-core already. Notably, the test and accumulo-minicluster modules already pull in accumulo-monitor and don't need log4j-core.