log4j-slf4j-impl dependency in solr-core

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

log4j-slf4j-impl dependency in solr-core

Jan Høydahl / Cominvent
Hi

Upgrading to log4j2 in https://issues.apache.org/jira/browse/SOLR-7887 added dependency on a bunch of log4j libs to solr-core, e.g. log4j-slf4j-impl which is a binding for slf4j to log using log4j.
This seems wrong since people who consume solr-core through Maven should be able to freely choose logging framework by throwing in the desired static binding jars on classpath.
Now you need to <exclude>org.apache.logging.log4j:log4j-slf4j-impl</exclude> in your solr-core dependency which is a workaround but not in the true spirit of slf4j.

WDYT? Shawn?

--
Jan Høydahl, search solution architect
Cominvent AS - www.cominvent.com


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: log4j-slf4j-impl dependency in solr-core

Shawn Heisey-2
On 9/11/2019 4:45 AM, Jan Høydahl wrote:
> Upgrading to log4j2 in https://issues.apache.org/jira/browse/SOLR-7887 added dependency on a bunch of log4j libs to solr-core, e.g. log4j-slf4j-impl which is a binding for slf4j to log using log4j.
> This seems wrong since people who consume solr-core through Maven should be able to freely choose logging framework by throwing in the desired static binding jars on classpath.
> Now you need to <exclude>org.apache.logging.log4j:log4j-slf4j-impl</exclude> in your solr-core dependency which is a workaround but not in the true spirit of slf4j.
>
> WDYT? Shawn?

I think it's correct for the Solr server to depend on log4j libraries by
default.  The only logging implementation that the admin UI supports is
log4j.  If somebody running our binary release wants to use a different
logging implementation on the server side (knowing that they can't
access logging in the admin UI), that can be done by replacing jars in
lib/ext.  Which jars to add and remove is specific to the desired
logging implementation.  The ref guide probably does need some info there.

If there are any users that integrate solr-core into their own code,
it's currently a bit of a land-mine situation to change logging
implementations.  If there's a way we can include log4j jars at build
time, but remove the log4j dependency on the published solr-core
artifact, that might work well.  We should do our best to make it so
people can use EmbeddedSolrServer without log4j jars.

If solrj depends on log4j jars, that's a problem.  It should only depend
on slf4j-api.  I believe this is the way it works now.  For the majority
of user code, solrj is what they'll use.

The log4j-slf4j-impl jar contains the org.slf4j.impl package, with 3
classes.  It also contains the org.apache.logging.slf4j package, with 7
classes.

Unless my investigation was flawed, only one of the 3 classes in
org.sl4j.impl is used in Solr code, and it's in exactly two places:
LogWatcher and StartupLoggingUtils.

The two classes that are used everywhere across Solr are in the
org.slf4j package, named Logger and LoggerFactory.  I checked the
WEB-INF/lib directory in the solr-8.2.0 download and could not find any
jars related to logging ... and I expected to find one, preferably
sl4fj-api directly from slf4j.org.  Instead, that jar is in lib/ext with
the log4j jars.  That location doesn't seem right, as slf4j-api is
ALWAYS required by Solr, regardless of the logging implementation that's
in use.

The slf4j-api dependency only provides part of what's used in Solr
server-side code, but it SHOULD fully satisfy compiling dependencies for
solrj.  Prior to version 7.4, the org.slf4j.impl package (for
server-side code only) was supplied by the slf4j-log4j12 jar, which came
from the slf4j project.  This is the jar that links slf4j and log4j1.
For log4j2, classes in that package come from Apache, not slf4j.

Thanks,
Shawn

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: log4j-slf4j-impl dependency in solr-core

Jan Høydahl / Cominvent
Slf4j-api and log4j dependencies are ok. But the slf4j-to-log4j static binding jar should be in the deploy phase, not a compile-time dep.

Jan Høydahl

> 11. sep. 2019 kl. 17:40 skrev Shawn Heisey <[hidden email]>:
>
>> On 9/11/2019 4:45 AM, Jan Høydahl wrote:
>> Upgrading to log4j2 in https://issues.apache.org/jira/browse/SOLR-7887 added dependency on a bunch of log4j libs to solr-core, e.g. log4j-slf4j-impl which is a binding for slf4j to log using log4j.
>> This seems wrong since people who consume solr-core through Maven should be able to freely choose logging framework by throwing in the desired static binding jars on classpath.
>> Now you need to <exclude>org.apache.logging.log4j:log4j-slf4j-impl</exclude> in your solr-core dependency which is a workaround but not in the true spirit of slf4j.
>> WDYT? Shawn?
>
> I think it's correct for the Solr server to depend on log4j libraries by default.  The only logging implementation that the admin UI supports is log4j.  If somebody running our binary release wants to use a different logging implementation on the server side (knowing that they can't access logging in the admin UI), that can be done by replacing jars in lib/ext.  Which jars to add and remove is specific to the desired logging implementation.  The ref guide probably does need some info there.
>
> If there are any users that integrate solr-core into their own code, it's currently a bit of a land-mine situation to change logging implementations.  If there's a way we can include log4j jars at build time, but remove the log4j dependency on the published solr-core artifact, that might work well.  We should do our best to make it so people can use EmbeddedSolrServer without log4j jars.
>
> If solrj depends on log4j jars, that's a problem.  It should only depend on slf4j-api.  I believe this is the way it works now.  For the majority of user code, solrj is what they'll use.
>
> The log4j-slf4j-impl jar contains the org.slf4j.impl package, with 3 classes.  It also contains the org.apache.logging.slf4j package, with 7 classes.
>
> Unless my investigation was flawed, only one of the 3 classes in org.sl4j.impl is used in Solr code, and it's in exactly two places: LogWatcher and StartupLoggingUtils.
>
> The two classes that are used everywhere across Solr are in the org.slf4j package, named Logger and LoggerFactory.  I checked the WEB-INF/lib directory in the solr-8.2.0 download and could not find any jars related to logging ... and I expected to find one, preferably sl4fj-api directly from slf4j.org.  Instead, that jar is in lib/ext with the log4j jars.  That location doesn't seem right, as slf4j-api is ALWAYS required by Solr, regardless of the logging implementation that's in use.
>
> The slf4j-api dependency only provides part of what's used in Solr server-side code, but it SHOULD fully satisfy compiling dependencies for solrj.  Prior to version 7.4, the org.slf4j.impl package (for server-side code only) was supplied by the slf4j-log4j12 jar, which came from the slf4j project.  This is the jar that links slf4j and log4j1. For log4j2, classes in that package come from Apache, not slf4j.
>
> Thanks,
> Shawn
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: log4j-slf4j-impl dependency in solr-core

Mark Miller-3
Going to handle this better in the gradle build.

We should either require a specific logging impl or get better at managing our logging jars (gradle is gonna help there).

Solr should not rely on any slf4j impl jar. It does this for some UI stuff, but if we want to do that and properly support changing the logging, we can't just stick slf4j bindings in the webapp. If the UI is going to depend on specific logging impls it needs to work when those jars are not present on the classpath. In some cases you can do this, like with the actual logging impl jars, that's not really a big deal, but it can't be done with the slf4j binding jars.

I'll fix all of this in some way on the gradle branch.

It's fine if slf4j-api is in ext as well - that's the place for our logging jars, Solr is not a webapp, I don't see any issue there. 

On Sat, Sep 14, 2019 at 5:23 AM Jan Høydahl <[hidden email]> wrote:
Slf4j-api and log4j dependencies are ok. But the slf4j-to-log4j static binding jar should be in the deploy phase, not a compile-time dep.

Jan Høydahl

> 11. sep. 2019 kl. 17:40 skrev Shawn Heisey <[hidden email]>:
>
>> On 9/11/2019 4:45 AM, Jan Høydahl wrote:
>> Upgrading to log4j2 in https://issues.apache.org/jira/browse/SOLR-7887 added dependency on a bunch of log4j libs to solr-core, e.g. log4j-slf4j-impl which is a binding for slf4j to log using log4j.
>> This seems wrong since people who consume solr-core through Maven should be able to freely choose logging framework by throwing in the desired static binding jars on classpath.
>> Now you need to <exclude>org.apache.logging.log4j:log4j-slf4j-impl</exclude> in your solr-core dependency which is a workaround but not in the true spirit of slf4j.
>> WDYT? Shawn?
>
> I think it's correct for the Solr server to depend on log4j libraries by default.  The only logging implementation that the admin UI supports is log4j.  If somebody running our binary release wants to use a different logging implementation on the server side (knowing that they can't access logging in the admin UI), that can be done by replacing jars in lib/ext.  Which jars to add and remove is specific to the desired logging implementation.  The ref guide probably does need some info there.
>
> If there are any users that integrate solr-core into their own code, it's currently a bit of a land-mine situation to change logging implementations.  If there's a way we can include log4j jars at build time, but remove the log4j dependency on the published solr-core artifact, that might work well.  We should do our best to make it so people can use EmbeddedSolrServer without log4j jars.
>
> If solrj depends on log4j jars, that's a problem.  It should only depend on slf4j-api.  I believe this is the way it works now.  For the majority of user code, solrj is what they'll use.
>
> The log4j-slf4j-impl jar contains the org.slf4j.impl package, with 3 classes.  It also contains the org.apache.logging.slf4j package, with 7 classes.
>
> Unless my investigation was flawed, only one of the 3 classes in org.sl4j.impl is used in Solr code, and it's in exactly two places: LogWatcher and StartupLoggingUtils.
>
> The two classes that are used everywhere across Solr are in the org.slf4j package, named Logger and LoggerFactory.  I checked the WEB-INF/lib directory in the solr-8.2.0 download and could not find any jars related to logging ... and I expected to find one, preferably sl4fj-api directly from slf4j.org.  Instead, that jar is in lib/ext with the log4j jars.  That location doesn't seem right, as slf4j-api is ALWAYS required by Solr, regardless of the logging implementation that's in use.
>
> The slf4j-api dependency only provides part of what's used in Solr server-side code, but it SHOULD fully satisfy compiling dependencies for solrj.  Prior to version 7.4, the org.slf4j.impl package (for server-side code only) was supplied by the slf4j-log4j12 jar, which came from the slf4j project.  This is the jar that links slf4j and log4j1. For log4j2, classes in that package come from Apache, not slf4j.
>
> Thanks,
> Shawn
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]



--