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

log: allow log handlers to be configured #688

Closed
oliver-sanders opened this issue Nov 29, 2021 · 6 comments · Fixed by #698
Closed

log: allow log handlers to be configured #688

oliver-sanders opened this issue Nov 29, 2021 · 6 comments · Fixed by #698

Comments

@oliver-sanders
Copy link
Contributor

oliver-sanders commented Nov 29, 2021

We can configure the log level and the log format, however, I don't think we can configure the log handlers?

There are cases where it would be helpful to be able to configure the application log to write to a file for reference as well as outputting to the default stream handler.

A simple solution would be to add a new trait for defining additional log handlers. A more complex solution could use a logging config to configure one or more handlers, potentially with different formats, etc. I don't think users would expect this to be reactive to config changes.

Has this been discussed before? Does this sound sensible?

@rmorshea
Copy link
Contributor

rmorshea commented Nov 29, 2021

I'm not super familiar with configurables in traitlets, but could we just make the LoggingConfigurable.log trait configurable instead? That would basically give you full control over log configuration without changing much.

Alternatively, you could just pass in Application(log=my_logger) when you construct the application, but maybe that's not sufficient in your case.

@oliver-sanders
Copy link
Contributor Author

I don't really want control over the log object, I want to define additional log handlers that traitlets can create/destroy for me with the same lifecycle as the default stream handler it provides.

My use case is writing the log output of Jupyter Servers to configured locations.

@rmorshea
Copy link
Contributor

By having control over the application's logger, you would therefore be able to configure handlers for that logger (or anything else about it for that matter).

If you'd like to be able to configure handlers directly, I'd be happy to take a contribution to that effect (probably a LoggingConfigurable.log_handlers list as you suggested), but that'd be a little more involved than just making the log trait configurable.

@oliver-sanders
Copy link
Contributor Author

Interested in making a contribution, scoping out opinions before I attempt it.

@rmorshea
Copy link
Contributor

rmorshea commented Nov 30, 2021

The main complication here is that you need to add the handlers to the logger after the logger trait has been assigned. The way to do that would be through an @observe method.

oliver-sanders added a commit to oliver-sanders/traitlets that referenced this issue Feb 8, 2022
* Closes ipython#688
* Allows fine configuration of logging via a `logging.config.dictConfig`.
* Changes the default log level from WARN to DEBUG and the default log
  handler level from undefined to WARN.
@oliver-sanders
Copy link
Contributor Author

oliver-sanders commented Feb 8, 2022

Taking a look at implementing, oliver-sanders@7dcea80 is most the way there. It's got an issue with a logging adapter test, needs better tests, docs, etc.

  • Unifies the pre-existing logging configurations into a standard "base" logging configuration dict.
  • Adds a new trait for providing a logging config dict which is merged into the "base" configuration allowing adding configs as well as redefining existing ones.
  • Changes the default logger level from WARN to DEBUG and the default handler level from undefined to WARN (I think the log_level should apply to the handler not the logger).

Need a hook for closing the logs, any ideas, __del__?

Does this approach look ok?

Tested with Jupyter Server, this allows configuring additional handlers (and overriding of the default one) for both the server and its extensions. Note every extension is a separate Application so must be configured separately:

c.ServerApp.logging_config = {
    'handlers': {
        'my_handler': {...}
    },
    'loggers': {
        'ServerApp': {
            handlers: ['console', 'my_handler']
        }
    }
}
c.ExtensionApp.logging_config = {
    'handlers': {
        'my_handler': {...}
    },
    'loggers': {
        'ExtensionApp': {
            handlers: ['console', 'my_handler']
        }
    }
}

oliver-sanders added a commit to oliver-sanders/traitlets that referenced this issue Apr 1, 2022
* Closes ipython#688
* Allows fine configuration of logging via a `logging.config.dictConfig`.
* Changes the default log level from WARN to DEBUG and the default log
  handler level from undefined to WARN.
oliver-sanders added a commit to oliver-sanders/traitlets that referenced this issue Apr 1, 2022
* Closes ipython#688
* Allows fine configuration of logging via a `logging.config.dictConfig`.
* Changes the default log level from WARN to DEBUG and the default log
  handler level from undefined to WARN.
oliver-sanders added a commit to oliver-sanders/traitlets that referenced this issue Apr 11, 2022
* Closes ipython#688
* Allows fine configuration of logging via a `logging.config.dictConfig`.
* Changes the default log level from WARN to DEBUG and the default log
  handler level from undefined to WARN.
oliver-sanders added a commit to oliver-sanders/traitlets that referenced this issue Apr 11, 2022
* Closes ipython#688
* Allows fine configuration of logging via a `logging.config.dictConfig`.
* Changes the default log level from WARN to DEBUG and the default log
  handler level from undefined to WARN.
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 a pull request may close this issue.

2 participants