You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by tison <wa...@gmail.com> on 2022/06/21 10:44:12 UTC
Pulsar logging framework design
Hi devs,
I learn that PulsarAdminImpl has a static block requires classpath contains
exact either of
* slf4j-jdk14
* jul-to-slf4j
I'm curious what logging framework Pulsar choose among the codebase.
Basically we should
depend on either jul facade or slf4j facade, and leave the class loading
issue to users who
include libraries depending on other logging framework.
Current logic causes an issue that if user doesn't depend on both
slf4j-jdk14 and jul-to-slf4j,
PulsarAdminImpl will panic because it requires exact one of these two deps
in the classpath.
And thus user should add one of them (basically jul-to-slf4j) even if they
don't depend on it
effectively.
Best,
tison.
Re: Pulsar logging framework design
Posted by tison <wa...@gmail.com>.
I created a pull request for removing jul-to-slf4j[1] and it seems all
existing tests aren't affected.
The Pulsar project uses slf4j as the logging facade consistently. If a user
want to add a dependency using a different logging framework, they should
take care of the packaging strategy themselves.
pulsar-sql has a dependency to slf4j-jdk14 which redirect slf4j to jul.
Since this is a unidirectional redirection, it won't cause runtime error,
but pulsar-sql will logging with jul framework, which is the same as with
previous workaround.
cc mmerli@apache.org as the original author of this stuff.
Best,
tison.
[1] https://github.com/apache/pulsar/pull/16320
tison <wa...@gmail.com> 于2022年6月21日周二 23:01写道：
> cc users@,
>
> Here is a discussion to remove jul-to-slf4j from pulsar's dependency tree.
>
> I'd like to do an investigation that whether any user depends on this
> transitive dependency for:
>
> 1. Implicitly add jul-to-slf4j to classpath from a pulsar-xxx fatjar.
> 2. Implicitly calling:
> SLF4JBridgeHandler.removeHandlersForRootLogger();
> SLF4JBridgeHandler.install();
>
> .. or any other use case.
>
> Best,
> tison.
>
>
> tison <wa...@gmail.com> 于2022年6月21日周二 19:29写道：
>
>> Thanks for your input @Enrico! I'll do some investigation in these two
>> weeks.
>> Does this estimate meet the schedule of 2.11?
>>
>> BTW, there is another dependency change PR[1] waiting for review. I don't
>> want
>> to mix this thread but throw it here under the same topic for more
>> visibility.
>>
>> Best,
>> tison.
>>
>> [1] https://github.com/apache/pulsar/pull/16109
>>
>>
>> Enrico Olivelli <eo...@gmail.com> 于2022年6月21日周二 19:15写道：
>>
>>> Tison,
>>>
>>> Il giorno mar 21 giu 2022 alle ore 12:44 tison <wa...@gmail.com>
>>> ha scritto:
>>> >
>>> > Hi devs,
>>> >
>>> > I learn that PulsarAdminImpl has a static block requires classpath
>>> contains
>>> > exact either of
>>> >
>>> > * slf4j-jdk14
>>> > * jul-to-slf4j
>>> >
>>> > I'm curious what logging framework Pulsar choose among the codebase.
>>>
>>> AKAIF we are using only SLF4J in Pulsar codebase.
>>> maybe jul-to-slf4j is there only for historical reasons
>>>
>>> ideally jul-to-slf4j is only something you should care about only
>>> during packaging, for runtime (or you can add it also for with test
>>> scope)
>>>
>>> we could try to remove it for the next major release (2.11)
>>>
>>> Would you like to do some trials ?
>>>
>>> Enrico
>>>
>>>
>>> > Basically we should
>>> > depend on either jul facade or slf4j facade, and leave the class
>>> loading
>>> > issue to users who
>>> > include libraries depending on other logging framework.
>>> >
>>> > Current logic causes an issue that if user doesn't depend on both
>>> > slf4j-jdk14 and jul-to-slf4j,
>>> > PulsarAdminImpl will panic because it requires exact one of these two
>>> deps
>>> > in the classpath.
>>> > And thus user should add one of them (basically jul-to-slf4j) even if
>>> they
>>> > don't depend on it
>>> > effectively.
>>> >
>>> > Best,
>>> > tison.
>>>
>>
Re: Pulsar logging framework design
Posted by tison <wa...@gmail.com>.
I created a pull request for removing jul-to-slf4j[1] and it seems all
existing tests aren't affected.
The Pulsar project uses slf4j as the logging facade consistently. If a user
want to add a dependency using a different logging framework, they should
take care of the packaging strategy themselves.
pulsar-sql has a dependency to slf4j-jdk14 which redirect slf4j to jul.
Since this is a unidirectional redirection, it won't cause runtime error,
but pulsar-sql will logging with jul framework, which is the same as with
previous workaround.
cc mmerli@apache.org as the original author of this stuff.
Best,
tison.
[1] https://github.com/apache/pulsar/pull/16320
tison <wa...@gmail.com> 于2022年6月21日周二 23:01写道：
> cc users@,
>
> Here is a discussion to remove jul-to-slf4j from pulsar's dependency tree.
>
> I'd like to do an investigation that whether any user depends on this
> transitive dependency for:
>
> 1. Implicitly add jul-to-slf4j to classpath from a pulsar-xxx fatjar.
> 2. Implicitly calling:
> SLF4JBridgeHandler.removeHandlersForRootLogger();
> SLF4JBridgeHandler.install();
>
> .. or any other use case.
>
> Best,
> tison.
>
>
> tison <wa...@gmail.com> 于2022年6月21日周二 19:29写道：
>
>> Thanks for your input @Enrico! I'll do some investigation in these two
>> weeks.
>> Does this estimate meet the schedule of 2.11?
>>
>> BTW, there is another dependency change PR[1] waiting for review. I don't
>> want
>> to mix this thread but throw it here under the same topic for more
>> visibility.
>>
>> Best,
>> tison.
>>
>> [1] https://github.com/apache/pulsar/pull/16109
>>
>>
>> Enrico Olivelli <eo...@gmail.com> 于2022年6月21日周二 19:15写道：
>>
>>> Tison,
>>>
>>> Il giorno mar 21 giu 2022 alle ore 12:44 tison <wa...@gmail.com>
>>> ha scritto:
>>> >
>>> > Hi devs,
>>> >
>>> > I learn that PulsarAdminImpl has a static block requires classpath
>>> contains
>>> > exact either of
>>> >
>>> > * slf4j-jdk14
>>> > * jul-to-slf4j
>>> >
>>> > I'm curious what logging framework Pulsar choose among the codebase.
>>>
>>> AKAIF we are using only SLF4J in Pulsar codebase.
>>> maybe jul-to-slf4j is there only for historical reasons
>>>
>>> ideally jul-to-slf4j is only something you should care about only
>>> during packaging, for runtime (or you can add it also for with test
>>> scope)
>>>
>>> we could try to remove it for the next major release (2.11)
>>>
>>> Would you like to do some trials ?
>>>
>>> Enrico
>>>
>>>
>>> > Basically we should
>>> > depend on either jul facade or slf4j facade, and leave the class
>>> loading
>>> > issue to users who
>>> > include libraries depending on other logging framework.
>>> >
>>> > Current logic causes an issue that if user doesn't depend on both
>>> > slf4j-jdk14 and jul-to-slf4j,
>>> > PulsarAdminImpl will panic because it requires exact one of these two
>>> deps
>>> > in the classpath.
>>> > And thus user should add one of them (basically jul-to-slf4j) even if
>>> they
>>> > don't depend on it
>>> > effectively.
>>> >
>>> > Best,
>>> > tison.
>>>
>>
Re: Pulsar logging framework design
Posted by tison <wa...@gmail.com>.
cc users@,
Here is a discussion to remove jul-to-slf4j from pulsar's dependency tree.
I'd like to do an investigation that whether any user depends on this
transitive dependency for:
1. Implicitly add jul-to-slf4j to classpath from a pulsar-xxx fatjar.
2. Implicitly calling:
SLF4JBridgeHandler.removeHandlersForRootLogger();
SLF4JBridgeHandler.install();
.. or any other use case.
Best,
tison.
tison <wa...@gmail.com> 于2022年6月21日周二 19:29写道：
> Thanks for your input @Enrico! I'll do some investigation in these two
> weeks.
> Does this estimate meet the schedule of 2.11?
>
> BTW, there is another dependency change PR[1] waiting for review. I don't
> want
> to mix this thread but throw it here under the same topic for more
> visibility.
>
> Best,
> tison.
>
> [1] https://github.com/apache/pulsar/pull/16109
>
>
> Enrico Olivelli <eo...@gmail.com> 于2022年6月21日周二 19:15写道：
>
>> Tison,
>>
>> Il giorno mar 21 giu 2022 alle ore 12:44 tison <wa...@gmail.com>
>> ha scritto:
>> >
>> > Hi devs,
>> >
>> > I learn that PulsarAdminImpl has a static block requires classpath
>> contains
>> > exact either of
>> >
>> > * slf4j-jdk14
>> > * jul-to-slf4j
>> >
>> > I'm curious what logging framework Pulsar choose among the codebase.
>>
>> AKAIF we are using only SLF4J in Pulsar codebase.
>> maybe jul-to-slf4j is there only for historical reasons
>>
>> ideally jul-to-slf4j is only something you should care about only
>> during packaging, for runtime (or you can add it also for with test
>> scope)
>>
>> we could try to remove it for the next major release (2.11)
>>
>> Would you like to do some trials ?
>>
>> Enrico
>>
>>
>> > Basically we should
>> > depend on either jul facade or slf4j facade, and leave the class loading
>> > issue to users who
>> > include libraries depending on other logging framework.
>> >
>> > Current logic causes an issue that if user doesn't depend on both
>> > slf4j-jdk14 and jul-to-slf4j,
>> > PulsarAdminImpl will panic because it requires exact one of these two
>> deps
>> > in the classpath.
>> > And thus user should add one of them (basically jul-to-slf4j) even if
>> they
>> > don't depend on it
>> > effectively.
>> >
>> > Best,
>> > tison.
>>
>
Re: Pulsar logging framework design
Posted by tison <wa...@gmail.com>.
cc users@,
Here is a discussion to remove jul-to-slf4j from pulsar's dependency tree.
I'd like to do an investigation that whether any user depends on this
transitive dependency for:
1. Implicitly add jul-to-slf4j to classpath from a pulsar-xxx fatjar.
2. Implicitly calling:
SLF4JBridgeHandler.removeHandlersForRootLogger();
SLF4JBridgeHandler.install();
.. or any other use case.
Best,
tison.
tison <wa...@gmail.com> 于2022年6月21日周二 19:29写道：
> Thanks for your input @Enrico! I'll do some investigation in these two
> weeks.
> Does this estimate meet the schedule of 2.11?
>
> BTW, there is another dependency change PR[1] waiting for review. I don't
> want
> to mix this thread but throw it here under the same topic for more
> visibility.
>
> Best,
> tison.
>
> [1] https://github.com/apache/pulsar/pull/16109
>
>
> Enrico Olivelli <eo...@gmail.com> 于2022年6月21日周二 19:15写道：
>
>> Tison,
>>
>> Il giorno mar 21 giu 2022 alle ore 12:44 tison <wa...@gmail.com>
>> ha scritto:
>> >
>> > Hi devs,
>> >
>> > I learn that PulsarAdminImpl has a static block requires classpath
>> contains
>> > exact either of
>> >
>> > * slf4j-jdk14
>> > * jul-to-slf4j
>> >
>> > I'm curious what logging framework Pulsar choose among the codebase.
>>
>> AKAIF we are using only SLF4J in Pulsar codebase.
>> maybe jul-to-slf4j is there only for historical reasons
>>
>> ideally jul-to-slf4j is only something you should care about only
>> during packaging, for runtime (or you can add it also for with test
>> scope)
>>
>> we could try to remove it for the next major release (2.11)
>>
>> Would you like to do some trials ?
>>
>> Enrico
>>
>>
>> > Basically we should
>> > depend on either jul facade or slf4j facade, and leave the class loading
>> > issue to users who
>> > include libraries depending on other logging framework.
>> >
>> > Current logic causes an issue that if user doesn't depend on both
>> > slf4j-jdk14 and jul-to-slf4j,
>> > PulsarAdminImpl will panic because it requires exact one of these two
>> deps
>> > in the classpath.
>> > And thus user should add one of them (basically jul-to-slf4j) even if
>> they
>> > don't depend on it
>> > effectively.
>> >
>> > Best,
>> > tison.
>>
>
Re: Pulsar logging framework design
Posted by tison <wa...@gmail.com>.
Thanks for your input @Enrico! I'll do some investigation in these two
weeks.
Does this estimate meet the schedule of 2.11?
BTW, there is another dependency change PR[1] waiting for review. I don't
want
to mix this thread but throw it here under the same topic for more
visibility.
Best,
tison.
[1] https://github.com/apache/pulsar/pull/16109
Enrico Olivelli <eo...@gmail.com> 于2022年6月21日周二 19:15写道：
> Tison,
>
> Il giorno mar 21 giu 2022 alle ore 12:44 tison <wa...@gmail.com>
> ha scritto:
> >
> > Hi devs,
> >
> > I learn that PulsarAdminImpl has a static block requires classpath
> contains
> > exact either of
> >
> > * slf4j-jdk14
> > * jul-to-slf4j
> >
> > I'm curious what logging framework Pulsar choose among the codebase.
>
> AKAIF we are using only SLF4J in Pulsar codebase.
> maybe jul-to-slf4j is there only for historical reasons
>
> ideally jul-to-slf4j is only something you should care about only
> during packaging, for runtime (or you can add it also for with test
> scope)
>
> we could try to remove it for the next major release (2.11)
>
> Would you like to do some trials ?
>
> Enrico
>
>
> > Basically we should
> > depend on either jul facade or slf4j facade, and leave the class loading
> > issue to users who
> > include libraries depending on other logging framework.
> >
> > Current logic causes an issue that if user doesn't depend on both
> > slf4j-jdk14 and jul-to-slf4j,
> > PulsarAdminImpl will panic because it requires exact one of these two
> deps
> > in the classpath.
> > And thus user should add one of them (basically jul-to-slf4j) even if
> they
> > don't depend on it
> > effectively.
> >
> > Best,
> > tison.
>
Re: Pulsar logging framework design
Posted by Enrico Olivelli <eo...@gmail.com>.
Tison,
Il giorno mar 21 giu 2022 alle ore 12:44 tison <wa...@gmail.com>
ha scritto:
>
> Hi devs,
>
> I learn that PulsarAdminImpl has a static block requires classpath contains
> exact either of
>
> * slf4j-jdk14
> * jul-to-slf4j
>
> I'm curious what logging framework Pulsar choose among the codebase.
AKAIF we are using only SLF4J in Pulsar codebase.
maybe jul-to-slf4j is there only for historical reasons
ideally jul-to-slf4j is only something you should care about only
during packaging, for runtime (or you can add it also for with test
scope)
we could try to remove it for the next major release (2.11)
Would you like to do some trials ?
Enrico
> Basically we should
> depend on either jul facade or slf4j facade, and leave the class loading
> issue to users who
> include libraries depending on other logging framework.
>
> Current logic causes an issue that if user doesn't depend on both
> slf4j-jdk14 and jul-to-slf4j,
> PulsarAdminImpl will panic because it requires exact one of these two deps
> in the classpath.
> And thus user should add one of them (basically jul-to-slf4j) even if they
> don't depend on it
> effectively.
>
> Best,
> tison.