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

Refactor DefaultLoggingRuleConfigurationBuilder to reduce usage of logback-classic #32418

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

linghengqian
Copy link
Member

@linghengqian linghengqian commented Aug 6, 2024

Fixes #32377.

Changes proposed in this pull request:

  • Refactor DefaultLoggingRuleConfigurationBuilder to reduce usage of logback-classic. Apparently the adapter for org.slf4j.impl.StaticLoggerBinder is not in logback-core. It looks like there is no other way. Keep logback-classic in the classpath of shardingsphere jdbc.
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/home/linghengqian/.m2/repository/org/apache/logging/log4j/log4j-slf4j-impl/2.18.0/log4j-slf4j-impl-2.18.0.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/home/linghengqian/.m2/repository/org/slf4j/slf4j-log4j12/1.7.25/slf4j-log4j12-1.7.25.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/home/linghengqian/.m2/repository/ch/qos/logback/logback-classic/1.2.13/logback-classic-1.2.13.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]

Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request.
  • I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes.

@linghengqian linghengqian marked this pull request as ready for review August 6, 2024 15:54
@RaigorJiang
Copy link
Contributor

@linghengqian
The logging rule is actually not finished. It should be like the xa transaction engine, allowing users to choose which logging component to use through SPI.

My idea is to remove the logging rule from the dependency tree, redesign it, and then add it in. Do you think it's okay?

@linghengqian
Copy link
Member Author

@linghengqian
The logging rule is actually not finished. It should be like the xa transaction engine, allowing users to choose which logging component to use through SPI.

My idea is to remove the logging rule from the dependency tree, redesign it, and then add it in. Do you think it's okay?

  • I noticed that there is a logging root node in the shardingsphere jdbc YAML node, but it looks like the dependencies are very deep. Maybe the original author of the corresponding function can help remove it? I can't find the corresponding documentation.

@RaigorJiang
Copy link
Contributor

@linghengqian
The logging rule is actually not finished. It should be like the xa transaction engine, allowing users to choose which logging component to use through SPI.
My idea is to remove the logging rule from the dependency tree, redesign it, and then add it in. Do you think it's okay?

  • I noticed that there is a logging root node in the shardingsphere jdbc YAML node, but it looks like the dependencies are very deep. Maybe the original author of the corresponding function can help remove it? I can't find the corresponding documentation.

Yes, I will try to communicate with the author.

@RaigorJiang RaigorJiang merged commit 4216911 into apache:master Aug 7, 2024
141 checks passed
@linghengqian linghengqian added this to the 5.5.1 milestone Aug 7, 2024
@linghengqian linghengqian deleted the remove-slf4j branch August 7, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shardingsphere-logging-core should not set the scope of logback-classic to compile
2 participants