I submitted a PR to fix this issue: https://github.com/apache/pulsar/pull/22607
Yunze Xu <x...@apache.org> 于2024年4月27日周六 14:27写道: > > 1.7 has not been maintained for almost 2 years. > > Logging is only a very simple part. SLF4J is even just a logging > facade that has less chance to suffer CVEs like log4j2. Unless there > is any CVE found for slf4j, breaking the user case is worse than > upgrading the slf4j. > > > when the slf4j version is inconsistent, we still need to use `<exclude>`. > > Yes, that's why we exclude some slf4j dependencies from 3rd party > dependencies like confluent-schema-registry in KSN. However, back to > the discussion, the slf4j upgrade breaks many downstream projects. At > least, nearly all StreamNative's plugins are affected by this breaking > change. For example, KSN chooses to exclude all slf4j dependencies > from other dependencies and depend on the slf4j dependency from > `pulsar-*`. So a refactoring of pom.xml is needed for the latest > Pulsar. > > Thanks, > Yunze > > On Sat, Apr 27, 2024 at 12:47 PM Zixuan Liu <node...@gmail.com> wrote: > > > > > I don't think so. As a logging facade, there is no convincing reason > > > to keep it latest unless some CVEs. > > > > I think it's better to keep up with the latest version, as we always get > > support from the community. 1.7 has not been maintained for almost 2 > years. > > > > >The point is not related to how I should resolve the dependency issue. > > > It is how you care for the downstream users. > > > > We should care for the downstream users. > > > > I think the root cause is the pulsar-** includes the slf4j-api > dependency, > > so the users must use the same version of slf4j-xxx(simple, reload...). > > > > If we switch the slf4j API scope to the `provider` from `compile` in the > > pom.xml, and then the users can use any version of slf4j. > > > > Unifying SLF4J is quite difficult, many libraries include the slf4j 1.x > or > > 2.x, and when the slf4j version is inconsistent, we still need to use > > `<exclude>`. > > > > Thanks, > > Zixuan > > > > Yunze Xu <x...@apache.org> 于2024年4月27日周六 11:46写道: > > > > > The point is not related to how I should resolve the dependency issue. > > > It is how you care for the downstream users. > > > > > > Assuming there is an application that depends on pulsar-client 3.0.0 > > > running well. Then after some days, maintainers tried upgrading the > > > pulsar-client to 3.3.0 and they found logs don't work. You tell them > > > to upgrade the slf4j bindings as well. It might generally be okay. > > > However, if they have developed their own slf4j binding for 1.7.x. > > > Now, they also have to change the code of their own slf4j binding. But > > > it could be a challenge because its own slf4j binding might also be > > > included on their other projects. > > > > > > Thanks, > > > Yunze > > > > > > On Sat, Apr 27, 2024 at 11:39 AM Yunze Xu <x...@apache.org> wrote: > > > > > > > > > we will encounter them sooner or later. > > > > > > > > I don't think so. As a logging facade, there is no convincing reason > > > > to keep it latest unless some CVEs. > > > > > > > > > so I used the `<exclusion>` to exclude the org.slf4j or > > > org.apache.logging.log4j, and so on. > > > > > > > > As I've mentioned before, things become complicated once your project > > > > is large. For example, the KSN project relies on the SLF4J from > Pulsar > > > > implicitly because it expects the Pulsar dependency carries the SLF4J > > > > dependency. > > > > > > > > ``` > > > > <artifactId>pulsar-broker</artifactId> > > > > <artifactId>pulsar-broker-auth-sasl</artifactId> > > > > <artifactId>pulsar-client-original</artifactId> > > > > <artifactId>pulsar-client-auth-sasl</artifactId> > > > > <artifactId>pulsar-common</artifactId> > > > > <artifactId>pulsar-client-admin-original</artifactId> > > > > ``` > > > > > > > > I have to exclude the slf4j dependency for each `pulsar-*` > dependency. > > > > Then import both the `slf4j-api` and `slf4j-log4j12` dependencies in > > > > the root pom.xml. > > > > > > > > ``` > > > > <dependencies> > > > > <dependency> > > > > <groupId>org.slf4j</groupId> > > > > <artifactId>slf4j-api</artifactId> > > > > <version>${slf4j.version}</version> > > > > </dependency> > > > > > > > > <dependency> > > > > <groupId>org.slf4j</groupId> > > > > <artifactId>slf4j-log4j12</artifactId> > > > > <version>${slf4j.version}</version> > > > > <scope>runtime</scope> > > > > </dependency> > > > > </dependencies> > > > > ``` > > > > > > > > And then it works, but it also shows how much changes are needed > > > > caused by such a PR in the upstream. Existing users might already > > > > depend on the carried slf4j dependency from Pulsar. The point is the > > > > upgrade could BREAK many existing downstream projects. > > > > > > > > Thanks, > > > > Yunze > > > > > > > > On Sat, Apr 27, 2024 at 1:21 AM Zixuan Liu <node...@gmail.com> > wrote: > > > > > > > > > > After upgrading to 2.0.13, some issues were exposed, and we will > > > encounter > > > > > them sooner or later. > > > > > > > > > > When upgrading the slf4j in the pulsar, I also encountered the > problem > > > you > > > > > mentioned, so I used the `<exclusion>` to exclude the org.slf4j > > > > > or org.apache.logging.log4j, and so on. > > > > > > > > > > Do you think this approach is fair? > > > > > > > > > > Thanks, > > > > > Zixuan > > > > > > > > > > Yunze Xu <x...@apache.org> 于2024年4月26日周五 20:41写道: > > > > > > > > > > > Hi all, > > > > > > > > > > > > Recently I upgraded my downstream project's dependency to the > latest > > > > > > master branch and found no logs are printed now. > > > > > > > > > > > > ``` > > > > > > SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder". > > > > > > SLF4J: Defaulting to no-operation (NOP) logger implementation > > > > > > ``` > > > > > > > > > > > > Finally I found it's caused by #22391 [1], which upgrades the > > > > > > slf4j-api version from 1.7.32 to 2.0.13. It's really a harmful > change > > > > > > to downstream projects. For example, assuming there is a project > that > > > > > > depends on the pulsar-client-all, it might adds a SLF4J binding > > > > > > dependency like: > > > > > > > > > > > > ```xml > > > > > > > > > > > > <dependency> > > > > > > <groupId>org.apache.pulsar</groupId> > > > > > > <artifactId>pulsar-client-all</artifactId> > > > > > > <version>3.2.2</version> > > > > > > </dependency> > > > > > > > > > > > > <dependency> > > > > > > <groupId>org.slf4j</groupId> > > > > > > <artifactId>slf4j-simple</artifactId> > > > > > > <version>1.7.36</version> > > > > > > </dependency> > > > > > > ``` > > > > > > > > > > > > Yeah it works well. However, after upgrading the > pulsar-client-all to > > > > > > the latest master, users have to upgrade the slf4j-simple to > 2.0.13, > > > > > > otherwise the logging won't work. > > > > > > > > > > > > Things become worse when the downstream project is large. I tried > > > > > > adding the slf4j-simple or slf4j-reload4j 2.0.13 dependency to > the > > > > > > protocol handler project that depends on pulsar-broker and none > of > > > > > > them works. It has already wasted me much time debugging this > logging > > > > > > issue. I'm really interested in what significant change that > slf4j > > > 2.x > > > > > > brings, except a higher version number that might make some > > > developers > > > > > > think it's better? > > > > > > > > > > > > Therefore, I suggest reverting #22391 unless there is a > significant > > > > > > advantage of slf4j 2.0.13. > > > > > > > > > > > > [1] https://github.com/apache/pulsar/pull/22391 > > > > > > > > > > > > Thanks, > > > > > > Yunze > > > > > > > > > >