From 5803f835f6fbf8c166af99cba1617cb0d3863210 Mon Sep 17 00:00:00 2001 From: Wojciech Malinowski Date: Thu, 6 Jun 2019 15:13:23 +0200 Subject: [PATCH 1/4] Added a possibility of logging the metrics to a Unix domain socket instead of UDP --- docs/source/settings.rst | 11 +++++++++++ gunicorn/config.py | 16 +++++++++++++++- gunicorn/instrument/statsd.py | 14 +++++++++++--- tests/test_config.py | 9 ++++++++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 16d8961ae..e759ded79 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -363,6 +363,17 @@ statsd_host .. versionadded:: 19.1 +.. _statsd-socket: + +statsd_socket +~~~~~~~~~~~~~ + +* ``--statsd-socket STATSD_SOCKET`` +* ``None`` + +Unix domain socket of the statsd server to log to. +Supersedes ``statsd_host`` if provided. + .. _dogstatsd-tags: dogstatsd_tags diff --git a/gunicorn/config.py b/gunicorn/config.py index e8e0f926a..3f48b6dd2 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -144,7 +144,9 @@ def logger_class(self): # 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: + statsd_address = 'statsd_socket' in self.settings and self.settings['statsd_socket'].value or \ + 'statsd_host' in self.settings and self.settings['statsd_host'].value + if statsd_address is not None: uri = "gunicorn.instrument.statsd.Statsd" logger_class = util.load_class( @@ -1478,6 +1480,18 @@ class StatsdHost(Setting): .. versionadded:: 19.1 """ +class StatsdSocket(Setting): + name = "statsd_socket" + section = "Logging" + cli = ["--statsd-socket"] + meta = "STATSD_SOCKET" + default = None + validator = validate_string + desc = """\ + Unix domain socket of the statsd server to log to. + Supersedes ``statsd_host`` if provided. + """ + # Datadog Statsd (dogstatsd) tags. https://docs.datadoghq.com/developers/dogstatsd/ class DogstatsdTags(Setting): name = "dogstatsd_tags" diff --git a/gunicorn/instrument/statsd.py b/gunicorn/instrument/statsd.py index 9a537205b..ee6d53086 100644 --- a/gunicorn/instrument/statsd.py +++ b/gunicorn/instrument/statsd.py @@ -27,10 +27,18 @@ def __init__(self, cfg): """ Logger.__init__(self, cfg) self.prefix = sub(r"^(.+[^.]+)\.*$", "\\g<1>.", cfg.statsd_prefix) - try: + + if cfg.statsd_socket: + address_family = socket.AF_UNIX + address = cfg.statsd_socket + elif cfg.statsd_host: + address_family = socket.AF_INET host, port = cfg.statsd_host - self.sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) - self.sock.connect((host, int(port))) + address = (host, int(port)) + + try: + self.sock = socket.socket(address_family, socket.SOCK_DGRAM) + self.sock.connect(address) except Exception: self.sock = None diff --git a/tests/test_config.py b/tests/test_config.py index 0587c63cf..db108b9b0 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -306,13 +306,20 @@ def nworkers_changed_3(server, new_value, old_value): assert c.nworkers_changed(1, 2, 3) == 3 -def test_statsd_changes_logger(): +def test_statsd_host_changes_logger(): c = config.Config() assert c.logger_class == glogging.Logger c.set('statsd_host', 'localhost:12345') assert c.logger_class == statsd.Statsd +def test_statsd_socket_changes_logger(): + c = config.Config() + assert c.logger_class == glogging.Logger + c.set('statsd_socket', '/var/run/sock') + assert c.logger_class == statsd.Statsd + + class MyLogger(glogging.Logger): # dummy custom logger class for testing pass From d1f6a7788c8416533d448cfe490957efabdb13a0 Mon Sep 17 00:00:00 2001 From: Wojciech Malinowski Date: Thu, 6 Jun 2019 15:22:14 +0200 Subject: [PATCH 2/4] Code style --- gunicorn/config.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gunicorn/config.py b/gunicorn/config.py index 3f48b6dd2..95b615d40 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -144,8 +144,10 @@ def logger_class(self): # if default logger is in use, and statsd is on, automagically switch # to the statsd logger if uri == LoggerClass.default: - statsd_address = 'statsd_socket' in self.settings and self.settings['statsd_socket'].value or \ - 'statsd_host' in self.settings and self.settings['statsd_host'].value + statsd_address = 'statsd_socket' in self.settings and \ + self.settings['statsd_socket'].value or \ + 'statsd_host' in self.settings and \ + self.settings['statsd_host'].value if statsd_address is not None: uri = "gunicorn.instrument.statsd.Statsd" From 15abac7e819174e6add9b3d979053f8666be297d Mon Sep 17 00:00:00 2001 From: larribas Date: Sun, 19 Jul 2020 19:45:50 +0200 Subject: [PATCH 3/4] Allow specifying a UDS socket address through --statsd-host --- docs/source/settings.rst | 16 +++++--------- gunicorn/config.py | 41 ++++++++++++++--------------------- gunicorn/instrument/statsd.py | 11 +++------- tests/test_config.py | 22 ++++++++++++------- tests/test_statsd.py | 12 ++++++++++ 5 files changed, 50 insertions(+), 52 deletions(-) diff --git a/docs/source/settings.rst b/docs/source/settings.rst index e759ded79..a3cb12dd1 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -359,20 +359,14 @@ statsd_host * ``--statsd-host STATSD_ADDR`` * ``None`` -``host:port`` of the statsd server to log to. +The address of the StatsD server to log to. -.. versionadded:: 19.1 - -.. _statsd-socket: - -statsd_socket -~~~~~~~~~~~~~ +Address is a string of the form: -* ``--statsd-socket STATSD_SOCKET`` -* ``None`` +* ``unix://PATH`` : for a unix domain socket. +* ``HOST:PORT`` : for a network address -Unix domain socket of the statsd server to log to. -Supersedes ``statsd_host`` if provided. +.. versionadded:: 19.1 .. _dogstatsd-tags: diff --git a/gunicorn/config.py b/gunicorn/config.py index 95b615d40..99b4ba319 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -144,11 +144,7 @@ def logger_class(self): # if default logger is in use, and statsd is on, automagically switch # to the statsd logger if uri == LoggerClass.default: - statsd_address = 'statsd_socket' in self.settings and \ - self.settings['statsd_socket'].value or \ - 'statsd_host' in self.settings and \ - self.settings['statsd_host'].value - if statsd_address is not None: + 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( @@ -499,15 +495,17 @@ def validate_chdir(val): return path -def validate_hostport(val): +def validate_address(val): val = validate_string(val) if val is None: return None - elements = val.split(":") - if len(elements) == 2: - return (elements[0], int(elements[1])) - else: - raise TypeError("Value must consist of: hostname:port") + + try: + address = util.parse_address(val, default_port='8125') + except RuntimeError: + raise TypeError("Value must be one of ('host:port', 'unix://PATH')") + + return address def validate_reload_engine(val): @@ -1475,23 +1473,16 @@ class StatsdHost(Setting): cli = ["--statsd-host"] meta = "STATSD_ADDR" default = None - validator = validate_hostport + validator = validate_address desc = """\ - ``host:port`` of the statsd server to log to. + The address of the StatsD server to log to. - .. versionadded:: 19.1 - """ + Address is a string of the form: -class StatsdSocket(Setting): - name = "statsd_socket" - section = "Logging" - cli = ["--statsd-socket"] - meta = "STATSD_SOCKET" - default = None - validator = validate_string - desc = """\ - Unix domain socket of the statsd server to log to. - Supersedes ``statsd_host`` if provided. + * ``unix://PATH`` : for a unix domain socket. + * ``HOST:PORT`` : for a network address + + .. versionadded:: 19.1 """ # Datadog Statsd (dogstatsd) tags. https://docs.datadoghq.com/developers/dogstatsd/ diff --git a/gunicorn/instrument/statsd.py b/gunicorn/instrument/statsd.py index ee6d53086..143d0bbc1 100644 --- a/gunicorn/instrument/statsd.py +++ b/gunicorn/instrument/statsd.py @@ -23,22 +23,17 @@ class Statsd(Logger): """statsD-based instrumentation, that passes as a logger """ def __init__(self, cfg): - """host, port: statsD server - """ Logger.__init__(self, cfg) self.prefix = sub(r"^(.+[^.]+)\.*$", "\\g<1>.", cfg.statsd_prefix) - if cfg.statsd_socket: + if isinstance(cfg.statsd_host, str): address_family = socket.AF_UNIX - address = cfg.statsd_socket - elif cfg.statsd_host: + else: address_family = socket.AF_INET - host, port = cfg.statsd_host - address = (host, int(port)) try: self.sock = socket.socket(address_family, socket.SOCK_DGRAM) - self.sock.connect(address) + self.sock.connect(cfg.statsd_host) except Exception: self.sock = None diff --git a/tests/test_config.py b/tests/test_config.py index db108b9b0..db33cb3f5 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -306,17 +306,23 @@ def nworkers_changed_3(server, new_value, old_value): assert c.nworkers_changed(1, 2, 3) == 3 -def test_statsd_host_changes_logger(): +def test_statsd_host(): c = config.Config() - assert c.logger_class == glogging.Logger - c.set('statsd_host', 'localhost:12345') - assert c.logger_class == statsd.Statsd - - -def test_statsd_socket_changes_logger(): + assert c.statsd_host is None + c.set("statsd_host", "localhost") + assert c.statsd_host == ("localhost", 8125) + c.set("statsd_host", "statsd:7777") + assert c.statsd_host == ("statsd", 7777) + c.set("statsd_host", "unix:/path/to.sock") + assert c.statsd_host == "/path/to.sock" + pytest.raises(TypeError, c.set, "statsd_host", 666) + pytest.raises(TypeError, c.set, "statsd_host", "host:string") + + +def test_statsd_changes_logger(): c = config.Config() assert c.logger_class == glogging.Logger - c.set('statsd_socket', '/var/run/sock') + c.set('statsd_host', 'localhost:12345') assert c.logger_class == statsd.Statsd diff --git a/tests/test_statsd.py b/tests/test_statsd.py index 06c1d9646..6f7bf426b 100644 --- a/tests/test_statsd.py +++ b/tests/test_statsd.py @@ -59,6 +59,18 @@ def test_statsd_fail(): logger.exception("No impact on logging") +def test_statsd_host_initialization(): + c = Config() + c.set('statsd_host', 'unix:test.sock') + logger = Statsd(c) + logger.info("Can be initialized and used with a UDS socket") + + # Can be initialized and used with a UDP address + c.set('statsd_host', 'host:8080') + logger = Statsd(c) + logger.info("Can be initialized and used with a UDP socket") + + def test_dogstatsd_tags(): c = Config() tags = 'yucatan,libertine:rhubarb' From 2a16fcd3ceed5a3fdd30a7b6fab0b065180576d6 Mon Sep 17 00:00:00 2001 From: larribas Date: Sun, 19 Jul 2020 20:40:08 +0200 Subject: [PATCH 4/4] Test and defend against the specific case where the statsd hostname is 'unix' --- gunicorn/config.py | 12 ++++++++++-- tests/test_config.py | 13 ++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/gunicorn/config.py b/gunicorn/config.py index 99b4ba319..5549a372a 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -495,11 +495,19 @@ def validate_chdir(val): return path -def validate_address(val): +def validate_statsd_address(val): val = validate_string(val) if val is None: return None + # As of major release 20, util.parse_address would recognize unix:PORT + # as a UDS address, breaking backwards compatibility. We defend against + # that regression here (this is also unit-tested). + # Feel free to remove in the next major release. + unix_hostname_regression = re.match(r'^unix:(\d+)$', val) + if unix_hostname_regression: + return ('unix', int(unix_hostname_regression.group(1))) + try: address = util.parse_address(val, default_port='8125') except RuntimeError: @@ -1473,7 +1481,7 @@ class StatsdHost(Setting): cli = ["--statsd-host"] meta = "STATSD_ADDR" default = None - validator = validate_address + validator = validate_statsd_address desc = """\ The address of the StatsD server to log to. diff --git a/tests/test_config.py b/tests/test_config.py index db33cb3f5..e3a3212bd 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -313,12 +313,23 @@ def test_statsd_host(): assert c.statsd_host == ("localhost", 8125) c.set("statsd_host", "statsd:7777") assert c.statsd_host == ("statsd", 7777) - c.set("statsd_host", "unix:/path/to.sock") + c.set("statsd_host", "unix:///path/to.sock") assert c.statsd_host == "/path/to.sock" pytest.raises(TypeError, c.set, "statsd_host", 666) pytest.raises(TypeError, c.set, "statsd_host", "host:string") +def test_statsd_host_with_unix_as_hostname(): + # This is a regression test for major release 20. After this release + # we should consider modifying the behavior of util.parse_address to + # simplify gunicorn's code + c = config.Config() + c.set("statsd_host", "unix:7777") + assert c.statsd_host == ("unix", 7777) + c.set("statsd_host", "unix://some.socket") + assert c.statsd_host == "some.socket" + + def test_statsd_changes_logger(): c = config.Config() assert c.logger_class == glogging.Logger