Use single NullHandler for whole library

I was trying to get a feel for what the rest of the python ecosystem
does with its logging, so I looked into the top few libraries on pypi:

urllib3 maintains a logger for not quite every module, but for each
one that does heavy lifting. The logger name is `__name__`, and no
handlers are registered for any module-level loggers, including
NullHandler. Their documentation spells out how to configure logging
for the library.

They explicitly configure a library root-level logger called `urllib3`
to which they attach a `NullHandler`. This addresses the "no handlers
could be found" problem.

Their tests explicitly configure handlers, just like ours do.

scrapy is more hands-on. It provides a configuration module for its
logging and a whole document on how to handle logging with scrapy. It
looks like log.py's whole reason for existence is making sure that a
handler is attached to to the scrapy handler at startup.

I think the extra complexity here is because scrapy also offers a CLI,
so there has to be some way to configure logging without resorting to
writing python, so I felt we didn't need to resort to this added
complexity.

---

Based on all of the libraries I've looked at, I think our current
approach is reasonable. The one change I would make is to explicitly
configure a `grpc` logger and to only attach a `NullHandler` to it
instead of putting the burden on the author of each new module to
configure it there.

With this change, I have

- Configured a logger in each module that cares about logging
- Removed all NullHandlers attached to module-level loggers
- Explicitly configured a `grpc` logger with a `NullHandler` attached

Resolves: #16572
Related: #17064
pull/17143/head
Richard Belleville 6 years ago
parent 01c6565fb5
commit ecd9063424
  1. 4
      src/python/grpcio/grpc/__init__.py
  2. 1
      src/python/grpcio/grpc/_channel.py
  3. 1
      src/python/grpcio/grpc/_common.py
  4. 1
      src/python/grpcio/grpc/_cython/_cygrpc/grpc_string.pyx.pxi
  5. 1
      src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi
  6. 1
      src/python/grpcio/grpc/_plugin_wrapping.py
  7. 1
      src/python/grpcio/grpc/framework/foundation/callable_util.py
  8. 1
      src/python/grpcio/grpc/framework/foundation/logging_pool.py
  9. 1
      src/python/grpcio/grpc/framework/foundation/stream_util.py
  10. 7
      src/python/grpcio_tests/tests/unit/_logging_test.py

@ -15,12 +15,14 @@
import abc import abc
import enum import enum
import logging
import sys import sys
import six import six
from grpc._cython import cygrpc as _cygrpc from grpc._cython import cygrpc as _cygrpc
logging.getLogger(__name__).addHandler(logging.NullHandler())
############################## Future Interface ############################### ############################## Future Interface ###############################

@ -25,7 +25,6 @@ from grpc._cython import cygrpc
from grpc.framework.foundation import callable_util from grpc.framework.foundation import callable_util
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
_USER_AGENT = 'grpc-python/{}'.format(_grpcio_metadata.__version__) _USER_AGENT = 'grpc-python/{}'.format(_grpcio_metadata.__version__)

@ -21,7 +21,6 @@ import grpc
from grpc._cython import cygrpc from grpc._cython import cygrpc
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
CYGRPC_CONNECTIVITY_STATE_TO_CHANNEL_CONNECTIVITY = { CYGRPC_CONNECTIVITY_STATE_TO_CHANNEL_CONNECTIVITY = {
cygrpc.ConnectivityState.idle: cygrpc.ConnectivityState.idle:

@ -15,7 +15,6 @@
import logging import logging
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
# This function will ascii encode unicode string inputs if neccesary. # This function will ascii encode unicode string inputs if neccesary.
# In Python3, unicode strings are the default str type. # In Python3, unicode strings are the default str type.

@ -19,7 +19,6 @@ import time
import grpc import grpc
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
cdef class Server: cdef class Server:

@ -21,7 +21,6 @@ from grpc import _common
from grpc._cython import cygrpc from grpc._cython import cygrpc
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
class _AuthMetadataContext( class _AuthMetadataContext(

@ -22,7 +22,6 @@ import logging
import six import six
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
class Outcome(six.with_metaclass(abc.ABCMeta)): class Outcome(six.with_metaclass(abc.ABCMeta)):

@ -18,7 +18,6 @@ import logging
from concurrent import futures from concurrent import futures
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
def _wrap(behavior): def _wrap(behavior):

@ -20,7 +20,6 @@ from grpc.framework.foundation import stream
_NO_VALUE = object() _NO_VALUE = object()
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
_LOGGER.addHandler(logging.NullHandler())
class TransformingConsumer(stream.Consumer): class TransformingConsumer(stream.Consumer):

@ -68,6 +68,13 @@ class LoggingTest(unittest.TestCase):
self.assertEqual(1, len(logging.getLogger().handlers)) self.assertEqual(1, len(logging.getLogger().handlers))
self.assertIs(logging.getLogger().handlers[0].stream, intended_stream) self.assertIs(logging.getLogger().handlers[0].stream, intended_stream)
@isolated_logging
def test_grpc_logger(self):
self.assertIn("grpc", logging.Logger.manager.loggerDict)
root_logger = logging.getLogger("grpc")
self.assertEqual(1, len(root_logger.handlers))
self.assertIsInstance(root_logger.handlers[0], logging.NullHandler)
if __name__ == '__main__': if __name__ == '__main__':
unittest.main(verbosity=2) unittest.main(verbosity=2)

Loading…
Cancel
Save