Fix Slf4j providers not found error #2420

Torch3333 · 2024-12-19T00:54:25Z

Description

~~This updates slf4j to the latest version to resolve the following logger binding issue when running ScalarDB unit or integration test.~~
This forces Gradle to use the slf4j version defined by ScalarDB over any version pulled transitively.

SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.
SLF4J: Class path contains SLF4J bindings targeting slf4j-api versions 1.7.x or earlier.
SLF4J: Ignoring binding found at [jar:file:/Users/vguilpain/.gradle/caches/modules-2/files-2.1/org.slf4j/slf4j-simple/1.7.36/a41f9cfe6faafb2eb83a1c7dd2d0dfd844e2a936/slf4j-simple-1.7.36.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See https://www.slf4j.org/codes.html#ignoredBindings for an explanation.

This problem appeared after adding the MariaDB driver dependency in #2391. This dependency pulls slf4j-api version 2.X, but sl4j-simple stays with version 1.7.X, causing a mismatch.

Related issues and/or PRs

N/A

Changes made

Update slf4j to the latest version.

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release note

N/A

komamitsu · 2024-12-19T02:25:15Z

@Torch3333 Can you tell me how to reproduce the warning message?

Torch3333 · 2024-12-19T02:41:27Z

@komamitsu
Running any integration or unit test produces this error message on my IDE (IntellijJ)

komamitsu

LGTM! 👍

brfrn169

LGTM! Thank you!

brfrn169 · 2024-12-19T06:23:09Z

@Torch3333 Ah, this change might break backward compatibility.

For example, if a user is using log4j with slf4j 1.x, they need to use log4j-slf4j-impl as follows:

    implementation "org.slf4j:slf4j-api:1.7.36"
    implementation "org.apache.logging.log4j:log4j-core:2.24.3"
    implementation "org.apache.logging.log4j:log4j-slf4j-impl:2.24.3"

However, if a user is using log4j with slf4j 2.x, they need to use log4j-slf4j2-impl as follows:

    implementation "org.slf4j:slf4j-api:2.0.16"
    implementation "org.apache.logging.log4j:log4j-core:2.24.3"
    implementation "org.apache.logging.log4j:log4j-slf4j2-impl:2.24.3"

Therefore, if we upgrade slf4j to 2.x, users will need to change their log4j module from log4j-slf4j-impl to log4j-slf4j2-impl. What do you think?

Torch3333 · 2024-12-19T07:24:59Z

@brfrn169 @komamitsu
Indeed, good point. Thank you.

I went with a different approach and set up Gradle to force the usage of the slf4j version set by ScalarDB over any transitively pulled version.
cf. the Gradle doc about the resolutionStrategy.force feature.

PTAL. 🙇

komamitsu · 2024-12-20T05:17:13Z

IIUC, the new way makes the library (org.mariadb.jdbc:mariadb-java-client -> com.github.waffle:waffle-jna) introduced org.slf4j:slf4j-api:2.0.7 use 1.7.36? It might cause another problem although it depends use cases.

|    +--- org.mariadb.jdbc:mariadb-java-client:3.5.1
|    |    \--- com.github.waffle:waffle-jna:3.3.0
|    |         +--- net.java.dev.jna:jna:5.13.0
|    |         +--- net.java.dev.jna:jna-platform:5.13.0
|    |         |    \--- net.java.dev.jna:jna:5.13.0
|    |         +--- org.slf4j:jcl-over-slf4j:2.0.7
|    |         |    \--- org.slf4j:slf4j-api:2.0.7 -> 1.7.36
|    |         +--- org.slf4j:slf4j-api:2.0.7 -> 1.7.36

I'm not sure if we need to continue using 1.x forever. I think our customers and we need to upgrade the dependency someday...

brfrn169 · 2024-12-25T07:56:08Z

I'm not sure if we need to continue using 1.x forever. I think our customers and we need to upgrade the dependency someday...

@komamitsu I think we can upgrade in the next major version (4.0.0). Or do you think it should be done in the next minor version instead?

brfrn169 · 2024-12-25T08:05:34Z

build.gradle

@@ -89,5 +89,9 @@ subprojects {
    installDist {
        duplicatesStrategy DuplicatesStrategy.EXCLUDE
    }
+
+    configurations.all {
+       resolutionStrategy.force "org.slf4j:slf4j-api:${slf4jVersion}", "org.slf4j:slf4j-simple:${slf4jVersion}"


Instead of this, how about excluding the slf4j dependency from the mariadb dependency?

Agreed. I think excluding it is a bit more polite way.

Yes, we can.
I thought that using the resolutionStrategy.force was future-proof in case another library we update pulls slf4j v2.

The Gradle doc explains that using the resolutionStrategy.force is a project-global excluding rule.

Excluding a particular transitive dependency does not guarantee that it does not show up in the dependencies of a given configuration. For example, some other dependency, which does not have any exclude rules, might pull in exactly the same transitive dependency. To guarantee that the transitive dependency is excluded from the entire configuration please use per-configuration exclude rules: [getExcludeRules](https://docs.gradle.org/current/kotlin-dsl/gradle/org.gradle.api.artifacts/-configuration/get-exclude-rules.html). In fact, in a majority of cases the actual intention of configuring per-dependency exclusions is really excluding a dependency from the entire configuration (or classpath). If your intention is to exclude a particular transitive dependency because you don't like the version it pulls in to the configuration then consider using forced versions' feature: [force](https://docs.gradle.org/current/kotlin-dsl/gradle/org.gradle.api.artifacts/-resolution-strategy/force.html).

cf. https://docs.gradle.org/current/kotlin-dsl/gradle/org.gradle.api.artifacts/-module-dependency/exclude.html

Could you share your concerns concerning the resolutionStrategy.force ?
I am ok with using either method but I just want to better understand your point of view.

The 2 ways have similar effects, and actually either is fine with me. IIUC, the slight difference is basically only global-scope or not. So, this might be also a matter of taste thing. I relatively prefer starting with small-scope effect, but I don't insist on it as I understand the pros of resolutionStrategy.force.

I was concerned about the side effects of resolutionStrategy.force since it’s a project-global configuration. Also, I believe we’ve usually used the exclude configuration for similar issues in the past. Do you think resolutionStrategy.force is a better approach than the exclude configuration?

My understanding was that resolutionStrategy.force could be used as project-wide exclude which means:

The forced dependency version was used project-wide when building ScalarDB

Other projects using ScalarDB as a dependency would also be presented with the forced dependency version, i.e. excluding the non-wanted version

But after testing locally, statement 2. is wrong. slf4j-api:2.0.7 was still listed as a transitive dependency of com.github.waffle:waffle-jna when using ScalarDB as an implementation dependency on a Gradle project.

In comparison, when using the exclude syntax. slf4j-api:2.0.7 was not listed as a dependency or pulled by com.github.waffle:waffle-jna when using ScalarDB as an implementation dependency on a Gradle project.
This is what we want to avoid the problem described by Toshi in #2420 (comment)

So, I will revise the PR to use the exclude syntax.

komamitsu · 2024-12-26T02:16:00Z

@brfrn169

I think we can upgrade in the next major version (4.0.0)

Sounds okay!

|    +--- org.mariadb.jdbc:mariadb-java-client:3.5.1
|    |    \--- com.github.waffle:waffle-jna:3.3.0
|    |         +--- net.java.dev.jna:jna:5.13.0
|    |         +--- net.java.dev.jna:jna-platform:5.13.0
|    |         |    \--- net.java.dev.jna:jna:5.13.0
|    |         +--- org.slf4j:jcl-over-slf4j:2.0.7
|    |         |    \--- org.slf4j:slf4j-api:2.0.7 -> 1.7.36
|    |         +--- org.slf4j:slf4j-api:2.0.7 -> 1.7.36

(cc: @Torch3333)

BTW, I checked the source code of com.github.waffle:waffle-jna and confirmed it doesn't use SLF4J's Fluent API introduced in v2 (See https://www.slf4j.org/faq.html#compatibility). And the library is for Windows auth. So, it should be completely safe to keep using org.slf4j:slf4j-api:1.7.36.

Torch3333 · 2024-12-26T02:22:27Z

@komamitsu

@brfrn169

I think we can upgrade in the next major version (4.0.0)

Sounds okay!
|    +--- org.mariadb.jdbc:mariadb-java-client:3.5.1
|    |    \--- com.github.waffle:waffle-jna:3.3.0
|    |         +--- net.java.dev.jna:jna:5.13.0
|    |         +--- net.java.dev.jna:jna-platform:5.13.0
|    |         |    \--- net.java.dev.jna:jna:5.13.0
|    |         +--- org.slf4j:jcl-over-slf4j:2.0.7
|    |         |    \--- org.slf4j:slf4j-api:2.0.7 -> 1.7.36
|    |         +--- org.slf4j:slf4j-api:2.0.7 -> 1.7.36
(cc: @Torch3333)

BTW, I checked the source code of com.github.waffle:waffle-jna and confirmed it doesn't use SLF4J's Fluent API introduced in v2 (See https://www.slf4j.org/faq.html#compatibility). And the library is for Windows auth. So, it should be completely safe to keep using org.slf4j:slf4j-api:1.7.36.

Ok, thank you for taking the time to check!

komamitsu

LGTM! 👍

Update slf4j major version

edd40f4

Torch3333 self-assigned this Dec 19, 2024

Torch3333 added the improvement label Dec 19, 2024

Torch3333 marked this pull request as ready for review December 19, 2024 01:44

Torch3333 requested review from a team, komamitsu, brfrn169 and feeblefakie and removed request for a team December 19, 2024 01:44

komamitsu approved these changes Dec 19, 2024

View reviewed changes

brfrn169 approved these changes Dec 19, 2024

View reviewed changes

Force usage of slf4j version set in ScalarDB

4e43092

Torch3333 changed the title ~~Update slf4j major version~~ Fix Slf4j providers not found error Dec 19, 2024

Torch3333 requested review from brfrn169 and komamitsu December 19, 2024 07:25

brfrn169 reviewed Dec 25, 2024

View reviewed changes

komamitsu approved these changes Dec 26, 2024

View reviewed changes

Exclude slf4j-api from maria-db driver

0d6afcf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Slf4j providers not found error #2420

Fix Slf4j providers not found error #2420

Torch3333 commented Dec 19, 2024 •

edited

Loading

komamitsu commented Dec 19, 2024

Torch3333 commented Dec 19, 2024 •

edited

Loading

komamitsu left a comment

brfrn169 left a comment

brfrn169 commented Dec 19, 2024 •

edited

Loading

Torch3333 commented Dec 19, 2024 •

edited

Loading

komamitsu commented Dec 20, 2024

brfrn169 commented Dec 25, 2024

brfrn169 Dec 25, 2024

komamitsu Dec 26, 2024

Torch3333 Dec 26, 2024 •

edited

Loading

Torch3333 Dec 26, 2024 •

edited

Loading

komamitsu Dec 26, 2024

brfrn169 Dec 26, 2024

Torch3333 Dec 26, 2024 •

edited

Loading

komamitsu commented Dec 26, 2024

Torch3333 commented Dec 26, 2024 •

edited

Loading

komamitsu left a comment

Fix Slf4j providers not found error #2420

Are you sure you want to change the base?

Fix Slf4j providers not found error #2420

Conversation

Torch3333 commented Dec 19, 2024 • edited Loading

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release note

komamitsu commented Dec 19, 2024

Torch3333 commented Dec 19, 2024 • edited Loading

komamitsu left a comment

Choose a reason for hiding this comment

brfrn169 left a comment

Choose a reason for hiding this comment

brfrn169 commented Dec 19, 2024 • edited Loading

Torch3333 commented Dec 19, 2024 • edited Loading

komamitsu commented Dec 20, 2024

brfrn169 commented Dec 25, 2024

brfrn169 Dec 25, 2024

Choose a reason for hiding this comment

komamitsu Dec 26, 2024

Choose a reason for hiding this comment

Torch3333 Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

Torch3333 Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

komamitsu Dec 26, 2024

Choose a reason for hiding this comment

brfrn169 Dec 26, 2024

Choose a reason for hiding this comment

Torch3333 Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

komamitsu commented Dec 26, 2024

Torch3333 commented Dec 26, 2024 • edited Loading

komamitsu left a comment

Choose a reason for hiding this comment

Torch3333 commented Dec 19, 2024 •

edited

Loading

Torch3333 commented Dec 19, 2024 •

edited

Loading

brfrn169 commented Dec 19, 2024 •

edited

Loading

Torch3333 commented Dec 19, 2024 •

edited

Loading

Torch3333 Dec 26, 2024 •

edited

Loading

Torch3333 Dec 26, 2024 •

edited

Loading

Torch3333 Dec 26, 2024 •

edited

Loading

Torch3333 commented Dec 26, 2024 •

edited

Loading