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
> > > > > >
> > >
>

Reply via email to