Re: LOG4J2-3243

[Ralph Goers <ra...@dslextreme.com>]

Volkan pointed out that the issue number in the subject was wrong.

Ralph

> On Dec 21, 2021, at 10:30 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> 
> This ticket complains because ConfigurationFactory looks to see if a system property named log4j.configuration is set.
> If it is then it tries to initialize the configuration it points to as a Log4j 1.x configuration using the PropertiesConfiguration I implemented.
> 
> Unfortunately, this is the same property name that Log4j 1.x uses. I probably thought it was a good thing at the time 
> but now that I think about it I believe it was a mistake.
> 
> The Log4j 1.x compatibility is still marked experimental. So I would like to propose that the property be renamed to log4j1.configurationFile. 
> It matches the format used for the Log4j 2 property but is clearly meant to reference a Log4j 1.x configuration. This would require users 
> who are using the compatibility (if there are any) to change the system property name but it would allow log4j 1.x to continue to function 
> if it is present in the app.
> 
> I do have a concern. Is this going to somehow be renamed as log4j2.log4j1.configurationFile by the properties system? That is ugly.
> 
> Thoughts?
> 
> Ralph

Re: LOG4J2-3243

[Volkan Yazıcı <vo...@yazi.ci>]

Thanks for the clarification. I will try to enumerate all the cases below:

*[Below, 1, 2, and B denote Log4j 1, Log4j 2, and log4j-1.2-api,
respectively.]*

*1/2/B:* failure, both 1 and B cannot be present (equivalent to #1 in your
enumeration)
*1/2/-:* 2 should detect `log4j.configuration`, yet due to the absence of
B, discard it; a warning is not necessary since 1 is present (equivalent to
#2 in your enumeration)
*1/-/B:* 1 should do all the work
*1/-/-:* 1 should do all the work
*-/2/B:* 2 should detect `log4j.configuration` and employ it (equivalent to
#3 in your enumeration)
*-/2/-:* 2 should detect `log4j.configuration`, yet due to the absence of
B, warn & discard it (equivalent to #4 in your enumeration)
*-/-/B:* user doesn't know what he/she is doing
*-/-/-:* recess for all

I think we can test against all these cases using dedicated
maven-surefire-plugin configurations where we exclude either 1, 2, and/or B
as needed.

Regarding Carter's *"when interesting plugin/webapp classloaders are used"*
remark... I will act like I haven't read that. 😅

On Wed, Dec 22, 2021 at 6:13 PM Ralph Goers <ra...@dslextreme.com>
wrote:

>
>
> > On Dec 22, 2021, at 9:20 AM, Volkan Yazıcı <vo...@yazi.ci> wrote:
> >
> > Ralph, mind elaborating a bit more on what the exact problem is, please?
> > `log4j.configuration` gets detected, log4j-1.2-api (provided by Log4j 2)
> > kicks in, and tries to load the Log4j 1 configuration. This sounds okay
> to
> > me. I guess it gets messed up when an application uses both Log4j 1 and
> 2,
> > right?
>
>
> Yes, if log4j 1.x is on the classpath they both think the system property
> is meant for them. There are a few scenarios:
> 1. Log4j 1.x and log4j-1.2-api are both present. This is an error as you
> can’t have both the legacy implementation and the compatibility bridge
> present at the same time.
> 2. Log4j-1.x is present and log4j-1.2-api is not present. Log4j will look
> for a factory for Log4j 1.x. It won’t find one and Log4j 2 will fail to
> configure.
> 3. Log4j 1.x is not present and log4j-1.2-api is present. Log4j will look
> for a factory for Log4j 1.x and find it in log4j-1.2-api.
> 4. Neither Log4j 1.x or Log4j-1.2-api are present. ConfigurationFactory
> won’t find a factory and will return null.
>
> It seems the logic here needs to be changed so that if it can’t find a
> Log4j 1.x configuration that it should continue looking for a Log4j 2
> configuration.  I guess having it use the Log4j 1.x property name isn’t the
> problem.
>
> Ralph

Re: LOG4J2-3243

[Ralph Goers <ra...@dslextreme.com>]

> On Dec 22, 2021, at 9:20 AM, Volkan Yazıcı <vo...@yazi.ci> wrote:
> 
> Ralph, mind elaborating a bit more on what the exact problem is, please?
> `log4j.configuration` gets detected, log4j-1.2-api (provided by Log4j 2)
> kicks in, and tries to load the Log4j 1 configuration. This sounds okay to
> me. I guess it gets messed up when an application uses both Log4j 1 and 2,
> right?


Yes, if log4j 1.x is on the classpath they both think the system property is meant for them. There are a few scenarios:
1. Log4j 1.x and log4j-1.2-api are both present. This is an error as you can’t have both the legacy implementation and the compatibility bridge present at the same time.
2. Log4j-1.x is present and log4j-1.2-api is not present. Log4j will look for a factory for Log4j 1.x. It won’t find one and Log4j 2 will fail to configure.
3. Log4j 1.x is not present and log4j-1.2-api is present. Log4j will look for a factory for Log4j 1.x and find it in log4j-1.2-api.
4. Neither Log4j 1.x or Log4j-1.2-api are present. ConfigurationFactory won’t find a factory and will return null.

It seems the logic here needs to be changed so that if it can’t find a Log4j 1.x configuration that it should continue looking for a Log4j 2 configuration.  I guess having it use the Log4j 1.x property name isn’t the problem.

Ralph

Re: LOG4J2-3243

[Carter Kozak <ck...@ckozak.net>]

Is the case that the log4j2 log4j-1.2-api is not on the classpath, but log4-1.2 itself is? Ideally we could detect that case and allow log4j1 to do its thing, but that's easier said than done outside of standard cases (for instance when interesting plugin/webapp classloaders are used)

On Wed, Dec 22, 2021, at 11:20, Volkan Yazıcı wrote:
> Ralph, mind elaborating a bit more on what the exact problem is, please?
> `log4j.configuration` gets detected, log4j-1.2-api (provided by Log4j 2)
> kicks in, and tries to load the Log4j 1 configuration. This sounds okay to
> me. I guess it gets messed up when an application uses both Log4j 1 and 2,
> right?
> 
> On Wed, Dec 22, 2021 at 5:14 PM Ralph Goers <ra...@dslextreme.com>
> wrote:
> 
> > Volkan pointed out that the issue number in the subject was wrong.
> >
> > Ralph
> >
> > > On Dec 21, 2021, at 10:30 PM, Ralph Goers <ra...@dslextreme.com>
> > wrote:
> > >
> > > This ticket complains because ConfigurationFactory looks to see if a
> > system property named log4j.configuration is set.
> > > If it is then it tries to initialize the configuration it points to as a
> > Log4j 1.x configuration using the PropertiesConfiguration I implemented.
> > >
> > > Unfortunately, this is the same property name that Log4j 1.x uses. I
> > probably thought it was a good thing at the time
> > > but now that I think about it I believe it was a mistake.
> > >
> > > The Log4j 1.x compatibility is still marked experimental. So I would
> > like to propose that the property be renamed to log4j1.configurationFile.
> > > It matches the format used for the Log4j 2 property but is clearly meant
> > to reference a Log4j 1.x configuration. This would require users
> > > who are using the compatibility (if there are any) to change the system
> > property name but it would allow log4j 1.x to continue to function
> > > if it is present in the app.
> > >
> > > I do have a concern. Is this going to somehow be renamed as
> > log4j2.log4j1.configurationFile by the properties system? That is ugly.
> > >
> > > Thoughts?
> > >
> > > Ralph
> >
> >
> 

-ck

Re: LOG4J2-3243

[Volkan Yazıcı <vo...@yazi.ci>]

Ralph, mind elaborating a bit more on what the exact problem is, please?
`log4j.configuration` gets detected, log4j-1.2-api (provided by Log4j 2)
kicks in, and tries to load the Log4j 1 configuration. This sounds okay to
me. I guess it gets messed up when an application uses both Log4j 1 and 2,
right?

On Wed, Dec 22, 2021 at 5:14 PM Ralph Goers <ra...@dslextreme.com>
wrote:

> Volkan pointed out that the issue number in the subject was wrong.
>
> Ralph
>
> > On Dec 21, 2021, at 10:30 PM, Ralph Goers <ra...@dslextreme.com>
> wrote:
> >
> > This ticket complains because ConfigurationFactory looks to see if a
> system property named log4j.configuration is set.
> > If it is then it tries to initialize the configuration it points to as a
> Log4j 1.x configuration using the PropertiesConfiguration I implemented.
> >
> > Unfortunately, this is the same property name that Log4j 1.x uses. I
> probably thought it was a good thing at the time
> > but now that I think about it I believe it was a mistake.
> >
> > The Log4j 1.x compatibility is still marked experimental. So I would
> like to propose that the property be renamed to log4j1.configurationFile.
> > It matches the format used for the Log4j 2 property but is clearly meant
> to reference a Log4j 1.x configuration. This would require users
> > who are using the compatibility (if there are any) to change the system
> property name but it would allow log4j 1.x to continue to function
> > if it is present in the app.
> >
> > I do have a concern. Is this going to somehow be renamed as
> log4j2.log4j1.configurationFile by the properties system? That is ugly.
> >
> > Thoughts?
> >
> > Ralph
>
>