Switch statsd config to zuul.conf

The automatic statsd configuration based on env variables has
proven cumbersome and counter-intuitive.  Move its configuration
into zuul.conf in preparation for other components emitting stats.

Change-Id: I3f6b5010d31c05e295f3d70925cac8460d334283

doc/source/admin/components.rst

@@ -101,6 +101,27 @@ The following sections of ``zuul.conf`` are used by all Zuul components:
 
       An openssl file containing the client private key in PEM format.
 
+.. attr:: statsd
+
+   Information about the optional statsd server.  If the ``statsd``
+   python module is installed and this section is configured,
+   statistics will be reported to statsd.  See :ref:`statsd` for more
+   information.
+
+   .. attr:: server
+
+      Hostname or IP address of the statsd server.
+
+   .. attr:: port
+      :default: 8125
+
+      The UDP port on which the statsd server is listening.
+
+   .. attr:: prefix
+
+      If present, this will be prefixed to all of the keys before
+      transmitting to the statsd server.
+
 .. NOTE: this is a white lie at this point, since only the scheduler
    uses this, however, we expect other components to use it later, so
    it's reasonable for admins to plan for this now.

doc/source/admin/monitoring.rst

@@ -3,6 +3,8 @@
 Monitoring
 ==========
 
+.. _statsd:
+
 Statsd reporting
 ----------------
 
@@ -13,20 +15,11 @@ which let you in turn generate nice graphics.
 Configuration
 ~~~~~~~~~~~~~
 
-Statsd support uses the statsd python module. Note that Zuul will start without
-the statsd python module, so an existing Zuul installation may be missing it.
+Statsd support uses the ``statsd`` python module.  Note that support
+is optional and Zuul will start without the statsd python module
+present.
 
-The configuration is done via environment variables STATSD_HOST and
-STATSD_PORT. They are interpreted by the statsd module directly and there is no
-such parameter in zuul.conf yet. Your init script will have to initialize both
-of them before executing Zuul.
-
-Your init script most probably loads a configuration file named
-``/etc/default/zuul`` which would contain the environment variables::
-
-  $ cat /etc/default/zuul
-  STATSD_HOST=10.0.0.1
-  STATSD_PORT=8125
+Configuration is in the :attr:`statsd` section of ``zuul.conf``.
 
 Metrics
 ~~~~~~~

etc/zuul.conf-sample

@@ -5,6 +5,9 @@ server=127.0.0.1
 ;ssl_cert=/path/to/client.pem
 ;ssl_key=/path/to/client.key
 
+[statsd]
+server=127.0.0.1
+
 [zookeeper]
 hosts=127.0.0.1:2181
 

tests/base.py

@@ -20,7 +20,6 @@ from contextlib import contextmanager
 import datetime
 import gc
 import hashlib
-import importlib
 from io import StringIO
 import json
 import logging
@@ -48,7 +47,6 @@ import fixtures
 import kazoo.client
 import kazoo.exceptions
 import pymysql
-import statsd
 import testtools
 import testtools.content
 import testtools.content_type
@@ -2047,14 +2045,9 @@ class ZuulTestCase(BaseTestCase):
         self.config.set('executor', 'state_dir', self.executor_state_root)
 
         self.statsd = FakeStatsd()
-        # note, use 127.0.0.1 rather than localhost to avoid getting ipv6
-        # see: https://github.com/jsocol/pystatsd/issues/61
-        os.environ['STATSD_HOST'] = '127.0.0.1'
-        os.environ['STATSD_PORT'] = str(self.statsd.port)
+        if self.config.has_section('statsd'):
+            self.config.set('statsd', 'port', str(self.statsd.port))
         self.statsd.start()
-        # the statsd client object is configured in the statsd module import
-        importlib.reload(statsd)
-        importlib.reload(zuul.scheduler)
 
         self.gearman_server = FakeGearmanServer(self.use_ssl)
 

tests/fixtures/zuul.conf

@@ -1,6 +1,11 @@
 [gearman]
 server=127.0.0.1
 
+[statsd]
+# note, use 127.0.0.1 rather than localhost to avoid getting ipv6
+# see: https://github.com/jsocol/pystatsd/issues/61
+server=127.0.0.1
+
 [scheduler]
 tenant_config=main.yaml
 

tests/unit/test_scheduler.py

@@ -2152,8 +2152,7 @@ class TestScheduler(ZuulTestCase):
 
     def test_statsd(self):
         "Test each of the statsd methods used in the scheduler"
-        import extras
-        statsd = extras.try_import('statsd.statsd')
+        statsd = self.sched.statsd
         statsd.incr('test-incr')
         statsd.timing('test-timing', 3)
         statsd.gauge('test-gauge', 12)

tox.ini

@@ -5,10 +5,7 @@ envlist = pep8,py35
 
 [testenv]
 basepython = python3
-# Set STATSD env variables so that statsd code paths are tested.
-setenv = STATSD_HOST=127.0.0.1
-         STATSD_PORT=8125
-         VIRTUAL_ENV={envdir}
+setenv = VIRTUAL_ENV={envdir}
          OS_TEST_TIMEOUT=120
 passenv = ZUUL_TEST_ROOT OS_STDOUT_CAPTURE OS_STDERR_CAPTURE OS_LOG_CAPTURE OS_LOG_DEFAULTS
 usedevelop = True

zuul/cmd/scheduler.py

@@ -29,6 +29,7 @@ import signal
 
 import zuul.cmd
 from zuul.lib.config import get_default
+from zuul.lib.statsd import get_statsd_config
 
 # No zuul imports here because they pull in paramiko which must not be
 # imported until after the daemonization.
@@ -97,8 +98,14 @@ class Scheduler(zuul.cmd.ZuulApp):
             os.close(pipe_write)
             self.setup_logging('gearman_server', 'log_config')
             import zuul.lib.gearserver
-            statsd_host = os.environ.get('STATSD_HOST')
-            statsd_port = int(os.environ.get('STATSD_PORT', 8125))
+
+            (statsd_host, statsd_port, statsd_prefix) = get_statsd_config(
+                self.config)
+            if statsd_prefix:
+                statsd_prefix += '.zuul.geard'
+            else:
+                statsd_prefix = 'zuul.geard'
+
             host = get_default(self.config, 'gearman_server', 'listen_address')
             port = int(get_default(self.config, 'gearman_server', 'port',
                                    4730))
@@ -112,7 +119,7 @@ class Scheduler(zuul.cmd.ZuulApp):
                                            host=host,
                                            statsd_host=statsd_host,
                                            statsd_port=statsd_port,
-                                           statsd_prefix='zuul.geard')
+                                           statsd_prefix=statsd_prefix)
 
             # Keep running until the parent dies:
             pipe_read = os.fdopen(pipe_read)

zuul/connection/__init__.py

@@ -14,8 +14,6 @@
 
 import abc
 
-import extras
-
 
 class BaseConnection(object, metaclass=abc.ABCMeta):
     """Base class for connections.
@@ -42,7 +40,6 @@ class BaseConnection(object, metaclass=abc.ABCMeta):
         self.driver = driver
         self.connection_name = connection_name
         self.connection_config = connection_config
-        self.statsd = extras.try_import('statsd.statsd')
 
     def logEvent(self, event):
         self.log.debug(
@@ -50,11 +47,11 @@ class BaseConnection(object, metaclass=abc.ABCMeta):
                 connection=self.connection_name,
                 event=event))
         try:
-            if self.statsd:
-                self.statsd.incr(
+            if self.sched.statsd:
+                self.sched.statsd.incr(
                     'zuul.event.{driver}.{event}'.format(
                         driver=self.driver.name, event=event.type))
-                self.statsd.incr(
+                self.sched.statsd.incr(
                     'zuul.event.{driver}.{connection}.{event}'.format(
                         driver=self.driver.name,
                         connection=self.connection_name,

zuul/lib/statsd.py

@@ -0,0 +1,33 @@
+# Copyright 2017 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import extras
+from zuul.lib.config import get_default
+
+
+def get_statsd_config(config):
+    statsd_host = get_default(config, 'statsd', 'server')
+    statsd_port = int(get_default(config, 'statsd', 'port', 8125))
+    statsd_prefix = get_default(config, 'statsd', 'prefix')
+    return (statsd_host, statsd_port, statsd_prefix)
+
+
+def get_statsd(config):
+    statsd = extras.try_import('statsd')
+    if statsd is None:
+        return None
+    (statsd_host, statsd_port, statsd_prefix) = get_statsd_config(config)
+    if statsd_host is None:
+        return None
+    return statsd.StatsClient(statsd_host, statsd_port, statsd_prefix)

zuul/scheduler.py

@@ -15,7 +15,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import extras
 import json
 import logging
 import os
@@ -31,6 +30,7 @@ from zuul import model
 from zuul import exceptions
 from zuul import version as zuul_version
 from zuul.lib.config import get_default
+from zuul.lib.statsd import get_statsd
 
 
 class ManagementEvent(object):
@@ -211,7 +211,7 @@ class Scheduler(threading.Thread):
         self.executor = None
         self.merger = None
         self.connections = None
-        self.statsd = extras.try_import('statsd.statsd')
+        self.statsd = get_statsd(config)
         # TODO(jeblair): fix this
         # Despite triggers being part of the pipeline, there is one trigger set
         # per scheduler. The pipeline handles the trigger filters but since