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

Allow using QueueHandler and QueueListener in logging.dictConfig #93162

Closed
spacemanspiff2007 opened this issue May 24, 2022 · 12 comments
Closed

Allow using QueueHandler and QueueListener in logging.dictConfig #93162

spacemanspiff2007 opened this issue May 24, 2022 · 12 comments
Assignees
Labels
stdlib type-feature

Comments

@spacemanspiff2007
Copy link

@spacemanspiff2007 spacemanspiff2007 commented May 24, 2022

Feature or enhancement

Allow using instances in the dict passed to dictConfig

Pitch

Currently the logging configuration does not allow passing already created instances of e.g. logging.Handler.
However this would be very useful for an application that uses the dict to configure the logger but wants to use a QueueHandler that implements custom logic.
Since the logging.Handlers and logging.Filters need to be attached to the QueueListener for logging to files it's almost impossible to set it up properly.
If it would be possible to pass an instance the setup could be simplified a lot.

Previous discussion

@vsajip
Copy link
Member

@vsajip vsajip commented May 24, 2022

You can already do this with minimal effort, as documented here:

In order to provide complete flexibility for user-defined object instantiation, the user needs to provide a ‘factory’ - a callable which is called with a configuration dictionary and which returns the instantiated object. This is signalled by an absolute import path to the factory being made available under the special key '()'.

So, just create a callable which sets things up as you want, and reference it in the configuration as shown in the example given in the posted link.

@spacemanspiff2007
Copy link
Author

@spacemanspiff2007 spacemanspiff2007 commented May 24, 2022

So, just create a callable which sets things up as you want, and reference it in the configuration as shown in the example given in the posted link.

You are right - I must have been confused when I created the PR. Sorry about that.

I want to use a queue to act as a buffer so I can write out the logs in a different thread. The log messages are created in bursts and the normal logging significantly slows down the application.

flowchart LR
    Logger --> QueueHandler
    QueueHandler -->|Buffer| QueueListener
    QueueListener --> FileHandler

But for this to work I need to be able to attach the logging handlers to the QueueListener which is currently not possible because
the handlers are created from the config file.

Imho there are two ways to solve this:

  1. Create all logging objects from the configuration. Then have another function on DictConfigurator which glues everything together. That way one could inherit from DictConfigurator and override the function. That way the Handlers could be attached to the user objects.
    That would be the most generic implementation and cover all use cases but a bigger code change.
  2. Add the possibility to define target handlers for every handler - just like with the MemoryHandler. However it would be nice if one could attach multiple handlers to one handler.

What do you think?

@vsajip
Copy link
Member

@vsajip vsajip commented May 24, 2022

What do you think?

It's hard to say with the information you've provided. One way of doing it would be to define your configuration dict as follows (shown as YAML for brevity):

...
handlers:
  ...
  queue_handler:
    (): my.package.factory
    handlers:
      file:  # dict with configuration for file handler
      console: # dict with configuration for console handler
      ... and any other handlers you need, likewise

Then, your factory will be called with a handlers kwarg which will be a dict containing all of the handler configuration data (using any schema you like). In the factory, you then:

  • Construct a queue
  • Construct all the handlers you need
  • Create a QueueListener configured with the queue and the handlers
  • Create a QueueHandler configured with the queue
  • Return that QueueHandler instance from the callable.

That assumes you need all the handler configuration in a configuration file. If not, you could make things simpler by just hardcoding it in the factory. Of course, you'll need to adapt this scheme to your specific needs.

This approach requires no changes to logging but should give you what you need. Of course you need to do a bit more work to instantiate the handlers etc. yourself, but that shouldn't be too onerous. This type of application is what the factory approach was meant for.

@vsajip vsajip closed this as completed May 24, 2022
@spacemanspiff2007
Copy link
Author

@spacemanspiff2007 spacemanspiff2007 commented May 24, 2022

That assumes you need all the handler configuration in a configuration file. If not, you could make things simpler by just hardcoding it in the factory. Of course, you'll need to adapt this scheme to your specific needs.

This approach requires no changes to logging but should give you what you need. Of course you need to do a bit more work to instantiate the handlers etc. yourself, but that shouldn't be too onerous. This type of application is what the factory approach was meant for.

If I had full control over the logging configuration it really would be that easy.
But it's an application where the users supply the logging configuration through the dict which then is loaded through dictConfig.
That would mean in my factory I would have to duplicate the complete code from logging.config.

Adding the possibility to chain handlers would effectively solve this issue because it would allow to inject the queue.

Example:

The user adds this configuration

handlers:
  ...
  user_file_handler:
    class: logging.handlers.TimedRotatingFileHandler
    ....

Which could be changed into

handlers:
  ...
  user_file_handler:
    (): QueueHandler
    target: queue_listener
    ...
  queue_listener:
    class: logging.handlers.TimedRotatingFileHandler
    ....

That would be a small change that would allow for the queue to work.
If you are unsure about the target key one could use handlers since this the the key from the loggers and less likely to be used.

Please reopen the issue, I am not the only one who is having issues with the QueueHandler.
It is included in logging.handlers so I really think there should be a supported way to use them with dictConfig.

@vsajip
Copy link
Member

@vsajip vsajip commented May 24, 2022

OK, but let me think a bit about the best way of solving this in a general way. Leave the PR closed, though, because I wouldn't be taking that approach.

@vsajip vsajip reopened this May 24, 2022
@AlexWaygood AlexWaygood added the stdlib label May 24, 2022
@spacemanspiff2007
Copy link
Author

@spacemanspiff2007 spacemanspiff2007 commented May 25, 2022

OK, but let me think a bit about the best way of solving this in a general way. Leave the PR closed, though, because I wouldn't be taking that approach.

No problem - better a well thought solution that takes longer than a quick and dirty fix.
Thank you for your help!

@spacemanspiff2007 spacemanspiff2007 changed the title Allow passing instances of logging objects to logging.dictConfig Allow using QueueHandler and QueueListener in logging.dictConfig May 25, 2022
@vsajip
Copy link
Member

@vsajip vsajip commented May 26, 2022

@spacemanspiff2007 I've got a first cut of a possible fix in #93269 - take a look and see if it will meet the requirements (from the documentation part - I'm not asking you to review the code and test in detail, though you're of course free to do so.

@spacemanspiff2007
Copy link
Author

@spacemanspiff2007 spacemanspiff2007 commented May 27, 2022

Thank you for the quick implementation. I'll take a deeper look at it in the mid of next week.

From what I understand your idea is to configure the listener and handler in one entry in the config dict.
I think it would be helpful to add the listener key in the example, e.g. with my.module.queuelistener_factory.

So from what I understand from the docs this will work:

listener_instance = MyQueueListener()

cfg = {
  'qhand': 'logging.handlers.QueueHandler',
  'queue': lambda: listener_instance.queue,
  'listener': listener_instance.set_handler_by_names
}

setConfig(cfg)

It's nice that it's possible to get handlers by name with getHandlerByName.
Would it make sense to provide this kind of mechanism for other logging objects e.g. Formatters and Filters?
Could we also add an option (or another function) to return all handlers?
After startup I want to iterate over all rotating file handlers and based on some internal criteria do a rotate on some of them.

@vsajip
Copy link
Member

@vsajip vsajip commented May 27, 2022

So from what I understand from the docs this will work:

No, that wouldn't work. I'll update the docs part to try and make things clearer.

It's nice that it's possible to get handlers by name

More than that, it's necessary to make this functionality usable - otherwise you couldn't get hold of the listener to start it.

Would it make sense to provide this kind of mechanism for other logging objects

Not really. Remember, every line of code becomes something to be maintained (mostly by me) into the far future. I don't want to add anything which isn't really necessary, but just nice-to-have once in a while. The principle should be that not everyone should have to "pay" for stuff that only a very few people use. If someone needs a very granular level of access over individual logging objects, they probably should configure logging programmatically. Remember, you have full access to everything in the API programmatically - so for more esoteric use cases, that's the way to go. For your example with the rotating handlers, you could just name them e.g. rh1, rh2 etc. and iterate over looking for matching handlers by name.

@spacemanspiff2007
Copy link
Author

@spacemanspiff2007 spacemanspiff2007 commented May 28, 2022

I'll update the docs part to try and make things clearer.

I think there is something missing. You write If the listener key is present, the corresponding value can be one of the following: but you add only one example. One of indicates that there are multiple choices but the docs only provide one.

No, that wouldn't work.

Is it because the callable can not be passed directly but must resolve?

listener_instance = MyQueueListener()

def configure_queue_instance(handlers_by_name):
    listener_instance.set_handler_by_names(handlers_by_name)
    return listener_instance

cfg = {
  'qhand': 'logging.handlers.QueueHandler',
  'queue': lambda: listener_instance.queue,
  'listener': 'my_module.configure_queue_instance'
}

setConfig(cfg)

How would one set maxsize for the Queue in an already existing application and how can a user pass additional args to the listener?
Wouldn't make it more sense to make the corresponding entries nested?

    handlers:
      qhand:
        class: logging.handlers.QueueHandler
        queue: 
          '()': my.module.queuefactory  # optional, but it would still be possible to specify maxsize
          'maxsize': 30
        listener: 
          '()': my.package.CustomListener
          'custom_arg': 30
        handlers:
          - hand_name_1
          - hand_name_2
          ...

For your example with the rotating handlers, you could just name them e.g. rh1, rh2 etc. and iterate over looking for matching handlers by name.

Configuration of logging is done by the user so I don't know about the handler names.
If it's possible to get a handler by name there should be at least a function that returns all handler names.

@vsajip
Copy link
Member

@vsajip vsajip commented May 29, 2022

I think there is something missing.

Good catch, there is supposed to be another entry in that list. I'll update.

Is it because the callable can not be passed directly but must resolve?

It's because you can't pass an instance, because the instance needs to be constructed with a queue. You need to pass a class which will be instantiated with a queue.

How would one set maxsize for the Queue in an already existing application and how can a user pass additional args to the listener?

I'm not sure a scenario where developer and user control a configuration is a scenario which is especially common, so I'm not really planning to support that specifically. However, I will give some more thought to how to make the configuration more generic for a developer.

@vsajip
Copy link
Member

@vsajip vsajip commented May 31, 2022

OK, I've now completed the changes I envisage making for this issue. If you have code-related comments, please add them on the PR rather than here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib type-feature
Projects
None yet
Development

No branches or pull requests

3 participants