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