Open
Conversation
This was referenced Mar 2, 2026
This was referenced Mar 2, 2026
emmuhamm
commented
Mar 2, 2026
Comment on lines
+206
to
+209
| session_name : str, | ||
| topic : str = "ers_stream", | ||
| address : str = "monkafka.cern.ch:30092", | ||
| ers_app_name : str | None = None, |
Contributor
Author
There was a problem hiding this comment.
Avoid defaults as much as possible. Ideally, this should only be 'defaulted' at the get_daq_logger stage, and nowhere else.
Need to decide if we can remove defaults in ERSKafka as well. Preferrably yes, but im not sure if im gonna break anything as that might be used by other people already
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes DUNE-DAQ/drunc#776
Fixes #48
Fixes #51
This is a long time coming
Superseeds #53 #49
Context
For some more comprehensive set of logic ideas, please refer to the attached slides
ers_refactor.pdf
During testing of drunc, it was identified that we should be able to configure the handlers of a logging instance based on the ERS config supplied. This posed a greater question of how we deal with configuring the initialisation of handlers for loggers in general. Keep in mind that so far the features of the LogHandlerConf are entirely used for filtering; this is completely separate compared to how the logging was initialised.
A couple of key points are identified:
extraargument was used to choose which Handler to useextra=HandlerType.Rich...in every messageThe changes here are meant to tackle the above.
Changes
Fallback Handlers
The concept of Fallback Handlers are introduced. These are basically the 'default case' for what happens when log message is put in.
Basically, when a user writes in
log.info("msg"), the code will parse it aslog.info("msg", extra={"handlers": fallback_handlers}. This happens at the filtering stage of each handler, so at eachHandleIDFilterattached to every handler.By default, the fallback handler for each handler is set to be itself, thereby following the default behaviour. Eg. the HandleIDFilter attached to the Rich handler will have the fallback handler to HandlerType.Rich, which will allow every message to pass.
The user experience is quite straight forward:
Test exceprt
All the details on how it works is in the annotated text above from a users perspective.
By default, it will set it to whatever the current handlertype is.
For example,
add_rich_handlerwill make the default case aHandlerType.Rich.Setup ERS Handlers
A new function called
setup_daq_ers_loggerhas been implemented.With everything set up, this is a relatively simple function. All it does is to get the oks configuration, gets a set of all HandlerTypes, and then passess it to
add_handlers_from_typesto get it added to the loggers.Importantly, the fallback_handlers for this is set to
HandlerType.Unknown. This means that none of the new handlers added by the function will not be called unless specifically requested for!logging demonstrator
Of course, the logging demonstrator has been updated.
Change topic name in ERS
Oh yeah I also fixed the ability to choose which session name the ERS handler goes to now.
Major refactor of the code
See the previously linked set of slides for the description, but in general we have now new set of files:
The nicest new features are as follows:
add_handlersfunction, that will add a handler based on which handlertype you call it with.Better imports
With this change, I added a couple of the useful imports in the
__init__.pyfile. This makes things much easier for users of daqpytools since they can just dofrom daqpytools.logging import xrather than having to know what each of the files do and stuff.Type of change
Testing checklist
dbt-build --unittest)pytest -s minimal_system_quick_test.py)dunedaq_integtest_bundle.sh)python -m pytest)pre-commit run --all-files)To test:
daqpytools-logging-demonstrator -eh -hc -r -f log.log -ht -fh -s -ep emir-test -tor similarThese are vague ish instructions, but they should be relatively clear based on the messages in the demonstrator itself
Further checks
dbt-build --lint, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)(Indicate issue here: # (issue))