Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JENKINS-50970 SLF4J logging not working in Swarm client #98

Merged
merged 1 commit into from
Jun 1, 2019

Conversation

basil
Copy link
Member

@basil basil commented Jun 1, 2019

Problem

When the Swarm Client starts, the following message gets printed:

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder"
SLF4J: Defaulting to no-operation (NOP) loger implementation

Note that this doesn't prevent java.util.logging logs from getting printed.

Evaluation

Both Swarm and Remoting use java.util.logging and import neither SLF4J nor Jakarta Commons Logging (JCL), so I wondered SLF4J was in the picture at all here. I asked myself: should we remove SLF4J? Then I realized that we depend on commons-httpclient, which itself depends on JCL:

+- commons-httpclient:commons-httpclient:jar:3.1:compile
|  \- commons-logging:commons-logging:jar:1.0.4:compile

So SLF4J isn't completely useless; we can use it to bridge the log statements made by commons-httpclient to the java.util.logging stream already used by Swarm and Remoting. But right now SLF4J doesn't have any backend configured, hence the warning.

Solution

Add an SLF4J backend. Of the many available choices, it made the most sense to use the java.util.logging backend. The vast majority of log statements are already made to java.util.logging directly by Swarm and Remoting. This change just adds the log statements made by commons-httpclient (through JCL) to this stream.

Bonus

Bumped the version of maven-shade-plugin while I was here.

Testing

Ran the Swarm client with a verbose java.util.logging configuration. Ensured that the error had disappeared and that JCL log statements from commons-httpclient were making it into the FINE level logs via SLF4J.

@basil basil merged commit f04e9cc into jenkinsci:master Jun 1, 2019
@darxriggs
Copy link
Contributor

Well done. I prepared nearly the same changes locally at the same time. :-)

One question: Can commons-logging:commons-logging not be removed as an implementation is provided by org.slf4j:jcl-over-slf4j?

@basil
Copy link
Member Author

basil commented Jun 2, 2019

Well done. I prepared nearly the same changes locally at the same time. :-)

:-)

I can guarantee you that I won't be racing with you to implement the build system cleanup suggested in #94. ;-) I don't know enough about Maven multi-project builds yet (I work with Gradle professionally).

One question: Can commons-logging:commons-logging not be removed as an implementation is provided by org.slf4j:jcl-over-slf4j?

Maybe... I'm not sure. I was going by the example in the "provided scope" section here. This is also how Jenkins core does it. At this point, I'm not inclined to go against the grain, especially since the solution is already working.

@darxriggs
Copy link
Contributor

After reading the quoted slf4j FAQ section the current implementation totally makes sense to me. Thanks.

@basil basil deleted the JENKINS-50970 branch June 2, 2019 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants