Skip to content

Conversation

@emmuhamm
Copy link
Contributor

@emmuhamm emmuhamm commented Dec 2, 2025

Description

This pull request aims to unify the consistency of the loggers within drunc.

This is taking advantage of the inheritance, handlers, and the other nice features introduced in daqpytools.

See issue #691

Logger definitions and conventions

  • The drunc logger acts as the 'root' logger and will contain no handlers. This logger is what defines most of the logger and handler severity levels.
  • The parent handlers, drunc.[parent], (also known as the '[parent]' logger, dropping the drunc prefix) will be responsible for having all the relevant handlers. By default, they will contain rich loggers
  • The utils logger is initialised in the __init__.py file of the utils module, as utils is used in several places. It is initialised with the rich handler
  • Other smaller apps (eg ssh_doctor) are defined at the top of their initial files.
  • Process manager will by default have a rich handler. When booting the process manager, a file handler is added to the process manager logger. This will store all outputs of anything within process manager and all its dependents
    • Important: The previous behaviour of the log file log_emmuhamm_SSH_SHELL_process_manager.txt is that it saves all outputs the drunc unified shell in here. This PR changes it so that this file only saves anything within the 'process_manager' log handler, significantly reducing the amount of lines in that file
  • The controller logger will not have any loggers, but it will be split into two descendant loggers: core and interface
    • core (controller.core) will have stream handlers, the output of which will be piped into a log file
    • interface (controller.interface) will have rich handlers, as this is whats going to be seen in the terminal

Type of change

  • Optimization (non-breaking change that improves code/performance)

Testing checklist

  • Unit tests pass (e.g. dbt-build --unittest)
  • Minimal system quicktest passes (pytest -s minimal_system_quick_test.py)
  • Full set of integration tests pass (daqsystemtest_integtest_bundle.sh)
  • Python tests pass if applicable (e.g. python -m pytest)
  • Pre-commit hooks run successfully if applicable (e.g. pre-commit run --all-files)

Testing instructions for the reviewer

Tested on NFD_DEV_251204_A9.

  • Check out NFD_DEV_251204_A9, and do the usual setup
  • Make a new folder, eg test_run_develop, and cd into it
  • Run:
drunc-unified-shell -l DEBUG ssh-standalone config/daqsystemtest/example-configs.data.xml local-1x1-config emir-test |& tee -a >(sed -u 's/\x1b\[[0-9;]*m//g' >> "out.log")

### in the drunc shell
start-run --run-number 1
wait 20
exit
  • check out the drunc repository on this branch

  • Make a new folder somewhere, eg test_run_changes and cd into it

  • Repeat the shell commands as above

  • Compare differences between the log files in the two folders. there will be differences, but they should follow as what is described above

It might be useful to strip the date and time in the log files to better compare the two files. The script below can help

Run the below script to remove dates and times ``` #!/bin/bash

logfile="$1"
outfile="$(dirname "$logfile")/uni_$(basename "$logfile")"

sed -E 's/[[0-9]{4}/[0-9]{2}/[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2} UTC]/[XXXX/XX/XX XX:XX:XX UTC]/g' "$logfile" > "$outfile"


<\details>
 

## Further checks

- [x] Code is commented where needed, particularly in hard-to-understand areas
- [x] Code style is correct (`dbt-build --lint`, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)
- [x] If applicable, new tests have been added or an issue has been opened to tackle that in the future.
  (Indicate issue here: # (issue))


## TODO:
- [ ] Update wiki 

@emmuhamm emmuhamm changed the base branch from develop to emmuhamm/use-daqpytools-logger-new December 2, 2025 09:04
Base automatically changed from emmuhamm/use-daqpytools-logger-new to develop December 3, 2025 17:15
@emmuhamm emmuhamm force-pushed the emmuhamm/update-loggers branch from 2f12788 to 5db142d Compare December 4, 2025 10:53
@emmuhamm emmuhamm self-assigned this Dec 4, 2025
@emmuhamm emmuhamm changed the title Update loggers (to be optimised) Unify the logger conventions in drunc Dec 5, 2025
@emmuhamm
Copy link
Contributor Author

emmuhamm commented Dec 5, 2025

Specific logger to review

The current set up as is will basically have the same log outputs as what is in develop (except log_emmuhamm_SSH_SHELL_process_manager.txt). But its worth checking if we want to change anything. These are marked with the # TODO: Verify core/interface choice

@emmuhamm emmuhamm marked this pull request as ready for review December 5, 2025 10:43
Copy link
Collaborator

@PawelPlesniak PawelPlesniak left a comment

Choose a reason for hiding this comment

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

Some corrections to do

@emmuhamm
Copy link
Contributor Author

emmuhamm commented Dec 8, 2025

Comments have been addressed, but after chatting offline it results in two commits which I want your review on.

  • 0e1a9f3 adds the hacky core fsm logger to set to critical so we dont see any logs in the tty for the unified shell fsm

  • When I tested setting the controller log level to debug instead of init, I see that there was a lot of debug-level messages from other apps (eg drunc.utils) that gets piped into the log output as well. Problem is, these are usually Rich text with ANSI characters. 78075b2 introduces a patch to clear the ANSI characters

Note that both issues would be solved when we eventually move away from piping outputs from terminal to using actual file handlers

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.

3 participants