log4j-core >= 2.22.0: Missing spotbugs-annotations dependency

[chrisribble]

Description

Commit 8996e9f added @SuppressFBWarnings annotations to some constructors, but the corresponding Maven dependency was not re-configured so that it would be available at compile time (at least for Gradle projects).

This results in compile warnings while building code that references these classes (org.apache.logging.log4j.core.LoggerContext in my application).

In all of my projects, I switch warnings to errors and fail the build, so this makes log4j-core >= 2.22.0 unusable without hacking my build configuration.

I suggest adding the appropriate Maven dependencies so that the annotations are added to the compile classpath in Maven/Gradle.

Adding the following dependency in Gradle is sufficient to avoid the compile warnings, but needing to do this in each of my applications/subprojects is sub-optimal.

compileOnly 'com.github.spotbugs:spotbugs-annotations:4.8.3'

Configuration

Version: 2.22.0 / 2.22.1

Operating system: Linux

JDK: OpenJDK 64-Bit Server VM Temurin-17.0.8.1+1

Logs

$ ./gradlew clean assemble
Configuration on demand is an incubating feature.

> Task :compileJava FAILED
/home/chris/.gradle/caches/modules-2/files-2.1/org.apache.logging.log4j/log4j-core/2.22.1/7183a25510a02ad00cc6a95d3b3d2a7d3c5a8dc4/log4j-core-2.22.1.jar(/org/apache/logging/log4j/core/LoggerContext.class): warning: Cannot find annotation method 'value()' in type 'SuppressFBWarnings': class file for edu.umd.cs.findbugs.annotations.SuppressFBWarnings not found
/home/chris/.gradle/caches/modules-2/files-2.1/org.apache.logging.log4j/log4j-core/2.22.1/7183a25510a02ad00cc6a95d3b3d2a7d3c5a8dc4/log4j-core-2.22.1.jar(/org/apache/logging/log4j/core/LoggerContext.class): warning: Cannot find annotation method 'justification()' in type 'SuppressFBWarnings'
error: warnings found and -Werror specified
1 error
2 warnings

[ppkarwasz]

Hi @chrisribble,

The dependencies on various annotation libraries are there, but they are in Maven's provided scope. This scope is the equivalent of Gradle's compileOnly configuration and is not transitive, so consumers of the artifacts don't get them automatically.

This scope was chosen since these libraries contain only annotations (which cause no exception at runtime if missing) and most of them has a retention of CLASS, so they are visible only to the compiler. Most users don't need them, since accessing log4j-core programmatically is not recommended (it should be a runtimeOnly dependency).

You can disable the compiler warnings with -Xlint:all,-classFile.

May I ask why do you need to access org.apache.logging.log4j.core.LoggerContext in all your projects? Did you handle the case of users using another Log4j API implementation (e.g. log4j-to-slf4j)? Maybe this an opportunity to refactor your code and either put log4j-core-specific code in a separate library that will also handle other logging backends.

[chrisribble]

@ppkarwasz,

I'm calling Configurator.shutdown(loggerContext); on the org.apache.logging.log4j.core.LoggerContext in a custom shutdown hook that handles graceful shutdown of my application with it receives SIGTERM. It's the very last thing that happens in our shutdown hook before the application exits.

This is not a "web" application and we also explicitly disable log4j's built-in shutdown hooks. Originally, we did this because we noticed that the built-in shutdown hook was running in parallel with our shutdown hook and resulted in non-deterministic log output (since the hooks were racing). We assumed it was important to shut down log4j and added the explicit shutdown.

However, it seems like it's not really all that important to explicitly shut down log4j in this scenario, since we're just logging to the console. k8s ships our console logs for us and we aren't using async logging. In my local testing with docker-compose stop it seems like I always see the log line at the end of the shutdown sequence even when I remove the call to Configurator.shutdown(loggerContext);.

So yeah, it seems like I can just remove this along with the explicit dependency on log4j-core. Doing so fixes the compile warnings too, of course.

[ppkarwasz]

I'm calling Configurator.shutdown(loggerContext); on the org.apache.logging.log4j.core.LoggerContext in a custom shutdown hook that handles graceful shutdown of my application with it receives SIGTERM. It's the very last thing that happens in our shutdown hook before the application exits.

You can use LogManager.shutdown() from log4j-api to obtain the same effect. The advantage is that we can implement it in different ways for different logging backends.

This is not a "web" application and we also explicitly disable log4j's built-in shutdown hooks. Originally, we did this because we noticed that the built-in shutdown hook was running in parallel with our shutdown hook and resulted in non-deterministic log output (since the hooks were racing). We assumed it was important to shut down log4j and added the explicit shutdown.

Yes, the lack of a determined order among shutdown hooks is somehow problematic.

[vy]

You can disable the compiler warnings with -Xlint:all,-classFile

I think we should not tell users striving for -Xlint:all-grade quality to relax their measures when it is our library breaking them, since this contradicts with the goal of the user.

You can use LogManager.shutdown() from log4j-api to obtain the same effect.

Agreed. This indeed appears like the best way forward.

@chrisribble, since you 👍'ed this resolution, I am closing the issue.

[ppkarwasz]

You can disable the compiler warnings with -Xlint:all,-classFile

I think we should not tell users striving for -Xlint:all-grade quality to relax their measures when it is our library breaking them, since this contradicts with the goal of the user.

Sorry, you are right. The optimal solution is to spread Spotbugs usage, which has many more tests than -Xlint:all.

We specifically try to describe every Spotbugs suppression, so that developers are for example aware that calling the LoggerContext(String, Object, String) constructor with a user-provided configLocn is a security risk.