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

Add support for subsystem logging.properties #4962

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

edewata
Copy link
Contributor

@edewata edewata commented Feb 20, 2025

The CMSEngine, ACMEEngine, and ESTEngine have been modified to support subsystem logging.properties. If the file exists, it can be used to configure the logging level in various libraries (e.g. RESTEasy) to help troubleshooting.

The CA, ACME, and EST tests have been modified to create the subsystem logging.properties then check the debug log.

https://github.com/dogtagpki/pki/wiki/Configuring-Subsystem-Debug-Log

If RESTEasy logging is enabled, it will show something like this:

2025-02-20 17:06:11 [main] INFO: RESTEASY002225: Deploying javax.ws.rs.core.Application: class org.dogtagpki.acme.server.ACMEApplication
2025-02-20 17:06:11 [main] INFO: RESTEASY002200: Adding class resource org.dogtagpki.acme.server.ACMELoginService from Application class org.dogtagpki.acme.server.ACMEApplication
2025-02-20 17:06:11 [main] INFO: RESTEASY002200: Adding class resource org.dogtagpki.acme.server.ACMELogoutService from Application class org.dogtagpki.acme.server.ACMEApplication
2025-02-20 17:06:11 [main] INFO: RESTEASY002200: Adding class resource org.dogtagpki.acme.server.ACMEEnableService from Application class org.dogtagpki.acme.server.ACMEApplication
...

The CMSEngine, ACMEEngine, and ESTEngine have been modified
to support subsystem logging.properties. If the file exists,
it can be used to configure the logging level in various
libraries (e.g. RESTEasy) to help troubleshooting.

The CA, ACME, and EST tests have been modified to create
the subsystem logging.properties then check the debug log.

https://github.com/dogtagpki/pki/wiki/Configuring-Subsystem-Debug-Log
@edewata edewata requested review from fmarco76 and jmagne February 20, 2025 17:20
Copy link
Member

@fmarco76 fmarco76 left a comment

Choose a reason for hiding this comment

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

For testing purpose I think it is as quicker as modify the logging.properties in the webapp.

If this is a was to allow different customisation in case of multiple instances running in the same server then we should provide a full customisation of the logger.

String value = properties.getProperty(key);

logger.info("- " + key + ": " + value);
if (!key.endsWith(".level")) continue;
Copy link
Member

Choose a reason for hiding this comment

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

Since we call the file logging.properties I am expecting that the provided values should overwrite the existing to modify the logs. So I would try to update the full configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I can try that. I wasn't sure whether the LogManager will override just the params specified in the custom logging.properties or replace the entire logging config with the custom logging.properties (meaning the handlers need to be redefined in the custom logging.properties).


logger.info("- " + key + ": " + value);
if (!key.endsWith(".level")) continue;

Copy link
Member

Choose a reason for hiding this comment

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

See above


logger.info("CMSEngine: - " + key + ": " + value);
if (!key.endsWith(".level")) continue;

Copy link
Member

Choose a reason for hiding this comment

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

See above

@edewata
Copy link
Contributor Author

edewata commented Feb 20, 2025

For testing purpose I think it is as quicker as modify the logging.properties in the webapp.

Yes, for testing we can just modify the default logging.properties but the changes are temporary (will be lost after upgrade).

If this is a was to allow different customisation in case of multiple instances running in the same server then we should provide a full customisation of the logger.

Even with a single instance, the custom logging.properties can be used to permanently customize the logging config (e.g. only log error messages) just like setting the debug.level in CS.cfg which won't be affected by upgrade. Let me try the LogManager as you suggested. Thanks!

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