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

Removes bridges from Log4j 3 #2924

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

ppkarwasz
Copy link
Contributor

The log4j-slf4j-impl, log4j-slf4j2-impl, log4j-to-jul and log4j-to-slf4j bridges are practically identical to their 2.x counterparts.

Unlike log4j-jpl (which requires Java 11) and log4j-jul (which requires breaking changes to remove optional log4j-core dependency), these bridges do not profit from a new release.

As agreed on dev@logging, this PR removes them and cleans up the dependencies they used.

The `log4j-slf4j-impl`, `log4j-slf4j2-impl`, `log4j-to-jul` and `log4j-to-slf4j` bridges are practically identical to their
2.x counterparts.

Unlike `log4j-jpl` (which requires Java 11) and `log4j-jul` (which requires breaking changes to remove optional `log4j-core` dependency), these bridges do not profit from a new release.

As agreed on [`dev@logging`](https://lists.apache.org/thread/hmngcrtw4fmc6zx59rsowllv6cnb01jr), this PR removes them and cleans up the dependencies they used.
@ppkarwasz ppkarwasz self-assigned this Sep 6, 2024
@gemmellr
Copy link
Member

Doesnt saying they won't benefit from a new release somewhat overlook how they are actually used and expected to behave by many users?

Users will often only have a Log4j related dependency on e.g log4j-slf4j-impl or log4j-slf4j2-impl currently, and fully expect that to provide them the Log4j [core] impl of the same version plus the bridging bits needed to make it the impl for the related SLF4J version. Following this PR removing them from 3.x that matched alignment wont be there any longer, and seems likely to complicate updates for users.

I realise you have added the 'we expect you to use the bom' warning to the docs now in this PR and I assume such users probably wont be affected (depending on what other boms etc they use, orders they are imported etc, which could still interact quite poorly), with the bom presumably updated to contain the mixed versions and override the old -core version dep the old bridge modules would otherwise use, though that doesnt help folks update that are quite likely not to be using the bom for their single-bridge-dependency usage currently.

With the 'no matching adapter version' approach in new final Log4J 3 release versions, it also seems likely some (many?) users are a) not to notice Log4J 3 final even exists for a period (ignoring betas, which you unfortunately have already released all these bridge modules in already, alas making those remain the newest versions when removed here..), because they possibly dont actually depend on log4j-core directly but rather just the bridge dep, and b) then once they do discover it exists, possibly still cant use Log4j 3 immediately without changing their dependency handling in some way (e.g depending on whether or not they already use the bom, or have interaction issues with other depManagement etc they have).

There also seemed to be an argument made about Java versions, with some APIs targeting e.g Java 8, however if Log4J 3 itself will require Java 17 it would actually seem far nicer if the bridge impls used with Log4J 3 also required Java 17 too, rather than a mix of requirements there will be in this approach.

It also feels weirder to keep some but not others.

TLDR: even if the code is absolutely the same as 2.x, for me I dont see a great benefit to removing most of these bridges from 3.x (plus any time you release 2.x with important fixes or updates to them, you will still need to also release 3.x to bring them there), whilst it does seem likely to cause various users pain. It also feels weirder to keep some but not others.

@ppkarwasz
Copy link
Contributor Author

Doesnt saying they won't benefit from a new release somewhat overlook how they are actually used and expected to behave by many users?

Users will often only have a Log4j related dependency on e.g log4j-slf4j-impl or log4j-slf4j2-impl currently, and fully expect that to provide them the Log4j [core] impl of the same version plus the bridging bits needed to make it the impl for the related SLF4J version. Following this PR removing them from 3.x that matched alignment wont be there any longer, and seems likely to complicate updates for users.

Nice remark! 💯

Effectively users consider log4j-slf4j-impl as "The Apache Log4j SLF4J API binding to Log4j 2 Core" and they expect to pull in log4j-core too. We corrected the name of the artifact to "SLF4J 1 Binding for Log4j API", but the old one stuck.

IMHO the correct way to add Log4j Core to a Maven project is:

<!-- Bridge -->
<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-slf4j2-impl</artifactId>
  <scope>runtime</scope>
</dependency>
<!-- Log4j API implementation -->
<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-core</artifactId>
  <scope>runtime</scope>
</dependency>

The is the exact equivalent to the way Logback should be added:

<!-- Bridge -->
<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-to-slf4j</artifactId>
  <scope>runtime</scope>
</dependency>
<!-- SLF4J implementation -->
<dependency>
  <groupId>ch.qos.logback</groupId>
  <artifactId>logback-classic</artifactId>
  <scope>runtime</scope>
</dependency>

These are the recommended methods on our installation page (see Installing Log4j Core and Installing Logback).

@gemmellr
Copy link
Member

Doesnt saying they won't benefit from a new release somewhat overlook how they are actually used and expected to behave by many users?
Users will often only have a Log4j related dependency on e.g log4j-slf4j-impl or log4j-slf4j2-impl currently, and fully expect that to provide them the Log4j [core] impl of the same version plus the bridging bits needed to make it the impl for the related SLF4J version. Following this PR removing them from 3.x that matched alignment wont be there any longer, and seems likely to complicate updates for users.

Nice remark! 💯

Effectively users consider log4j-slf4j-impl as "The Apache Log4j SLF4J API binding to Log4j 2 Core" and they expect to pull in log4j-core too. We corrected the name of the artifact to "SLF4J 1 Binding for Log4j API", but the old one stuck.

The old description/name stuck because that name is what the module is to almost every user of it, and is what it still actually does do, and is what it has [almost] always done. As the log4j-slf4j-impl module name itself reflects, it acts as the Log4j SLF4F Impl, not just Log4j API to SLF4J API.

The bridge dependency for SLF4J -> Log4j has shipped the Log4j impl for every variant of the SLF4J binding so far as I know. It does that for earlier the SLF4J-homed variant for Log4J 1 (then reload4j). It does it for the the Log4j-homed Log4J 2.x variant for SLF4J 1. It does it for the Log4J-homed variant Log4J 2.x for SLF4J 1.8. People immediately complained (https://issues.apache.org/jira/browse/LOG4J2-3601) when the core dep was dropped from the SLF4J 2 binding on its first release, and it was restored in the next, and remains. It still supplies log4j-core for the Log4J 3.x variants to be deleted by this PR.

Which makes sense. Its what almost all users seem to expect and want (and are already long used to) a 'log4j-slf4j<version>-impl' dependency to do; supply a Log4J impl of that version to handle output for SLF4J API.

The low number of people who actually want to implement a binding from SLF4J to Log4J API without using Log4J Core, because they implemented log4j-api themselves, can still easily use that artifact by excluding the log4j-core dep. Though it seems just as likely they wouldnt use slf4j-log4j-impl at all as if they have their own logging framework impl.

IMHO the correct way to add Log4j Core to a Maven project is:

<!-- Bridge -->
<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-slf4j2-impl</artifactId>
  <scope>runtime</scope>
</dependency>
<!-- Log4j API implementation -->
<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-core</artifactId>
  <scope>runtime</scope>
</dependency>

The is the exact equivalent to the way Logback should be added:

<!-- Bridge -->
<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-to-slf4j</artifactId>
  <scope>runtime</scope>
</dependency>
<!-- SLF4J implementation -->
<dependency>
  <groupId>ch.qos.logback</groupId>
  <artifactId>logback-classic</artifactId>
  <scope>runtime</scope>
</dependency>

Those 2 situations arent really the same, and comparing them seems rather apples to oranges.

In the first case, the people who have components using SLF4J as their logging API, but wish to use Log4J as their impl, add e.g log4j-slf4j2-impl, and simply get what they want. They already had an API, now they have a binding for it to the Log4j impl. Naturally that impl uses log4j-api, but the user often does necessarily need to know or care about it or the specific log4j deps. They add log4j-slf4j2-impl to their pom and get a Log4J impl of the same version. All very in line with expectation for adding something called log4j-slf4j2-impl that actually provides the Log4j core impl and [essentially] always has. Many will never depend on log4j-core in any way because their code never touches it since it/components-they-use used SLF4J, and since they already just got it when asking for a log4j impl via adding log4j-slf4j2-impl.

In the second case, people who have components using log4j-api but dont want to use log4j for their logging impl, but some other SLF4J impl, first add log4j-to-slf4j to bring in the binding for 'redirecting log4j api to SLF4J', which supplies no impl and doesnt suggest it in the name, and then they add their preferred SLF4J impl or maybe even have implemented it themselves. It does actually make sense that log4j-to-slf4j doesnt supply a specific impl because it actually is a binding purely to the API, not an impl, and there are multiple impls it could supply so its not obvious one particular one should be picked. It obviously could make sense if the module was clearly aimed something more specific, say something like a log4j-to-logback-classic, in which case theyd again be fairly likely to just have that single import.

These are the recommended methods on our installation page (see Installing Log4j Core and Installing Logback).

Your example for log4j-slf4j2-impl above is not how many users use log4j-slf4j2-impl. They want e.g Log4j 2.24.0 as impl, but use SLF4J as API, so they'll often simply depend on 'log4j-slf4j2-impl' 2.24.0 and get what they want, end of story. Many will not depend on log4j-core, as they didnt need to. From their perspect, they already depended on what they needed, and got what they wanted.

Your example is also not how the Log4j website indicates users can use log4j-slf4j2-impl (or log4j-slf4j-impl). It simply indicates making the same single-dependency import that many users do in their builds (including me, to be clear):
https://github.com/apache/logging-log4j2/blob/fda45ecf6858734136ff584544b76d558843a3a4/src/site/antora/modules/ROOT/partials/components/log4j-slf4j2-impl.adoc

Yes, you have recently modified the description there in the new site to say it translates to the API (versus the old site indicating "to use Log4j 2 as the implementation"), but as above most users already historically know and expect and use it as a translation to the impl, with the same single import the site suggests, because thats what it actually does and [almost] always did.

@vy
Copy link
Member

vy commented Sep 12, 2024

@gemmellr, first and foremost, I am very very happy to see that a user steps up to share his thoughts, in detail. I am very thankful for that, please keep it coming. And if you haven't noticed so far, please also be ready to be challenged. 😉 Sadly Fortunately Log4j is not financially backed by a company 😅, there are no secret agendas, there is no BDFL, but a handful of volunteers trying to deliver for the better good.

If I may, all your arguments against

  • removal of certain bridges in Log4j 3 and
  • their scoping (i.e., should SLF4J to Log4j API Bridge depend on Log4j Core or not)

can be summarized as these changes are against users' expectations and what they are used to.

Users' expectations

I completely get this sentiment and I am very well aware that introducing changes that break users' setups is the shortest path to killing a project. Though an expectation doesn't imply a correct behaviour. In the case of Log4j, this is even worse: its typical usage pattern is infested with several ill-advised practices, which are stuck to it mostly due to backward compatibility concerns. This is our curse and bliss. It is a curse, because features, even those that can perfectly be qualified as a bug, can only be added to Log4j, nothing can be removed. It is a bliss, because it creates (almost) zero maintenance burden for its users.

Major releases

AFAICT, major releases are chances to improve this curse-and-bliss balance. Maintainers are allowed to carry out major improvements at the cost of breaking certain setups. Almost all popular library major releases require minor changes from developers. The science is finding the sweet spot.

Let's think the other way around: If we are not allowed to break backward compatibility in a major release, what is the point of a major release? We can very well keep on piling up to Log4j 2 and learn to live with shame and bloat.

Log4j 3

Log4j 2 hasn't seen a major release since its incarnation, 2014. In a decade, Log4j 2 hasn't lost any weight, but gained, a lot! Any maintainer can show you tens of thousands of LoC they would love to see gone, and another equally sized batch to be implemented in a totally different way. Hence, Log4j 3 is an ideal milestone for some healthy diet.

I dont see a great benefit to removing most of these bridges

Log4j 3 was forked in... 2016! 😱 Now imagine the brutal magnitude of code divergence between 2 and 3 branches! The maintenance burden is immense! The code base size is huge! Any one-liner fix becomes a full day job to sync. between branches. It practically renders changes spreading across multiple modules impossible. By avoiding code duplication, we will greatly improve the efficiency of our limited development resources.

It also feels weirder to keep some but not others.

As @ppkarwasz stated in the ticket description, some are kept because Log4j 3 can offer improvements. For the rest that are left in Log4j 2, they are functionally identical.

I think a majority of this weirdness stems from incorrect modularization of the project. In an ideal world, Log4j API, Log4j Core, Log4j-to-SLF4J bridge, etc. should have all been different projects with their own release cycles. Then only releasing a new major version for Log4j Core and some bridges wouldn't feel weird at all. Though due to the current status quo... Allow me to remind that it is 2-3 months ago that, upon my proposal, PMC agreed to drop the Log4j 3 API and embrace the Log4j 2 API as "the Log4j API" and use that in Log4j 3 instead. Once Log4j 3 is out, we will move "the Log4j API" to its own repository with its own release cycle. Hence, things are improving, but we have a lot to do.

@vy vy added slf4j Affects SLF4J integration JUL Affects `java.util.logging` integration labels Sep 12, 2024
@gemmellr
Copy link
Member

My argument was more around the fact it actually is not just a "SLF4J to Log4j API Bridge" as you suggested in your summary, and it never really was, and it doesnt make sense to me to treat it as one now when it still wont be one. If it always had been one, there wouldnt be the same issue to generate the friction this will.

I do think e.g log4j-slf4j2-impl should supply a dep on the -core impl of the same version, just as it always has. I believe most users do too.

That said, it could also compatibly depend on an actual api-only bridge that was independent, should one exist with a different and perhaps more descriptive name. Though as previously I dont think this is what many users want.

You are right that in general, people having existing expectations doesnt necessarily mean they are correct. In this case I dont believe what many users do/expect with the bridges is incorrect though. They are using it as it was described and has always worked from the start, and still does, and what it does is perfectly valid and in line with its naming. I think you have rethought things recently in a way that suits you, but which doenst actually match what the module still does or what came before. That is of course entirely your right as the developers either way, and I'd actually be all for if it really delivered significant gains or corrected some notable issue users have, but I dont really see that it truly will in this case. Instead I see friction for many users, without the real offsetting gain. I also see alternatives without introducing that friction.

If you were going to move them all to their own release cycle and repo, fair enough, that would be a good reason, and as above could even still be done without the friction. Though your comment doesnt indicate that is the case here for the bridges, instead being just being noted for the API (which isnt likely to trouble many the same way, since those using it directly will still be doing so, and it doesnt depend on anything else thats being messed with). Neither does keeping some of the bridges in 3.x for now but not others. That just makes things even muddier from the user perspective, bridges that arent independent but some exist as 2.x only and others as both 2.x and 3.x.

One argument for removing them is apparently that they have not diverged, but you then also cite the brutal divergence between 2.x and 3.x and entire days spent on porting 1 line fixes. These arguments seem to largely cancel each other out to me as far as the bridges are concerned; if they dont diverge, and indeed after this cant going forward since the 2.x module still has to work with both versions, then there doesnt really seem to be a huge scope for issues in porting many changes between the versions either.

Overall this also seems like a good way to extend Log4j 2.x usage, both deliberately and unwittingly. E.g from people deliberately putting off making the tweaks needed to upgrade rather than it being more seemless. Or from users just not discovering the final 3.x is even out because the bridge modules they actually depend on never appear with the finished 3.x versions, stopping at the current betas. Or from dependency resolution behaviours meaning some downstream usages will actually still get the 2.x core even when an intermediate module they depended upon had used the 3.x core via the bom, because the 2.x bridge modules themselves will still depend on the 2.x core and dependency resolution will run with that instead in some transitive cases.

Anyways, I've burned enough time replying here I think. If you are still fine with the user friction then so be it. At least I am aware of the coming hassle myself now, thanks to randomly stumbling across this PR.

@ppkarwasz
Copy link
Contributor Author

@gemmellr,

My argument was more around the fact it actually is not just a "SLF4J to Log4j API Bridge" as you suggested in your summary, and it never really was, and it doesnt make sense to me to treat it as one now when it still wont be one. If it always had been one, there wouldnt be the same issue to generate the friction this will.

Technically it is a "SLF4J to Log4j API Bridge" and the code does not depend on Log4j Core. We agree however that it was branded as "SLF4J Binding for Log4j Core" for so long, that even part of the dev team thought it was true.

I do think e.g log4j-slf4j2-impl should supply a dep on the -core impl of the same version, just as it always has. I believe most users do too.

We are not breaking that expectation: log4j-slf4j2-impl version 2.24.0 does depend on log4j-core version 2.24.0. So:

  • people that just include log4j-slf4j2-impl:2.24.0 in their project will still use Log4j Core 2.
  • people that use log4j-bom:3.0.0 directly in their project will use Log4j Core 3, whether or not they explicitly add log4j-core.
  • people that use spring-boot-starter-log4j2 in a project with parent POM spring-boot-starter-parent will use Log4j Core 3 once they bump the log4j2.version variable to 3.0.0.
  • people that use spring-boot-starter-log4j2 in a project that does not use the spring-boot-starter-parent will use Log4j Core 2 until Spring bumps the version. We muss discuss this with the Spring Boot team.

Note that user version expectations in Log4j 3 must change anyway: the version of log4j-api and log4j-core will differ. I can add a note about this to our Migrating from Log4j 2 guide.

It does make sense to expand the migration guide to include not only the new dependencies that must be added, but also the old ones that can be removed. For example if a user wants Asynchronous Loggers, he must add log4j-async-logger, but should remove com.lmax:disruptor from the direct dependencies (obviously unless he uses it directly).

I'll add the changes to this PR.

That said, it could also compatibly depend on an actual api-only bridge that was independent, should one exist with a different and perhaps more descriptive name. Though as previously I dont think this is what many users want.

Great idea 💯! I can add an (empty) slf4j-to-log4j:3.0.0 artifact that depends on log4j-slf4j2-impl, but excludes the log4j-core transitive dependency. I'll add it to this PR.

I think users are confused by artifact names like X-over-Y, X-Y-impl, X-Y and Y-X to indicate a bridge between logging APIs. A coherent X-to-Y nomenclature would be useful.

Overall this also seems like a good way to extend Log4j 2.x usage, both deliberately and unwittingly. E.g from people deliberately putting off making the tweaks needed to upgrade rather than it being more seemless. Or from users just not discovering the final 3.x is even out because the bridge modules they actually depend on never appear with the finished 3.x versions, stopping at the current betas. Or from dependency resolution behaviours meaning some downstream usages will actually still get the 2.x core even when an intermediate module they depended upon had used the 3.x core via the bom, because the 2.x bridge modules themselves will still depend on the 2.x core and dependency resolution will run with that instead in some transitive cases.

Regarding the problems of transitive deps, that can not be avoided. In our own deps we have libraries depending on SLF4J 1 and SLF4J 2 and the result is random. There is no other solution than adding slf4j-api explicitly as runtime dependency and a matching logback-classic (or in the case of our tests slf4j-nop).

@gemmellr, could you take a look at #2935? Since we are breaking BC anyway, I think that log4j-jul should be split this way:

  • jul-to-log4j, which contains just o.a.l.l.j.LogManager (duly repackaged). That is something that only makes sense in the binary distribution of an application. Spring Boot and other runtimes do not use it. There is also no SLF4J equivalent, so it can be used by Logback users too.
  • log4j-jul, which contains the o.a.l.l.j.Log4jBridgeHandler. This is equivalent to slf4j-to-jul, so the package could have a hard dependency on Log4j Core and allow level propagation from Log4j Core to JUL by default.

@gemmellr
Copy link
Member

gemmellr commented Sep 16, 2024

@gemmellr,

My argument was more around the fact it actually is not just a "SLF4J to Log4j API Bridge" as you suggested in your summary, and it never really was, and it doesnt make sense to me to treat it as one now when it still wont be one. If it always had been one, there wouldnt be the same issue to generate the friction this will.

Technically it is a "SLF4J to Log4j API Bridge" and the code does not depend on Log4j Core. We agree however that it was branded as "SLF4J Binding for Log4j Core" for so long, that even part of the dev team thought it was true.

Because it is true. The module is non-technically a "SLF4J Binding for Log4j Core" as far as a user is concerned, and always was. It is correct to describe it that way, which is likely why it was described that way for more than a decade, as it matches what the module does.

The user goes from having only SLF4J API, to having actual logging coming out of Log4j, by adding that single module dependency. As far as any user is concerned that makes the module a "SLF4J Binding for Log4j Core". It definitely is not only an API bridge or there would be no actual logging occurring. That the core is there in a dependency module is largely an implementation detail to the user, e.g if you were to actually shade/bundle the core classes and the user would still get exactly the same behaviour they do now, which again shows that it is not just an API-only bridge module to the users.

I realise it achieves that by having code that bridges to Log4j API, and then providing Log4j core which 'just makes it actuallylog', but the point is that it does do that. That makes the module, not just the code in it, a "SLF4J Binding for Log4j Core" for the people using it.

It always has been, even for the log4j 1.x version homed at SLF4J before that.

I do think e.g log4j-slf4j2-impl should supply a dep on the -core impl of the same version, just as it always has. I believe most users do too.

We are not breaking that expectation: log4j-slf4j2-impl version 2.24.0 does depend on log4j-core version 2.24.0. So:

Yes, except log4j-slf4j2-impl 3.0.0 wont [exist at all], so you are in fact breaking the expectation going forward. Just not for the old bit. The user expectation is e.g 'I want Log4j 3.x so I'll depend on the matching bridge'; that is actually breaking.

  • people that just include log4j-slf4j2-impl:2.24.0 in their project will still use Log4j Core 2.

  • people that use log4j-bom:3.0.0 directly in their project will use Log4j Core 3, whether or not they explicitly add log4j-core.

This is the change. There wont be a log4j-slf4j2-impl:3.0.0 matching the log4j-core 3.0.0 which is what the expectation is. There are lots of users that dont use the bom, because they dont need to currently, many depend on 1 single module, e.g log4j-slf4j2-impl, and then get Log4j of that version. Many wont notice 3.0.0 exists at all as a result because they only depend on log4j-slf4j2-impl which would remain at 2.x.

  • people that use spring-boot-starter-log4j2 in a project with parent POM spring-boot-starter-parent will use Log4j Core 3 once they bump the log4j2.version variable to 3.0.0.

  • people that use spring-boot-starter-log4j2 in a project that does not use the spring-boot-starter-parent will use Log4j Core 2 until Spring bumps the version. We muss discuss this with the Spring Boot team.

Note that user version expectations in Log4j 3 must change anyway: the version of log4j-api and log4j-core will differ. I can add a note about this to our Migrating from Log4j 2 guide.

Though I imagine some might need to adjust for the API, it will be those that actually depend on it, and I dont think as many will be as bothered by that as they will about the bridges.

Also, people using the bridges largely do not use/depend on the API at all. Many of them depend solely on the bridge as they always did (and as the website still suggests today), get the core along with it, and will often never even consider the API dep/version in any fashion.

It does make sense to expand the migration guide to include not only the new dependencies that must be added, but also the old ones that can be removed. For example if a user wants Asynchronous Loggers, he must add log4j-async-logger, but should remove com.lmax:disruptor from the direct dependencies (obviously unless he uses it directly).

I'll add the changes to this PR.

That said, it could also compatibly depend on an actual api-only bridge that was independent, should one exist with a different and perhaps more descriptive name. Though as previously I dont think this is what many users want.

Great idea 💯! I can add an (empty) slf4j-to-log4j:3.0.0 artifact that depends on log4j-slf4j2-impl, but excludes the log4j-core transitive dependency. I'll add it to this PR.

That is partly the opposite of what I was meaning. I meant you could still have a pom-only log4j-slf4j2-impl 3.0.0 module for compatibility, that does depend on core and on an actual api-only bridge module that doesnt, e.g your proposed slf4j-to-log4j, should one ever actually exist. Though again, I dont think such an api-only bridge module is what users want.

I think users are confused by artifact names like X-over-Y, X-Y-impl, X-Y and Y-X to indicate a bridge between logging APIs. A coherent X-to-Y nomenclature would be useful.

Whilst a consistent nomenclature would be good, in this case I dont think users are confused at all. It seems to be the developers that are.

Users seem to fully expect log4j-slf4j2-impl to supply the core and begin handling actual logging for SLF4J2, in alignment with both the impl in its name and its actual description for >10 years as a binding to Core, and related behaviour of the similar bridge for Log4j 1.x long before that. As https://issues.apache.org/jira/browse/LOG4J2-3601 shows from last time you tried making the bridge module an api-only module.

Overall this also seems like a good way to extend Log4j 2.x usage, both deliberately and unwittingly. E.g from people deliberately putting off making the tweaks needed to upgrade rather than it being more seemless. Or from users just not discovering the final 3.x is even out because the bridge modules they actually depend on never appear with the finished 3.x versions, stopping at the current betas. Or from dependency resolution behaviours meaning some downstream usages will actually still get the 2.x core even when an intermediate module they depended upon had used the 3.x core via the bom, because the 2.x bridge modules themselves will still depend on the 2.x core and dependency resolution will run with that instead in some transitive cases.

Regarding the problems of transitive deps, that can not be avoided. In our own deps we have libraries depending on SLF4J 1 and SLF4J 2 and the result is random. There is no other solution than adding slf4j-api explicitly as runtime dependency and a matching logback-classic (or in the case of our tests slf4j-nop).

There are many subtleties to dependency resolution, and I'm not clear you are discussing the same one I was. Yes, if you depend on e.g 2 different components and each of those has different deps on e.g SLF4J1 and on SLF4J2, the end resolution has to pick 1 of them, clearly.

I am talking about a specific case though, where e.g a final build depending on 1 single intermediate component version, which in turn depended on 1 exact version of log4j-slf4j2-impl by using the bom and gets its specified log4j-core. Unless both bits use the bom (which again, lots dont, because they dont need to currently) then they will actually both get different versions of log4j-core with the proposed change for 3.x, whereas today they would both get the same version of log4j-core.

E.g consider an example where a pretend log4j-bom I made, sets the versions of log4j-slf4j2-impl to 2.23.1 (acting as a proxy for the ongoing 2.x releases) and log4j-core to 2.24.0 (newer than 2.23.1 to act as a proxy for future 3.x core releases). An intermediate-component actually uses the bom, and depends on log4j-slf4j2-impl only. It gets the dep versions you would expect, core 2.24.0 and log4j-slf4j2-impl 2.23.1 (again, a proxy for future 3.x cores with older 2.x bridges). An application then depends on that intermediate-component and naturally gets all the intermediate-component dependencies. But not at the same versions. Even though intermediate-component used log4j-core 2.24.0 specifically, the end usage instead gets log4j-core 2.23.1 because thats what log4j-slf4j2-impl 2.23.1 depends upon, and it doesnt matter that the log4j-bom was used by intermediate-component.

[INFO] --- dependency:3.7.1:tree (default-cli) @ intermediate-component ---
[INFO] org.apache.logging.log4j.example:intermediate-component:pom:1.0.0-SNAPSHOT
[INFO] \- org.apache.logging.log4j:log4j-slf4j2-impl:jar:2.23.1:runtime
[INFO]    +- org.apache.logging.log4j:log4j-api:jar:2.23.1:runtime
[INFO]    +- org.slf4j:slf4j-api:jar:2.0.9:runtime
[INFO]    \- org.apache.logging.log4j:log4j-core:jar:2.24.0:runtime
[INFO] 
[INFO] --------< org.apache.logging.log4j.example:example-application >--------
[INFO] Building example-application 1.0.0-SNAPSHOT                        [3/4]
[INFO]   from example-application/pom.xml
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- dependency:3.7.1:tree (default-cli) @ example-application ---
[INFO] org.apache.logging.log4j.example:example-application:pom:1.0.0-SNAPSHOT
[INFO] \- org.apache.logging.log4j.example:intermediate-component:jar:1.0.0-SNAPSHOT:compile
[INFO]    \- org.apache.logging.log4j:log4j-slf4j2-impl:jar:2.23.1:runtime
[INFO]       +- org.apache.logging.log4j:log4j-api:jar:2.23.1:runtime
[INFO]       +- org.slf4j:slf4j-api:jar:2.0.9:runtime
[INFO]       \- org.apache.logging.log4j:log4j-core:jar:2.23.1:runtime

This specific case would be avoided by not using log4j-slf4j2-impl in the proposed fashion within the log4j 2.x and 3.x builds. If there was a log4j-slf4j2-impl 3.x module that actually depended on the 3.x core then this wouldnt happen, the same way it doesnt happen today.

@ppkarwasz ppkarwasz marked this pull request as draft September 17, 2024 07:00
@ppkarwasz
Copy link
Contributor Author

I am talking about a specific case though, where e.g a final build depending on 1 single intermediate component version, which in turn depended on 1 exact version of log4j-slf4j2-impl by using the bom and gets its specified log4j-core. Unless both bits use the bom (which again, lots dont, because they dont need to currently) then they will actually both get different versions of log4j-core with the proposed change for 3.x, whereas today they would both get the same version of log4j-core.

Sure, I understand that versions set through a BOM do not propagate to dependents, but log4j-slf4j2-impl is an implementation of SLF4J and logging API implementations should not appear as transitive dependencies:

Note: I am putting this PR in draft mode for now. We will discuss it at the next PMC meeting.

@ppkarwasz ppkarwasz added this to the 3.x milestone Sep 17, 2024
@gemmellr
Copy link
Member

Sure, I understand that versions set through a BOM do not propagate to dependents, but log4j-slf4j2-impl is an implementation of SLF4J and logging API implementations should not appear as transitive dependencies:

Agreed in a lot of cases. However it does still happen, and in various cases it was/is intended to.

* If `intermediate-component` is a third-party library, it certainly should not mess with your logging. There are known examples of libraries that leak logging implementations to the runtime scope, but these are usually considered bugs (e.g. [ZOOKEEPER-4820](https://issues.apache.org/jira/browse/ZOOKEEPER-4820), [[Bug] Maven artifacts should not depend on logging backend eventmesh#4700](https://github.com/apache/eventmesh/issues/4700), [rocketmq-tools should not depend on logback-classic rocketmq#5347](https://github.com/apache/rocketmq/issues/5347)).

The RocketMQ one discusses how the module in question is an executable tool, in which case it seems reasonable enough that it has such a dependency, possibly why it hasnt been actioned in 2 years. The main issue seems to stem from the use of the same module for some very different use cases, meaning that there are different perspectives on whether/when it should be there. That similarly seems the case in the Zookeeper one, with a shared server and client module.

In both cases, there appear to be situations / viewpoints where it is a bug, and other situations / viewpoints where it is not.

Regardless which case/viewpoint a specific user has, both situations do show that it can and does happen, intentionally or unwittingly, that logging dependencies get passed on to consuming components.

* If `intermediate-component` is a library whose purpose is to configure logging (e.g., `spring-boot-starter-log4j2`), I would expect that each application has only one such dependency.

Note: I am putting this PR in draft mode for now. We will discuss it at the next PMC meeting.

👍

@ppkarwasz
Copy link
Contributor Author

At the PMC meeting we came to the conclusion that user friction is inevitable. To minimize the discomfort we will:

@ppkarwasz ppkarwasz marked this pull request as ready for review September 30, 2024 15:28
@gemmellr
Copy link
Member

User friction is inevitable? Despite the obvious ways to avoid it? Hmm..

So you are actually now going yet another way, adding more friction sooner, and just actually breaking most existing Log4J users using the SLF4J bridges immediately during 2.x upgrade? How on earth is that minimising discomfort, by just breaking even the 2.x users now?

This frankly boggles the mind. Lots of users, rather than merely some, will be broken by #3038 (e.g see last time you did that, on a new module, and got immediate complaints) . Even your own websites current instructions of how to use those modules only direct to add that single dependency. Most uses following those instructions will be broken, even now including those that are using the bom, rather than just those many who aren't.

So you are going to take an existing 'bridge to the log4j impl' behaviour that pretty much everyone wants and expects to work the way it does already, and in one form or other has done so for over 15 years, and re-purpose it to do something few if anyone wants, in order to not release something that is said to be unchanged anyway and thus should take little effort at all to release. Even if still doing that overall rather than e.g. making it actually independent, there are still ways to do so that dont need to break most existing usages of said components, but you aren't doing that either; instead lets just break everyone now. I find this all completely nonsensical.

@ppkarwasz
Copy link
Contributor Author

User friction is inevitable? Despite the obvious ways to avoid it? Hmm..

You mean releasing an artifact that only differs in the pom.xml and call it 3.0.0. This doesn't go in the right direction.

Even your own websites current instructions of how to use those modules only direct to add that single dependency.

We recently rewrote most of the website. Could you point us to the page that suggest to only add log4j-slf4j2-impl?

@gemmellr
Copy link
Member

gemmellr commented Oct 1, 2024

User friction is inevitable? Despite the obvious ways to avoid it? Hmm..

You mean releasing an artifact that only differs in the pom.xml and call it 3.0.0. This doesn't go in the right direction.

That is certainly one option. Probably still the most obvious to me given you are actually keeping other bridges in 3.x and with apparently no plans to make any of them actually-independent, which would at least be a compelling reason to kick off down this path. (Though I'd expect it to differ in JDK support just like the Core does, so users didnt have mismatched requirements, which is understandable when independent but not so much when not).

There are other options though that still went more in the direction you want, or potentially further, yet without immediately breaking most users, as we discussed on this PR already.

Even your own websites current instructions of how to use those modules only direct to add that single dependency.

We recently rewrote most of the website. Could you point us to the page that suggest to only add log4j-slf4j2-impl?

https://logging.apache.org/log4j/2.x/manual/installation.html#impl-core-bridge-slf4j is what I am thinking of.

We assume you use [log4j-bom](https://logging.apache.org/log4j/2.x/components.html#log4j-bom) for dependency management.

<dependency>
  <groupId>org.apache.logging.log4j</groupId>
  <artifactId>log4j-slf4j2-impl</artifactId>
  <scope>runtime</scope>
</dependency>

Though because I couldnt understand your confusion over it I looked closer this time, on perhaps the 5th time of looking at the page in the past month during this PR discussion, and see now that 'installing bridges' is actually a sub-section of 'installing core'. I guess thats the intent to say you need to do both, but I have missed that nuance until looking at it more closely today and even now I see it I expect many will do the same. I went to the docs, saw the menu listing SLF4J bridge, clicked on it. It presents the single dep 'do this'. I do that, and it works. I expect lots of people will do that. Particularly since...

I guess that I (and again, many many users..) am also entrenched in the long standing expectation and knowledge that it brings in Core/equivalent impl based on the 15+ years of these bridges always doing so, and their name suggesting it, and the recently-replaced website always saying it did for ages (along with similar single-dependency 'do this' instructions), and even the module description itself until recently indicating that its a bridge to Core; because it always was and because it still is. At least until the mess-in-waiting that is #3038.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JUL Affects `java.util.logging` integration slf4j Affects SLF4J integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants