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

Use Log4j2 instead of SLF4J for our own loggers #864

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

scholzj
Copy link
Member

@scholzj scholzj commented Jan 31, 2024

Right now, the bridge seems to be using SLF4J for logging. It seems to have also a mix of ways how the loggers are defined.

This PR moves it to use Log4j2 and unifies the way the loggers are created (as a static final variables called LOGGER). The only exception is the logging of the HTTP request that seems to not use logger based on the class but rather based on the endpoint - this behavior is kept. These changes bring the Bridge more in-line with Strimzi operators and should make it easier to manage and test the logging dependencies. It also allows it to remove SLF4J as a direct dependency (but it remains a transitive dependency for things such as Kafka clients). This does change only the API used for logging. The actual logging was already before done with Log4j2.

Signed-off-by: Jakub Scholz <www@scholzj.com>
@scholzj scholzj added this to the 0.28.0 milestone Jan 31, 2024
@scholzj scholzj requested a review from ppatierno January 31, 2024 10:13
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM. Assuming it's still logging :-)

@ppatierno ppatierno merged commit 6479490 into strimzi:main Jan 31, 2024
13 checks passed
@scholzj scholzj deleted the use-log4j2-for-our-own-loggers branch January 31, 2024 11:52
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