Skip to content

Comments

Add ability to configure logger based on ERS configuration safely#49

Draft
emmuhamm wants to merge 11 commits intodevelopfrom
emmuhamm/configure-ers-session
Draft

Add ability to configure logger based on ERS configuration safely#49
emmuhamm wants to merge 11 commits intodevelopfrom
emmuhamm/configure-ers-session

Conversation

@emmuhamm
Copy link
Contributor

@emmuhamm emmuhamm commented Feb 16, 2026

Description

Fixes (No existing issue)

Context

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:

  • We need to be able to add the ERS handlers automatically to the logging instance, based on what handlers are used in the entirity of ERS
  • There needs to be a way for the logger to remember what handlers it was initialised with, and only use those if no extra argument was used to choose which Handler to use
    • The problem is that adding handlers to the logger is easy, but by default a logger will use all available handlers when sending a message
    • When configuring ERS handlers, all these will be added to the logger. Any usual message will therefore be sent to all the handlers that exist, which is not what we want.
    • This should happen by default; we do not want to modify drunc and add in extra=HandlerType.Rich... in every message
    • There is functionality to filter on which handler to use on a per message level, and also functionality for the filter to default to a specific set of handlers when none are used
    • This needs to be further expanded on to deal with ERS

The changes here are meant to tackle the above.

Changes

Fallback Handlers

The concept of Fallback Handlers are introduced. The user experience is quite straight forward:

Test exceprt
    fallback_log: logging.Logger = get_daq_logger(
        logger_name="fallback_logger",
        log_level=log_level,
        stream_handlers=False,
        rich_handler=True, # Only rich has been added
    )

    # As rich has been added, only rich should be used
    fallback_log.info("Rich Only")
    
    # Adding a stdout handler as usual
    ### Importantly, by default the fallback_handler will use HandlerType.stdout (for this case)
    ### So that it will fire automatically by default if no extras are added
    add_stdout_handler(fallback_log, True)

    # Adding an stderr handler with a fallback_handler to Unknown
    ### This makes it so that if no extra exist, then the attached filter will default check if 
    ### stderr == unknown
    ### Basically, the code then does an equivalent of 
    ### log.warning("msg", extra={HandlerType.Unknown})
    ### Which obviously fails for the stderr case
    add_stderr_handler(fallback_log, True, {HandlerType.Unknown})


    ## Result is that both RICH and STDout pass, but stderr fails because it defaults to nonworking
    fallback_log.critical("Rich + stdout only")
    
    ## Ofcourse, the handler still exists.
    ## so if you manually call it using the extra feature, it will print
    fallback_log.critical(
        "Rich + stdout + stderr",
        extra={"handlers": [HandlerType.Rich, HandlerType.Stream]},
    )

All the details on how it works is in the annotated text above from a users perspective.

Technically speaking, the difference compared to before is found in replacing this line:

allowed = getattr(record, "handlers", LogHandlerConf.get_base())

And making it so we can modify it by hand.

And then we modify every single add_x_handler to change this value to whatever we want. And by default, it will set it to whatever the current handlertype is.

For example, add_rich_handler will make the default case a HandlerType.Rich.

Slight refactoring of code

To get through the next bit of change, theres a slight refactoring of the code.

The most important development of which is the new function add_handlers_from_types, which is a generic function that allows you to add whatever handler you want by submitting a (set of) HandlerTypes. This cleans up the get_daq_logger code decently well, while making it quite adjustable.

The bulk of the new code is based on having this handler_configs, which is a dict that holds the HandlerType, the type(Handler) as seen by python, and the installer code that it has. This can then be looped to install as necessary.

Something that requires a slight refactor is the code that checks if a logger has a handler or not. The code is now more general so that we can check if the current logger has it, or an ancestor. This is used several times in add_handlers_from_types.

With this, the new add_handlers_from_types will check the loggers to see if any of the logger or any of the ancestors have that handler. If it does, it will not add it.

Note, that while this is useful, it is now a bit bloated. See #51

Setup ERS Handlers

With the previous changes implemented, a new function called setup_daq_ers_logger has 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_types to 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.

Follow up

This PR, while useful, reveals how bloated the handlers.py file is, and how theres too much mixing of responsibilities with handlers, filters, and loggers. These should be refactored to prevent bloat and work on maintainability.

A description of suggested changes and suggested implementation has been added in #51. This will need to be tackled before any future developments can happen

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature (non-breaking change which adds functionality)
  • Optimization (non-breaking, back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Key checklist

  • All tests pass (eg. python -m pytest)
  • Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added or an issue has been opened to tackle that in the future.
    (Indicate issue here: # (issue))

@emmuhamm emmuhamm self-assigned this Feb 16, 2026
@emmuhamm emmuhamm changed the title Add ability to configure ers session Add ability to configure logger based on ERS configuration safely Feb 19, 2026
@emmuhamm emmuhamm force-pushed the emmuhamm/configure-ers-session branch from 713f4a1 to 30ce383 Compare February 19, 2026 10:10
@emmuhamm emmuhamm force-pushed the emmuhamm/configure-ers-session branch from 30ce383 to 67123b9 Compare February 19, 2026 10:14
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