Skip to content
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

remove SLF4J dependency from spring-boot-starter-log4j2 #39627

Closed
codefromthecrypt opened this issue Feb 20, 2024 · 13 comments
Closed

remove SLF4J dependency from spring-boot-starter-log4j2 #39627

codefromthecrypt opened this issue Feb 20, 2024 · 13 comments
Labels
status: invalid An issue that we don't feel is valid

Comments

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Feb 20, 2024

Had a chat with @jonatan-ivanov who encouraged me to raise issues on things I feel are bugs. So, I feel that despite being intentionally done in #12649 (and really it started before that), spring-boot-starter-log4j2 having a dependency on SLF4J anything is a bug not a feature.

I can understand the fight over logback and looking for a way out, but it did take some time to understand this, as SLF4J and Log4J are different libraries, and coupling them creates issues that can be tricky to detect. The workaround is to manually exclude the dep, but anyway. over to you!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 20, 2024
@ciscoo
Copy link

ciscoo commented Feb 20, 2024

What issues specifically? SLF4J is a logging abstraction whereas Log4J is a concrete logging implementation/framework.

@codefromthecrypt
Copy link
Contributor Author

@ciscoo there are compatibility concerns with libraries and which SLF4J version is in use, notably logback. both SLF4J and Log4J have logging abstraction APIs, so I think many tend to think of these as alternatives vs one being an impl, even if sometimes one is. If you avoid forcing a specific SLF4J version, then there's no compatibility conflict in that ecosystem.

Specifically, in zipkin we have two server builds, and one uses log4J as an impl, the other logback. All of our dependencies that use SLF4J, use version 1.x and none use 2.x. There was an issue where certain logs just didn't appear anymore, and basically we found out it was over incompatibility (after noticing the log4j2 starter actually causes a version side effect on SLF4J). While we were able to get rid of the problem by excluding the dependency, the point that these are both abstractions. It is as weird for log4j2 starter to add a slf4j dep as it is for vise versa to be true, IMHO. you may not agree, but anyway that's why I raised the issue. hopefully to help the next person who discovers this by losing time troubleshooting missing logs.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 21, 2024
@philwebb
Copy link
Member

It's been quite a while since the logging starters were developed, but I think the logic behind adding SLFJ is that many libraries make use of it for logging and so we wanted to have it as a managed dependency. Whilst it's possible that a user might choose to use only Log4J APIs in their applications, they may need it indirectly when if they start depending on specific libraries. For example, I think Hikari uses SLF4J for logging.

I'm not sure what the best option here would be for typical users. If we don't manage the dependency we're likely to have folks hit the same problem but in a more random way (depending on what libraries they use and the transitive dependencies that get pulled in).

@codefromthecrypt Have you got more details of the SLF4J 1.x/2.x compatibility issue? Is the problem that libraries compiled against 1.x just don't log correctly when 2.x is used as a replacement?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Feb 21, 2024
@codefromthecrypt
Copy link
Contributor Author

@codefromthecrypt Have you got more details of the SLF4J 1.x/2.x compatibility issue? Is the problem that libraries compiled against 1.x just don't log correctly when 2.x is used as a replacement?

yeah this was the battle openzipkin/zipkin#3714

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 21, 2024
@wilkinsona
Copy link
Member

It is as weird for log4j2 starter to add a slf4j dep as it is for vise versa to be true

The SLF4J dependency is required to route logging that's performed using the SLF4J API into Log4j2. Similarly, we include the Log4j2 API in our Logback-based starter so that logging that's performed using the Log4j2 API can be routed into Logback through SLF4J. If we were to remove one or both, I believe that logging would be lost unless users performed some additional configuration.

From an API perspective, my understanding is that SLF4J 2.0 is backwards compatible with SLFJ 1.7. The area where things are incompatible is with the binding of SLF4J to a particular back end and there you need to match things up. This is why the SLF4J and Logback upgrades had to be done together.

Having read through openzipkin/zipkin#3714, I don't understand why logging was being lost. It would be very useful to see a minimal example of that so that we can fully understand the cause. Based on my current understanding of SLF4J's backwards compatibility, it should work. If it does not, it may be there there's a bug that can be fixed elsewhere.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 21, 2024
@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Feb 21, 2024

@reta if you recall any specifics on this let us know.

@wilkinsona let me try to summarize, as it sounds this is here to stay regardless. Correct me if I'm wrong!

Log4J does not require SLF4J, and neither does spring boot. boot has logback marked optional and works without it.

However there is gravity around logback in boot and versions should match. To defend against this, there's a dep on slf4j in the log4j2 starter, and that dep matches the version of logback set by the dependencies bom. This is a safeguard to prevent lost logs for folks using logback.

Folks that don't require slf4j or want to use an old version can exclude the dep (like we did in zipkin).

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 21, 2024
@wilkinsona
Copy link
Member

Log4J does not require SLF4J, and neither does spring boot. boot has logback marked optional and works without it.

Correct. However, we manage some libraries that do require SLF4J. That is to say, they use the SLF4J API for their logging and if it doesn't get routed somewhere, it gets lost.

However there is gravity around logback in boot and versions should match.

That's fair. Logback's our default which has some influence on the choices we make and it is vital that the version of Logback being used is compatible with the version of SLF4J being used. That said, there are also some constraints on the version of SLF4J being used even when Logback isn't being used. More on this below.

To defend against this, there's a dep on slf4j in the log4j2 starter, and that dep matches the version of logback set by the dependencies bom. This is a safeguard to prevent lost logs for folks using logback.

Note quite. It's a safeguard to prevent lost logs for anything that uses the SLF4J API. We manage a number of dependencies that do so, including:

  • ActiveMQ
  • Cassandra
  • Hikari CP
  • Jetty
  • Thymeleaf

Some of these libraries compile against SLF4J 2.0.x and may not work with SLF4J 1.x as there are new APIs in 2.0 that they may be using. From an API perspective, SLF4J 2.0 should be backwards compatible with 1.x so this leads us to using SLF4J 2.0.x as our managed version. I think that makes sense with or without Logback's involvement.

Folks that don't require slf4j or want to use an old version can exclude the dep (like we did in zipkin).

Correct. If you're not using anything that uses the SLF4J API for logging, you can exclude org.apache.logging.log4j:log4j-slf4j2-impl from our Log4j2 starter with no ill effects. With this exclusion in place, I'd check that org.slf4j:slf4j-api doesn't appear anywhere else in the dependency graph. If it does then you're probably going to lose logs.

@reta
Copy link

reta commented Feb 21, 2024

@reta if you recall any specifics on this let us know.

Yeah, sure, so there is no binary incompatibilities between LF4J 2.0.x and SLF4J 1.7.x, what changes is the discovery part (initialization). In context of openzipkin/zipkin#3714, we run into a mix where:

  • zipkin was using SLF4J 2.x
  • but one of the dependencies (in this case armenia) was (and still is) using SLF4J 1.7.x

The logs were not showing, and as per https://www.slf4j.org/faq.html#changesInVersion200, this is expected. Where Spring Boot made things a bit more complicated is that Logback is the default (#39627 (comment)) for it but not for zipkin

@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 21, 2024
@wilkinsona
Copy link
Member

Thanks, @reta. Unfortunately, I'm still confused as to why the problem was occurring.

In the SLF4J FAQ I cannot see a situation where nothing will be logged. At worst, you should see a warning that either a binding or a provider could not be found. The FAQ also states that 2.0.x should be a drop-in replacement. In your situation where you have some use of 2.0.x and some use of 1.7.x, resolving that version conflict by using 2.0.x and ensuring that there's a 2.0.x-compatible provider on the classpath such as Logback 1.4 or Log4j2's log4j-slf4j2-impl module should result in a working setup.

Can you please share a minimal example that demonstrates that this is not the case?

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 22, 2024
@reta
Copy link

reta commented Feb 22, 2024

Thanks @wilkinsona

In the SLF4J FAQ I cannot see a situation where nothing will be logged. At worst, you should see a warning that either a binding or a provider could not be found.

Yes, I believe this is the case, and the absence of the binding provider is the cause of no logs being shown up. I should probably clarify here that the logs from Spring Boot itself are there, but not from other components (like armenia)

Can you please share a minimal example that demonstrates that this is not the case?

Absolutely, I will try to have one shortly, thank you

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 22, 2024
@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 22, 2024
@reta
Copy link

reta commented Feb 23, 2024

@wilkinsona quick update, I was working on a simple reproducer (and I reproduced the issue) but it seems like the cause is slightly different and is related to the way log4j and slf4j were configured. To explain a bit more in details, using log4j-slf4j-impl with SLF4J 2.x API does not show any relevant logs (since the provider is not discovered):

		<dependency>
			<groupId>org.apache.logging.log4j</groupId>
			<artifactId>log4j-slf4j-impl</artifactId>
			<version>2.21.0</version>
		</dependency>

However replacing it with the log4j-slf4j2-impl (which should be used with SLF4J 2.x) brings the relevant logs back:

		<dependency>
			<groupId>org.apache.logging.log4j</groupId>
			<artifactId>log4j-slf4j2-impl</artifactId>
			<version>2.21.0</version>
		</dependency>

I think this is kind of expected - no provider, no logs.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 23, 2024
@wilkinsona
Copy link
Member

Yes, that's to be expected. If you're using SLF4J 2, you need to use log4j-slf4j2-impl. A Spring Boot 3.x app shouldn't use log4j-slf4j-impl as that's for SLF4J 1.x. I'll close this one as is it sounds like we've identified the root cause as being the use of the incorrect Log4j2 SLF4J implementation module.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2024
@wilkinsona wilkinsona added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Feb 24, 2024
@codefromthecrypt
Copy link
Contributor Author

I think the root takeaway here is "A Spring Boot 3.x app shouldn't use ... SLF4J 1.x". This was the thing that led to me opening this issue, as I mentioned Spring had an opinion on SLF4J 2, someone was surprised I would think that, and here we are. At least we are on the same page!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

6 participants