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

Logging Interface/API #337

Open
Nintorac opened this issue Mar 22, 2022 · 3 comments
Open

Logging Interface/API #337

Nintorac opened this issue Mar 22, 2022 · 3 comments
Assignees

Comments

@Nintorac
Copy link
Contributor

Nintorac commented Mar 22, 2022

We will need to tie NVFlare into our logging infrastructure at some point. When that happens we will need for NVFlare to emit logs in JSON format in order for ingestion to be simple. I would propose adding a Logging interface that then lets users implement their own backends.

The interface should look something like this

class ILogger():

    @abstractmethod
    def _log(self, level, message, **kwargs):
        pass

    def info(self, message, **kwargs):
        self._log("info", message, **kwargs)

    def warn(self, message, **kwargs):
        self._log("warn", message, **kwargs)

    def error(self, message, **kwargs):
        self._log("error", message, **kwargs)

    def critical(self, message, **kwargs):
        self._log("critical", message, **kwargs)

Dagster does this fairly well, can look to their for inspiration - https://docs.dagster.io/concepts/logging/loggers

@Nintorac Nintorac changed the title Logging Interface/API and cleanup Logging Interface/API Mar 22, 2022
@Nintorac Nintorac reopened this Mar 22, 2022
@yanchengnv
Copy link
Collaborator

Yup, this aligns well with our direction of spec-based programming model. We'll add this item for future versions.

@taleinat
Copy link
Contributor

NVFlare seems to currently use the Python stdlib logging module. That supplies a similar interface to that described here, as well as extensive and well-documented ways to customize log formatting, routing, filtering and more.

I can't think of a good reason that NVFlare should implement its own way of logging. So I'd suggest:

  1. Make the NVFlare code stick to the convention of using logger names prefixed with their module or part of the system, always starting with nvflare.. This currently doesn't seem to be the case.
  2. Document how NVFlare's internal logging is done and examples of how to customize it.

With this, switching NVFlare and one's own model code to use structured JSON logging would be a straightforward matter of configuring logging to use an appropriate formatter, using the existing knowledge and tools for the logging module.

@Nintorac
Copy link
Contributor Author

Nintorac commented Mar 30, 2022

can't think of a good reason that NVFlare should implement its own way of logging

Hmm, yea I agree, it makes a lot of sense to lean on existing libraries for non-core functionality like this.

I still think it makes sense to bring a logging interface into the picture. Some of the benefits I believe this will bring:

  • Default logger configurations
    If there was a set of default loggers, then 99% of use cases are probably already covered. No need to mess around with confusing (IMO) python logging config files
  • Adding context
    The default logger class(es?) could wrap loggers in a LoggerAdapter to add valuable context to logs that users shouldn't need to worry about adding themselves. (hostname, run number, application name, application hash, executing user...etc).
  • Customization
    While standard logging should be used it could be useful for some users to provide a way to implement their own logging if they feel the need (eg a user may prefer loguru or the next flavour of the month logging framework)
  • Default handlers
    As a user it would be useful to have some way to forward logs to the calling process. I am imagining a use case where eg. You are in the exploration phase developing some applications on MRI's and the files are too large to transfer offsite. You want to be able to run your code from an offsite location, have it trigger jobs in the onsite location and stream the logs back to you.

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

No branches or pull requests

6 participants