From 782b173e875a590bc13e84a4f993a32a5dffc1b8 Mon Sep 17 00:00:00 2001 From: Simon Davy Date: Fri, 22 Jan 2016 14:21:01 +0000 Subject: [PATCH 1/3] Allow configuring logger_class with statsd_host Currently if you configure statsd_host, a configured logger_class will never be used. I think this makes a user configured logger class always take priority. (This is PoC change, I will come back and add tests/docs if it's worth pursuing) --- gunicorn/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gunicorn/config.py b/gunicorn/config.py index 3b028e7f89..ff0a9a847d 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -147,7 +147,7 @@ def logger_class(self): uri = "gunicorn.glogging.Logger" # if statsd is on, automagically switch to the statsd logger - if 'statsd_host' in self.settings and self.settings['statsd_host'].value is not None: + if uri is not None and 'statsd_host' in self.settings and self.settings['statsd_host'].value is not None: logger_class = util.load_class("gunicorn.instrument.statsd.Statsd", section="gunicorn.loggers") else: From 96ecdd42b8883bde15b062ba936f4a31868984cd Mon Sep 17 00:00:00 2001 From: bloodearnest Date: Fri, 29 Jan 2016 10:28:00 +0000 Subject: [PATCH 2/3] Always use the the user configured logger class. Previously, configuring statsd_host would override the configured class --- docs/source/settings.rst | 7 +++++++ gunicorn/config.py | 22 ++++++++++++---------- tests/test_config.py | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 2a76aa1b79..439c63f831 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -532,6 +532,10 @@ The logger you want to use to log events in Gunicorn. The default class (``gunicorn.glogging.Logger``) handle most of normal usages in logging. It provides error and access logging. +If you enable statsd support, then a special subclass +(``gunicorn.instrument.statsd.Statsd``) is used instead, which handles +sending the metrics to *statsd_host* + You can provide your own worker by giving Gunicorn a Python path to a subclass like ``gunicorn.glogging.Logger``. Alternatively the syntax can also load the Logger class @@ -611,6 +615,9 @@ statsd_host ``host:port`` of the statsd server to log to. +Note: enabling this switches the default *logger_class* to +``gunicorn.instrument.statsd.Statsd`` + .. versionadded:: 19.1 statsd_prefix diff --git a/gunicorn/config.py b/gunicorn/config.py index ff0a9a847d..47bc09fcbc 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -144,16 +144,18 @@ def logger_class(self): uri = self.settings['logger_class'].get() if uri == "simple": # support the default - uri = "gunicorn.glogging.Logger" - - # if statsd is on, automagically switch to the statsd logger - if uri is not None and 'statsd_host' in self.settings and self.settings['statsd_host'].value is not None: - logger_class = util.load_class("gunicorn.instrument.statsd.Statsd", - section="gunicorn.loggers") - else: - logger_class = util.load_class(uri, - default="gunicorn.glogging.Logger", - section="gunicorn.loggers") + uri = LoggerClass.default + + # if default logger is in use, and statsd is on, automagically switch + # to the statsd logger + if uri == LoggerClass.default: + if 'statsd_host' in self.settings and self.settings['statsd_host'].value is not None: + uri = "gunicorn.instrument.statsd.Statsd" + + logger_class = util.load_class( + uri, + default="gunicorn.glogging.Logger", + section="gunicorn.loggers") if hasattr(logger_class, "install"): logger_class.install() diff --git a/tests/test_config.py b/tests/test_config.py index f235564ed5..9f9a2fe977 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -13,6 +13,8 @@ from gunicorn import config from gunicorn.app.base import Application from gunicorn.workers.sync import SyncWorker +from gunicorn import glogging +from gunicorn.instrument import statsd dirname = os.path.dirname(__file__) def cfg_module(): @@ -57,6 +59,9 @@ def test_property_access(): # Class was loaded assert c.worker_class == SyncWorker + # logger class was loaded + assert c.logger_class == glogging.Logger + # Workers defaults to 1 assert c.workers == 1 c.set("workers", 3) @@ -259,3 +264,24 @@ def nworkers_changed_3(server, new_value, old_value): c.set("nworkers_changed", nworkers_changed_3) assert c.nworkers_changed(1, 2, 3) == 3 + + +def test_statsd_changes_logger(): + c = config.Config() + assert c.logger_class == glogging.Logger + c.set('statsd_host', 'localhost:12345') + assert c.logger_class == statsd.Statsd + + +class MyLogger(glogging.Logger): + # dummy custom logger class for testing + pass + + +def test_always_use_configured_logger(): + c = config.Config() + c.set('logger_class', __name__ + '.MyLogger') + assert c.logger_class == MyLogger + c.set('statsd_host', 'localhost:12345') + # still uses custom logger over statsd + assert c.logger_class == MyLogger From 1359b3de3abfcba50f4a0bad5b75628f29ad13a1 Mon Sep 17 00:00:00 2001 From: bloodearnest Date: Fri, 12 Feb 2016 11:42:41 +0000 Subject: [PATCH 3/3] Update docs - more explicit info on logger_class --- docs/source/settings.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 439c63f831..133bda35e5 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -616,7 +616,10 @@ statsd_host ``host:port`` of the statsd server to log to. Note: enabling this switches the default *logger_class* to -``gunicorn.instrument.statsd.Statsd`` +``gunicorn.instrument.statsd.Statsd``. If you wish to use statsd with +a custom *logger_class*, you should make sure your class is API +compatible with ``gunicorn.instrument.statsd.Statsd``, or inherit from +it. .. versionadded:: 19.1