Skip to content

fix toString() method in configuration implementations (apache#3599) #3658

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

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

anindita-sarkarArray
Copy link

@anindita-sarkarArray anindita-sarkarArray commented May 11, 2025

Added toString implementation to AbstractConfiguration and removed from all derived classes.

@garydgregory
Copy link
Member

Why? This makes debugging worse as we loosing information.

@anindita-sarkarArray
Copy link
Author

Why? This makes debugging worse as we loosing information.

hi @garydgregory , instead of AbstractConfiguration , shall I put my changes to all derived classes so that it can retain the additional info of subclasses.

Copy link

Job Requested goals Build Tool Version Build Outcome Build Scan®
build-macos-latest clean install 3.9.8 Build Scan PUBLISHED
build-ubuntu-latest clean install 3.9.8 Build Scan PUBLISHED
build-windows-latest clean install 3.9.8 Build Scan PUBLISHED
Generated by gradle/develocity-actions

@garydgregory
Copy link
Member

I am still -1 here: toString() is for debugging. If a call site needs a different view on the object, then it can call another method on that object or build that view (like a String) itself.

@ppkarwasz
Copy link
Contributor

Hi @anindita-sarkarArray,

I discussed this with @garydgregory, and I agree that the toString() methods should remain unchanged to preserve a better debugging experience within IDEs.

Following the discussion in #3100, I also think it's a good idea to reduce verbosity in Log4j Core by default. With that in mind, I propose the following:

  1. Downgrade the log level of the INFO messages I added in LoggerContext and AbstractConfiguration to DEBUG, with one exception:

  2. Add an INFO message in LoggerContext.setConfiguration(Configuration) to notify users when a new configuration becomes active (i.e., after the updateLoggers() call). The message could be formatted like:

    <logger_context_class_name>[name=<name>] is using <configuration_class_name>[location=<location>, lastModified=<last_modified>].
    

Let me know what you think.

@ppkarwasz ppkarwasz added waiting-for-user More information is needed from the user labels Jun 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-user More information is needed from the user
Projects
Status: To triage
Development

Successfully merging this pull request may close these issues.

3 participants