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

Multiple SLF4J bindings #3032

Closed
bfedoronchuk opened this issue May 20, 2021 · 5 comments
Closed

Multiple SLF4J bindings #3032

bfedoronchuk opened this issue May 20, 2021 · 5 comments
Assignees
Labels
bug Something isn't working logging P2
Projects

Comments

@bfedoronchuk
Copy link

bfedoronchuk commented May 20, 2021

Environment Details

  • Helidon Version: 2.3.0
  • Helidon MP
  • JDK version: 11
  • OS: MacOS

Problem Description

It seems that transitive library io.helidon.microprofile.cdi:helidon-microprofile-cdi:2.3.0 includes an SLF4J binding that conflicts with the one configured by application developers.

When running a Helidon MP application with SLF4J binding in dependencies, the following warning from SLF4J is logged at startup:

SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/.../.m2/repository/org/slf4j/slf4j-jdk14/1.7.30/slf4j-jdk14-1.7.30.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/.../.m2/repository/ch/qos/logback/logback-classic/1.2.3/logback-classic-1.2.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.slf4j.impl.JDK14LoggerFactory]

Here is the dependency tree:

[INFO] --- maven-dependency-plugin:3.1.2:tree (default-cli) @ helidon-mp-slf4j ---
[INFO] com.test:helidon-mp-slf4j:jar:1.0-SNAPSHOT
[INFO] +- io.helidon.microprofile.bundles:helidon-microprofile:jar:2.3.0:compile
[INFO] |  +- io.helidon.microprofile.bundles:helidon-microprofile-core:jar:2.3.0:compile
[INFO] |  |  \- io.helidon.microprofile.server:helidon-microprofile-server:jar:2.3.0:compile
[INFO] |  |     +- io.helidon.microprofile.cdi:helidon-microprofile-cdi:jar:2.3.0:compile
[INFO] |  |     |  +- io.helidon.microprofile.weld:weld-se-core:jar:2.3.0:compile
[INFO] |  |     |  |  +- io.helidon.microprofile.weld:weld-core-impl:jar:2.3.0:compile
[INFO] |  |     |  |  +- org.jboss.weld.environment:weld-environment-common:jar:3.1.6.Final:compile
[INFO] |  |     |  |  +- org.jboss.weld:weld-api:jar:3.1.SP2:compile
[INFO] |  |     |  |  +- org.jboss.weld:weld-spi:jar:3.1.SP2:compile
[INFO] |  |     |  |  \- org.jboss.classfilewriter:jboss-classfilewriter:jar:1.2.4.Final:compile
[INFO] |  |     |  \- org.slf4j:slf4j-jdk14:jar:1.7.30:runtime.     <--------
...

So, the wrong SLF4J binding was chosen during startup.
Seems like this issue is similar to: #786

Steps to reproduce

Build and run an application with the following parent and dependencies:

    <parent>
        <groupId>io.helidon.applications</groupId>
        <artifactId>helidon-mp</artifactId>
        <version>2.3.0</version>
        <relativePath/>
    </parent>
    ...
        <dependency>
            <groupId>io.helidon.microprofile.bundles</groupId>
            <artifactId>helidon-microprofile</artifactId>
        </dependency>
        <dependency>
            <groupId>jakarta.activation</groupId>
            <artifactId>jakarta.activation-api</artifactId>
            <scope>runtime</scope>
        </dependency>
       
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
        </dependency>
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>jul-to-slf4j</artifactId>
        </dependency>
        <dependency>
            <groupId>io.helidon.logging</groupId>
            <artifactId>helidon-logging-slf4j</artifactId>
        </dependency>
        <dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-classic</artifactId>
        </dependency>

The warning is logged at startup.

@danielkec danielkec added this to Needs triage in Backlog via automation May 20, 2021
@m0mus m0mus added the bug Something isn't working label May 20, 2021
@romain-grecourt
Copy link
Contributor

From http://www.slf4j.org/codes.html#multiple_bindings:

Embedded components such as libraries or frameworks should not declare a dependency on any SLF4J binding but only depend on slf4j-api. When a library declares a compile-time dependency on a SLF4J binding, it imposes that binding on the end-user, thus negating SLF4J's purpose. When you come across an embedded component declaring a compile-time dependency on any SLF4J binding, please take the time to contact the authors of said component/library and kindly ask them to mend their ways.

Basically this means this helidon-microprofile-cdi should NOT declare a dependency on org.slf4j:slf4j-jdk14

The pattern as described by the SL4J documentation is to let the user pick a binding implementation, this means we need to add an explicit dependency to a binding artifact in every example, or MP based archetype.

CC @tomas-langer @barchetta

dalexandrov added a commit to dalexandrov/helidon that referenced this issue May 25, 2021
@m0mus m0mus moved this from Needs triage to In Progress in Backlog May 27, 2021
@barchetta
Copy link
Member

I searched the source for dependencies on slf4j-jdk14 that are not in applications or tests (or test scope) and came up with this list. We should inspect each of these:

./microprofile/grpc/metrics/pom.xml:66:            <artifactId>slf4j-jdk14</artifactId>
./microprofile/grpc/server/pom.xml:74:            <artifactId>slf4j-jdk14</artifactId>
./microprofile/cdi/pom.xml:56:            <artifactId>slf4j-jdk14</artifactId>
./tracing/jaeger/pom.xml:64:            <artifactId>slf4j-jdk14</artifactId>

Backlog automation moved this from In Progress to Closed Jul 1, 2021
@bfedoronchuk
Copy link
Author

bfedoronchuk commented Jul 5, 2021

Guys, this issue is closed but there are no resolutions like a linked ticket or closed PR. What's next with it?

@romain-grecourt
Copy link
Contributor

Things look out of order, not sure what happened. The PR (#3047) just got merged, so the changes will be available with the next 2.x release.

@bfedoronchuk
Copy link
Author

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logging P2
Projects
Backlog
  
Closed
Development

No branches or pull requests

5 participants