You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by "Piotr P. Karwasz" <pi...@gmail.com> on 2022/03/10 06:36:30 UTC
Order of property sources
Hello,
Among the problems I found in the ordering of property sources[1],
there is the question of the meaning of the numeric priority value:
* the manual says that lower numeric values override higher values[2],
* the Javadoc suggests an inverted order[3].
Which one should be the correct one? At first I implemented the
documentation order[4], since users might have learned that one
(although this order never worked, the first JIRA issue reporting it
is from last December). However this leaves a bitter aftertaste,
because the provider's priority uses the "highest value wins"
strategy.
I rebased my PR [4] (taking LOG4J2-3413 into account in the tests) and
I am planning to merge it in the weekend.
Piotr
[1] https://issues.apache.org/jira/browse/LOG4J2-3366
[2] https://logging.apache.org/log4j/2.x/manual/configuration.html#SystemProperties
[3] https://logging.apache.org/log4j/2.x/log4j-api/apidocs/org/apache/logging/log4j/util/PropertySource.html#getPriority--
[4] https://github.com/apache/logging-log4j2/pull/742
Re: Order of property sources
Posted by Ralph Goers <ra...@dslextreme.com>.
That is actually an interesting question. If you look at the logic, all the providers are loaded. They are then looped through and the first one that returns a LoggerFactory is used. I really consider this code unfinished. When I wrote it I considered supporting multiple logging implementations but I couldn’t really figure out how that would work so I just stopped after loading the first one. But in theory we could have multiple logger factories and there could be an implementation that calls each of the logging implementations.
But I have never thought of a rational reason for doing it so I’ve just left it the way it is.
Ralph
> On Mar 15, 2022, at 2:31 PM, Tim Perry <ti...@gmail.com> wrote:
>
> In that case, should log4j log a warning if more than one provider is included?
>
> Tim
>
>
>> On Mar 15, 2022, at 1:44 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>
>> I would not be in favor of this change. Providers have been designed since day one that highest wins and it is documented - https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory <https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory>.
>>
>> The fact that the user was surprised may be true, but the ordering was intentional. If you include log4j-to-slf4j it means you want to log to SLF4J. In that case log4j-core shouldn’t even be present.
>>
>> Ralph
>>
>>> On Mar 14, 2022, at 4:37 AM, Piotr P. Karwasz <pi...@gmail.com> wrote:
>>>
>>>> On Mon, Mar 14, 2022 at 10:54 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
>>>> Regarding your remark about providers... `Provider#getPriority()` has the
>>>> following javadoc: *"Gets the priority (natural ordering) of this Provider"*,
>>>> hence I would have expected it to work the same lowest-comes-first way, but
>>>> apparently not – relying on your assessment here. I am also inclined to
>>>> align them with the lowest-comes-first strategy, though this might have a
>>>> bigger impact if there are 3rd party providers out there in the wild. Maybe
>>>> others can weigh in here?
>>>
>>> I can provide some additional evidence supporting the necessity to
>>> invert the order. At least one user on Stack Overflow was surprised
>>> that after adding `log4j-core` to his project, the loggers are still
>>> of type `org.apache.logging.slf4j.SLF4JLogger`:
>>>
>>> https://stackoverflow.com/q/70487959/11748454
>>>
>>> Piotr
>>
Re: Order of property sources
Posted by Tim Perry <ti...@gmail.com>.
In that case, should log4j log a warning if more than one provider is included?
Tim
> On Mar 15, 2022, at 1:44 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>
> I would not be in favor of this change. Providers have been designed since day one that highest wins and it is documented - https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory <https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory>.
>
> The fact that the user was surprised may be true, but the ordering was intentional. If you include log4j-to-slf4j it means you want to log to SLF4J. In that case log4j-core shouldn’t even be present.
>
> Ralph
>
>> On Mar 14, 2022, at 4:37 AM, Piotr P. Karwasz <pi...@gmail.com> wrote:
>>
>>> On Mon, Mar 14, 2022 at 10:54 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
>>> Regarding your remark about providers... `Provider#getPriority()` has the
>>> following javadoc: *"Gets the priority (natural ordering) of this Provider"*,
>>> hence I would have expected it to work the same lowest-comes-first way, but
>>> apparently not – relying on your assessment here. I am also inclined to
>>> align them with the lowest-comes-first strategy, though this might have a
>>> bigger impact if there are 3rd party providers out there in the wild. Maybe
>>> others can weigh in here?
>>
>> I can provide some additional evidence supporting the necessity to
>> invert the order. At least one user on Stack Overflow was surprised
>> that after adding `log4j-core` to his project, the loggers are still
>> of type `org.apache.logging.slf4j.SLF4JLogger`:
>>
>> https://stackoverflow.com/q/70487959/11748454
>>
>> Piotr
>
Re: Order of property sources
Posted by Ralph Goers <ra...@dslextreme.com>.
I would not be in favor of this change. Providers have been designed since day one that highest wins and it is documented - https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory <https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory>.
The fact that the user was surprised may be true, but the ordering was intentional. If you include log4j-to-slf4j it means you want to log to SLF4J. In that case log4j-core shouldn’t even be present.
Ralph
> On Mar 14, 2022, at 4:37 AM, Piotr P. Karwasz <pi...@gmail.com> wrote:
>
> On Mon, Mar 14, 2022 at 10:54 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
>> Regarding your remark about providers... `Provider#getPriority()` has the
>> following javadoc: *"Gets the priority (natural ordering) of this Provider"*,
>> hence I would have expected it to work the same lowest-comes-first way, but
>> apparently not – relying on your assessment here. I am also inclined to
>> align them with the lowest-comes-first strategy, though this might have a
>> bigger impact if there are 3rd party providers out there in the wild. Maybe
>> others can weigh in here?
>
> I can provide some additional evidence supporting the necessity to
> invert the order. At least one user on Stack Overflow was surprised
> that after adding `log4j-core` to his project, the loggers are still
> of type `org.apache.logging.slf4j.SLF4JLogger`:
>
> https://stackoverflow.com/q/70487959/11748454
>
> Piotr
Re: Order of property sources
Posted by "Piotr P. Karwasz" <pi...@gmail.com>.
On Mon, Mar 14, 2022 at 10:54 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
> Regarding your remark about providers... `Provider#getPriority()` has the
> following javadoc: *"Gets the priority (natural ordering) of this Provider"*,
> hence I would have expected it to work the same lowest-comes-first way, but
> apparently not – relying on your assessment here. I am also inclined to
> align them with the lowest-comes-first strategy, though this might have a
> bigger impact if there are 3rd party providers out there in the wild. Maybe
> others can weigh in here?
I can provide some additional evidence supporting the necessity to
invert the order. At least one user on Stack Overflow was surprised
that after adding `log4j-core` to his project, the loggers are still
of type `org.apache.logging.slf4j.SLF4JLogger`:
https://stackoverflow.com/q/70487959/11748454
Piotr
Re: Order of property sources
Posted by Volkan Yazıcı <vo...@yazi.ci>.
Okay, then we have consensus for changing the property source priority
field handling to lowest-comes-first strategy. I recommend landing this
feature in 2.18.0 rather than 2.17.3 or something, since this is
potentially a breaking change of a not working feature.
Regarding your remark about providers... `Provider#getPriority()` has the
following javadoc: *"Gets the priority (natural ordering) of this Provider"*,
hence I would have expected it to work the same lowest-comes-first way, but
apparently not – relying on your assessment here. I am also inclined to
align them with the lowest-comes-first strategy, though this might have a
bigger impact if there are 3rd party providers out there in the wild. Maybe
others can weigh in here?
On Mon, Mar 14, 2022 at 10:38 AM Piotr P. Karwasz <pi...@gmail.com>
wrote:
> Hi Volkan,
>
> On Mon, Mar 14, 2022 at 9:23 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
> > My preference would be the *lowest-value-comes-first* strategy for the
> > following reasons:
> >
> > - Natural ordering in Java (e.g., `Stream#sorted()`) yields the same
> > result
> > - This is how it works for Spring's `@Order` annotation
>
> I like it, together with the current documentation that makes 3
> reasons to keep the current order.
> However `org.apache.logging.log4j.core.config.Order` seems to suggest
> an inverse order.
>
>
> > I would appreciate it if you can align javadoc and manual with this PR
> too.
>
> It should be already aligned.
>
> > Mind explaining what do you mean by *"the provider's priority uses the
> > highest-value-wins strategy"*, please?
>
> Sorry, I was referring to the `org.apache.logging.log4j.spi.Provider`
> selection algorithm. Unless I am mistaken, `LogManager` chooses the
> provider with the highest numerical value:
>
>
> https://github.com/apache/logging-log4j2/blob/10460ec5200aa7eafb33b200f0f349a66e16c708/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java#L86
>
> So if multiple Log4j 2.x API implementations are present, they are
> chosen in the order:
>
> 1. `log4j-jul` (priority 20)
> 2. `log4j-to-slf4j` (priority 15)
> 3. `log4j-core` (priority 10)
>
> Thinking about it, this seems like a bug. I don't recall the provider
> priority being documented anywhere, so maybe this can be also reversed
> in `release-2.x`.
>
> Piotr
>
Re: Order of property sources
Posted by "Piotr P. Karwasz" <pi...@gmail.com>.
Hi Volkan,
On Mon, Mar 14, 2022 at 9:23 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
> My preference would be the *lowest-value-comes-first* strategy for the
> following reasons:
>
> - Natural ordering in Java (e.g., `Stream#sorted()`) yields the same
> result
> - This is how it works for Spring's `@Order` annotation
I like it, together with the current documentation that makes 3
reasons to keep the current order.
However `org.apache.logging.log4j.core.config.Order` seems to suggest
an inverse order.
> I would appreciate it if you can align javadoc and manual with this PR too.
It should be already aligned.
> Mind explaining what do you mean by *"the provider's priority uses the
> highest-value-wins strategy"*, please?
Sorry, I was referring to the `org.apache.logging.log4j.spi.Provider`
selection algorithm. Unless I am mistaken, `LogManager` chooses the
provider with the highest numerical value:
https://github.com/apache/logging-log4j2/blob/10460ec5200aa7eafb33b200f0f349a66e16c708/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java#L86
So if multiple Log4j 2.x API implementations are present, they are
chosen in the order:
1. `log4j-jul` (priority 20)
2. `log4j-to-slf4j` (priority 15)
3. `log4j-core` (priority 10)
Thinking about it, this seems like a bug. I don't recall the provider
priority being documented anywhere, so maybe this can be also reversed
in `release-2.x`.
Piotr
Re: Order of property sources
Posted by Volkan Yazıcı <vo...@yazi.ci>.
My preference would be the *lowest-value-comes-first* strategy for the
following reasons:
- Natural ordering in Java (e.g., `Stream#sorted()`) yields the same
result
- This is how it works for Spring's `@Order` annotation
I would appreciate it if you can align javadoc and manual with this PR too.
Mind explaining what do you mean by *"the provider's priority uses the
highest-value-wins strategy"*, please?
*[Sorry for the late response Piotr. I was off for a week and trying to
catch up. BTW, I can't be happier that you have spotted these ordering
issues and addressed them swiftly. Keep up the great job! 💯]*
On Thu, Mar 10, 2022 at 7:36 AM Piotr P. Karwasz <pi...@gmail.com>
wrote:
> Hello,
>
> Among the problems I found in the ordering of property sources[1],
> there is the question of the meaning of the numeric priority value:
>
> * the manual says that lower numeric values override higher values[2],
> * the Javadoc suggests an inverted order[3].
>
> Which one should be the correct one? At first I implemented the
> documentation order[4], since users might have learned that one
> (although this order never worked, the first JIRA issue reporting it
> is from last December). However this leaves a bitter aftertaste,
> because the provider's priority uses the "highest value wins"
> strategy.
>
> I rebased my PR [4] (taking LOG4J2-3413 into account in the tests) and
> I am planning to merge it in the weekend.
>
> Piotr
>
> [1] https://issues.apache.org/jira/browse/LOG4J2-3366
> [2]
> https://logging.apache.org/log4j/2.x/manual/configuration.html#SystemProperties
> [3]
> https://logging.apache.org/log4j/2.x/log4j-api/apidocs/org/apache/logging/log4j/util/PropertySource.html#getPriority--
> [4] https://github.com/apache/logging-log4j2/pull/742
>