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

KAFKA-18804: Remove slf4j warning when using tool script #18918

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

Yunyung
Copy link
Contributor

@Yunyung Yunyung commented Feb 16, 2025

Description

When running tools scripts, we encounter "multiple SLF4J bindings" warnings. Take some random scripts, for example:

$ ./bin/kafka-storage.sh -h
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/home/yungh/kafka/kafka_fork/kafka_test/core/build/dependant-libs-2.13.15/log4j-slf4j-impl-2.24.1.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/home/yungh/kafka/kafka_fork/kafka_test/tools/build/dependant-libs-2.13.15/log4j-slf4j-impl-2.24.1.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.apache.logging.slf4j.Log4jLoggerFactory]
usage: kafka-storage [-h] {info,format,version-mapping,feature-dependencies,random-uuid} ...
-- snip --

$ ./bin/kafka-metadata-shell.sh -h
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/home/yungh/kafka/kafka_fork/kafka/core/build/dependant-libs-2.13.15/log4j-slf4j-impl-2.24.1.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/home/yungh/kafka/kafka_fork/kafka/tools/build/dependant-libs-2.13.15/log4j-slf4j-impl-2.24.1.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.apache.logging.slf4j.Log4jLoggerFactory]
usage: kafka-metadata-shell [-h] [--snapshot SNAPSHOT] [command [command ...]]
-- snip --

Verification

After applying this patch:

$ ./bin/kafka-storage.sh -h
usage: kafka-storage [-h] {info,format,version-mapping,feature-dependencies,random-uuid} ...
-- snip --

$ ./bin/kafka-metadata-shell.sh -h
usage: kafka-metadata-shell [-h] [--snapshot SNAPSHOT] [command [command ...]]
-- snip --

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community build Gradle build or GitHub Actions small Small PRs labels Feb 16, 2025
Copy link
Contributor

@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I have validated the patch on my local machine:
Screenshot from 2025-02-16 15-47-13

@frankvicky
Copy link
Contributor

Hi @chia7712
Could you please take a look when you have a free moment?
Many thanks 🙇🏼

@Yunyung
Copy link
Contributor Author

Yunyung commented Feb 16, 2025

This should be backported to 4.0, as the same issue happens in 4.0.

Copy link
Contributor

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, left some questions.

When I run ./gradlew releaseTarGz, I do not see any Log4j warning messages. However, these messages appear when using these tools in the source code. Does this only happen in the development environment?

And why don't we exclude the Log4j dependency in the core module? It is defined in build.gradle at line 1137.

@frankvicky
Copy link
Contributor

Hi @m1a2st
You should see the warning of slf4j, but it is different from this one.

> Task :core:genProtocolErrorDocs
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.

The above warning is handled by KAFKA-18752

And why don't we exclude the Log4j dependency in the core module? It is defined in build.gradle at line 1137.

We could do that, but it is not necessary. The warning states that slf4j detects multiple bindings in projects, so removing one is enough. In this situation, it is easiest to exclude one in the tools module.

@m1a2st
Copy link
Contributor

m1a2st commented Feb 16, 2025

The above warning is handled by KAFKA-18752

I'm not referring to this warning. I tested the steps below, and no warning messages appeared.

./gradlew clean
./gradlew releaseTarGz
tar -zxvf $(find ./core/build/distributions/ -maxdepth 1 -type f \( -iname "kafka*tgz" ! -iname "*sit*" \)) -C /tmp/opt/kafka --strip-components=1
./bin/kafka-storage.sh -h
usage: kafka-storage [-h] {info,format,version-mapping,feature-dependencies,random-uuid} ...

@frankvicky
Copy link
Contributor

At the source code root, you need to complete the build or releaseTarGz at least first. This will generate a dependant-libs-2.13.15 directory.
Currently, only core and tools use releaseOnly. The copyDependantLibs tasks for these two modules will copy the releaseOnly dependencies to dependant-libs-2.13.15.
Therefore, when running tool scripts under the source code root, it will find two slf4-impl jars.
So this is indeed a dev-specific issue.

@github-actions github-actions bot removed the triage PRs from the community label Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Gradle build or GitHub Actions ci-approved small Small PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants