Remove deprecated statsd label_mode

Hopefully if we never do a release that supports signalfx no one will
ever use it and we won't have to maintain it.

Drive-by: refactor label model dispatch to fix a weird bug where a
config name could be a class attribute and blow up weird.

Change-Id: I2c67b59820c5ca094077bf47628426f4b0445ba0

doc/source/metrics/labels.rst

@@ -21,7 +21,6 @@ parsed by `statsd_exporter <https://github.com/prometheus/statsd_exporter/>`__:
 - ``librato``
 - ``influxdb``
 - ``dogstatsd``
-- ``signalfx``
 - ``graphite``
 
 See the ``proxy-server.conf-sample`` file for more information on configuring

etc/proxy-server.conf-sample

@@ -71,7 +71,7 @@ bind_port = 8080
 #
 # Statsd metrics may include labeling information in a variety of formats.
 # Available options:
-#     disabled, dogstatsd, graphite, influxdb, librato, signalfx.
+#     disabled, dogstatsd, graphite, influxdb, librato.
 # Defaults to disabled; enable statsd_label_mode by setting another option.
 # See also: https://github.com/prometheus/statsd_exporter#tagging-extensions.
 # Note that enabling statsd_label_mode will likely increase the number of time

swift/common/statsd_client.py

@@ -39,49 +39,45 @@ def _build_line_parts(metric, value, metric_type, sample_rate):
     return line
 
 
-class LabeledFormats:
+def librato(metric, value, metric_type, sample_rate, labels):
-    disabled = None
+    # https://www.librato.com/docs/kb/collect/collection_agents/stastd/#stat-level-tags
+    if labels:
+        metric += '#' + ','.join('%s=%s' % (k, v) for k, v in labels)
+    line = _build_line_parts(metric, value, metric_type, sample_rate)
+    return line
 
-    @staticmethod
-    def librato(metric, value, metric_type, sample_rate, labels):
-        # https://www.librato.com/docs/kb/collect/collection_agents/stastd/#stat-level-tags
-        if labels:
-            metric += '#' + ','.join('%s=%s' % (k, v) for k, v in labels)
-        line = _build_line_parts(metric, value, metric_type, sample_rate)
-        return line
 
-    @staticmethod
+def influxdb(metric, value, metric_type, sample_rate, labels):
-    def influxdb(metric, value, metric_type, sample_rate, labels):
+    # https://www.influxdata.com/blog/getting-started-with-sending-statsd-metrics-to-telegraf-influxdb/#introducing-influx-statsd
-        # https://www.influxdata.com/blog/getting-started-with-sending-statsd-metrics-to-telegraf-influxdb/#introducing-influx-statsd
+    if labels:
-        if labels:
+        metric += ''.join(',%s=%s' % (k, v) for k, v in labels)
-            metric += ''.join(',%s=%s' % (k, v) for k, v in labels)
+    line = _build_line_parts(metric, value, metric_type, sample_rate)
-        line = _build_line_parts(metric, value, metric_type, sample_rate)
+    return line
-        return line
 
-    @staticmethod
-    def signalfx(metric, value, metric_type, sample_rate, labels):
-        # https://web.archive.org/web/20211123040355/https://docs.signalfx.com/en/latest/integrations/agent/monitors/collectd-statsd.html#adding-dimensions-to-statsd-metrics
-        # https://docs.splunk.com/Observability/gdi/statsd/statsd.html#adding-dimensions-to-statsd-metrics
-        if labels:
-            metric += '[%s]' % ','.join('%s=%s' % (k, v) for k, v in labels)
-        line = _build_line_parts(metric, value, metric_type, sample_rate)
-        return line
 
-    @staticmethod
+def graphite(metric, value, metric_type, sample_rate, labels):
-    def graphite(metric, value, metric_type, sample_rate, labels):
+    # https://graphite.readthedocs.io/en/latest/tags.html#carbon
-        # https://graphite.readthedocs.io/en/latest/tags.html#carbon
+    if labels:
-        if labels:
+        metric += ''.join(';%s=%s' % (k, v) for k, v in labels)
-            metric += ''.join(';%s=%s' % (k, v) for k, v in labels)
+    line = _build_line_parts(metric, value, metric_type, sample_rate)
-        line = _build_line_parts(metric, value, metric_type, sample_rate)
+    return line
-        return line
 
-    @staticmethod
+
-    def dogstatsd(metric, value, metric_type, sample_rate, labels):
+def dogstatsd(metric, value, metric_type, sample_rate, labels):
-        # https://docs.datadoghq.com/developers/dogstatsd/datagram_shell/?tab=metrics
+    # https://docs.datadoghq.com/developers/dogstatsd/datagram_shell/?tab=metrics
-        line = _build_line_parts(metric, value, metric_type, sample_rate)
+    line = _build_line_parts(metric, value, metric_type, sample_rate)
-        if labels:
+    if labels:
-            line += '|#' + ','.join('%s:%s' % (k, v) for k, v in labels)
+        line += '|#' + ','.join('%s:%s' % (k, v) for k, v in labels)
-        return line
+    return line
+
+
+LABEL_MODES = {
+    'disabled': None,
+    'librato': librato,
+    'influxdb': influxdb,
+    'graphite': graphite,
+    'dogstatsd': dogstatsd,
+}
 
 
 def _get_labeled_statsd_formatter(label_mode):
@@ -93,11 +89,9 @@ def _get_labeled_statsd_formatter(label_mode):
     :returns: a label formatting function.
     """
     try:
-        return getattr(LabeledFormats, label_mode)
+        return LABEL_MODES[label_mode]
-    except AttributeError:
+    except KeyError:
-        label_modes = [
+        label_modes = LABEL_MODES.keys()
-            f for f in LabeledFormats.__dict__
-            if not f.startswith('__')]
         raise ValueError(
             'unknown statsd_label_mode %r; '
             'expected one of %r' % (label_mode, label_modes))
@@ -439,7 +433,7 @@ class LabeledStatsdClient(AbstractStatsdClient):
     A statsd client that supports annotating metrics with labels.
 
     Labeled metrics can be emitted in the style of Graphite, Librato, InfluxDB,
-    DogStatsD, or SignalFX by specifying the corresponding ``label_mode`` when
+    or DogStatsD, by specifying the corresponding ``label_mode`` when
     constructing a client. If ``label_mode`` is ``disabled`` then no metrics
     are emitted by the client.
 
@@ -459,8 +453,8 @@ class LabeledStatsdClient(AbstractStatsdClient):
         value between 0 and 1.
     :param sample_rate_factor: A multiplier to apply to the rate at which
         metrics are sampled. Should be a float value between 0 and 1.
-    :param label_mode: one of 'graphite', 'dogstatsd', 'signalfx', 'librato',
+    :param label_mode: one of 'graphite', 'dogstatsd', 'librato', 'influxdb'
-        'influxdb' or 'disabled'.
+        or 'disabled'.
     :param default_labels: a dictionary of labels that will be added to every
         metric emitted by the client.
     :param logger: A stdlib logger instance.

test/unit/common/test_statsd_client.py

@@ -27,8 +27,7 @@ from queue import Queue, Empty
 
 
 from swift.common import statsd_client
-from swift.common.statsd_client import StatsdClient, get_statsd_client, \
+from swift.common.statsd_client import StatsdClient, get_statsd_client
-    LabeledFormats
 
 from test.debug_logger import debug_logger
 
@@ -254,12 +253,19 @@ class TestGetLabeledStatsdClientConfParsing(BaseTestStatsdClient):
         conf = {'statsd_label_mode': 'dogstatsd'}
         logger = debug_logger(log_route='my-log-route')
         client = statsd_client.get_labeled_statsd_client(conf, logger)
-        self.assertEqual(LabeledFormats.dogstatsd, client.label_formatter)
+        self.assertEqual(statsd_client.dogstatsd, client.label_formatter)
         log_lines = logger.get_lines_for_level('debug')
         self.assertEqual(1, len(log_lines))
         self.assertEqual(
             'Labeled statsd mode: dogstatsd (my-log-route)', log_lines[0])
 
+    def test_weird_invalid_attrname_label_mode(self):
+        conf = {'statsd_label_mode': '__class__'}
+        with self.assertRaises(ValueError) as cm:
+            statsd_client.get_labeled_statsd_client(conf, self.logger)
+        self.assertIn("unknown statsd_label_mode '__class__'",
+                      str(cm.exception))
+
     def test_disabled_by_default(self):
         conf = {}
         logger = debug_logger(log_route='my-log-route')
@@ -994,42 +1000,6 @@ class TestGetLabeledStatsdClientOutput(BaseTestStatsdClientOutput):
             'the_counter#action=some,result=ok:1|c|@0.45',
             labeled_statsd.increment, 'the_counter', labels=labels)
 
-    def test_statsd_methods_signalfx(self):
-        conf = {
-            'log_statsd_host': 'localhost',
-            'log_statsd_port': str(self.port),
-            'log_statsd_metric_prefix': 'my_prefix',
-            'statsd_label_mode': 'signalfx',
-        }
-        labeled_statsd = statsd_client.get_labeled_statsd_client(conf)
-        labels = {'action': 'some', 'result': 'ok'}
-        self.assertStat(
-            'the_counter[action=some,result=ok]:1|c',
-            labeled_statsd.increment, 'the_counter', labels=labels)
-        self.assertStat(
-            'the_counter[action=some,result=ok]:-1|c',
-            labeled_statsd.decrement, 'the_counter', labels=labels)
-        self.assertStat(
-            'the_timing[action=some,result=ok]'
-            ':6280.0|ms',
-            labeled_statsd.timing, 'the_timing', 6.28 * 1000, labels=labels)
-        self.assertStat(
-            'the_stat[action=some,result=ok]:3|c',
-            labeled_statsd.update_stats, 'the_stat', 3, labels=labels)
-
-    def test_statsd_methods_signalfx_sample_rate(self):
-        conf = {
-            'log_statsd_host': 'localhost',
-            'log_statsd_port': str(self.port),
-            'statsd_label_mode': 'signalfx',
-            'log_statsd_default_sample_rate': '0.9',
-            'log_statsd_sample_rate_factor': '0.5'}
-        labeled_statsd = statsd_client.get_labeled_statsd_client(conf)
-        labels = {'action': 'some', 'result': 'ok'}
-        self.assertStat(
-            'the_counter[action=some,result=ok]:1|c|@0.45',
-            labeled_statsd.increment, 'the_counter', labels=labels)
-
     def _do_test_statsd_methods_no_labels(self, label_mode):
         conf = {
             'log_statsd_host': 'localhost',
@@ -1069,6 +1039,3 @@ class TestGetLabeledStatsdClientOutput(BaseTestStatsdClientOutput):
 
     def test_statsd_methods_librato_no_labels(self):
         self._do_test_statsd_methods_no_labels('librato')
-
-    def test_statsd_methods_signalfx_no_labels(self):
-        self._do_test_statsd_methods_no_labels('signalfx')