Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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