Skip to content

Fix toString() method in configuration implementations #3599

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
ppkarwasz opened this issue Apr 10, 2025 · 3 comments
Open

Fix toString() method in configuration implementations #3599

ppkarwasz opened this issue Apr 10, 2025 · 3 comments
Labels
configuration Affects the configuration system in a general way enhancement Additions or updates to features

Comments

@ppkarwasz
Copy link
Contributor

Description

PR #2941 introduces some basic logging of the most important Configuration lifecycle events at an INFO level. The messages are meant to convey:

  • which configuration file is used.
  • the last modification timestamp of the file to distinguish between two revisions of the same file.

Unfortunately this information only appears in XmlConfiguration:

public String toString() {
return getClass().getSimpleName() + "[location=" + getConfigurationSource() + ", lastModified="
+ Instant.ofEpochMilli(getConfigurationSource().getLastModified()) + "]";
}

JsonConfiguration only prints the name of the configuration file, while PropertiesConfiguration does not have a toString() method at all:

2025-04-04T19:10:16.058147325Z pool-10-thread-1 INFO Stopping configuration org.apache.logging.log4j.core.config.properties.PropertiesConfiguration@18765d9b...
2025-04-04T19:10:16.058147313Z pool-9-thread-1 INFO Stopping configuration org.apache.logging.log4j.core.config.properties.PropertiesConfiguration@18f5f66d...
2025-04-04T19:10:16.058631896Z pool-10-thread-1 INFO Configuration org.apache.logging.log4j.core.config.properties.PropertiesConfiguration@18765d9b stopped.
2025-04-04T19:10:16.058880638Z pool-9-thread-1 INFO Configuration org.apache.logging.log4j.core.config.properties.PropertiesConfiguration@18f5f66d stopped.

We probably should add the toString() method to AbstractConfiguration and remove it from its derived classes.

@anindita-sarkarArray
Copy link

@ppkarwasz I have made the requested changes. #3658 Please review

@garydgregory
Copy link
Member

Why? This is not better since are now loosing information that was previously provided.

@anindita-sarkarArray
Copy link

anindita-sarkarArray commented May 11, 2025

hi @garydgregory , instead of AbstractConfiguration , shall I put my changes to all derived classes so that it can retain the additional info of subclasses.
OR
in addition to adding in AbstractConfiguration, I will modify the subclasses already having toString to have additional properties like [location=" + getConfigurationSource() + ", lastModified="
+ Instant.ofEpochMilli(getConfigurationSource().getLastModified())

@vy vy added enhancement Additions or updates to features configuration Affects the configuration system in a general way and removed waiting-for-maintainer labels May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Affects the configuration system in a general way enhancement Additions or updates to features
Projects
Status: To triage
Development

No branches or pull requests

4 participants