NVIDIA / NVFlare

NVIDIA Federated Learning Application Runtime Environment
https://nvidia.github.io/NVFlare/
Apache License 2.0
604 stars 171 forks source link

Logging Interface/API #337

Open Nintorac opened 2 years ago

Nintorac commented 2 years ago

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

yanchengnv commented 2 years ago

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

taleinat commented 2 years ago

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 commented 2 years ago

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: