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

Pawel plesniak/logger inheritance #322

Draft
wants to merge 29 commits into
base: develop
Choose a base branch
from

Conversation

PawelPlesniak
Copy link
Contributor

@PawelPlesniak PawelPlesniak commented Dec 6, 2024

Upgrading the logging inheritance, in preparation for the ERS upgrade.

Requirements

  1. Need a hierarchical structure for the loggers to correctly inherit handlers. Define the psuedo-root controller drunc, with associated children loggers drunc.process_manager etc. (pseudo as the true root logger is created by logging.getlogger(), but in the scope of drunc this will be the new root logger).
  2. Need to use keep the logger output for the process_manager and the controllers, but not any of the shells.
  3. Handle all of the exception handling through loggers, remove print_traceback.
  4. Remove the controller RichHandler as it is not needed.
  5. Need to move away from default logging.getLogger as this can cause problems if the logging root logger is initialized.
  6. Need to define a scope for choosing which handlers are used on a per-logger basis, providing the basic platform for extending this with ers.

Summary of changes made

  1. Defined a function setup_root_logger that creates the drunc logger
  2. Defined a function get_logger that changes the logging name to drunc.<log_name>. This will treat drunc as the pseudo-root logger. This will easily be extended for ers when it is ready.
  3. Defined a logging hierarchy such that
    • drunc is the root logger
    • There are separate parent loggers for the process_manager, controller, and utils with the appropriate handlers
    • When new loggers are redefined, theyw ill not override the handlers
    • Child loggers will either have a RichHandler or a StreamHandler
  4. logging.basicConfig default loggers removed, this will force us to be stricter with the choice of handlers. This addresses the issue of no controller RichHandlers. console printing has been left as this is information that will remain in the termina output at all times without information being commited to any logs.
  5. Changed all instances of self.logger and self._log to self.log
  6. Added .nfs* and info.*.json to .gitignore. We don't want these files in the repo anyway
  7. Created a root logger with level logging.ERROR when spawning the unified_shell to ensure that there is always a drunc logger before any logging is set. The level is overridden in setup_root_logger if the level is logging.ERROR

Notes

  1. There is a process_manager logger instantiated with the unified_shell, this is needed as the process_manager_driver does not see the logger created by the process-manager itself (I suspect this is because it is run in a separate mp.process and does not register with the unified_shell process)

Still requires

Validating the parent logger selection, checking if more are needed. Still reviewing the FSM sepcific ones.
Add additional parent
Batch unit test

Extensions for separate PRs

Want ERS to be handled through logging, need to discuss this with Marco. This would be done as per here.

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.

1 participant