launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/turnip:add-statsd-env-label into turnip:master.

Commit message:
Add environment label for Statsd metrics

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/turnip/+git/turnip/+merge/393116

This branch changes the prefix from 'turnip_production_git'/'turnip_qastaging_git'/ 'turnip_test_git' to 'turnip' for all environments. This prefix together with the gauge name set to 'statsd' will give us the generic top label: 'turnip_statsd'.

Notes:
1: We already have an environment label in production, but not in QAS:
https://chat.canonical.com/canonical/pl/qgpj6kfuoifs7xqnin8pwqwfsa
https://chat.canonical.com/canonical/pl/4dagm948kb8btja3eenqk5kmny

For this reason I went with 'env' as the label name so it doesn't clash with the existent one in Production.

2: We need to pass in some name ('statsd') for the gauge so that the top label stays generic (currently in this branch: 'turnip_statsd' https://chat.canonical.com/canonical/pl/41jwxw9zu7nt8by4czr6qx61dh). 
If we pass in only the prefix, the top label will not remain generic as the first gauge label (operation) gets attached to the prefix and results in as many top labels as there are git operations (git-receive-pack, git-upload-pack....): https://chat.canonical.com/canonical/pl/m3cpr1q98bfhxmso7w8anm4cqw
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/turnip:add-statsd-env-label into turnip:master.

diff --git a/charm/turnip-pack-backend/config.yaml b/charm/turnip-pack-backend/config.yaml
index 489c6ec..ed37077 100644
--- a/charm/turnip-pack-backend/config.yaml
+++ b/charm/turnip-pack-backend/config.yaml
@@ -23,3 +23,7 @@ options:
     type: string
     default: turnip.test
     description: Prefix for statsd metrics.
+  statsd_environment:
+    type: string
+    default: local
+    description: Environment variable set for statsd metrics labels.
diff --git a/charm/turnip-pack-backend/templates/turnip-pack-backend.service.j2 b/charm/turnip-pack-backend/templates/turnip-pack-backend.service.j2
index cc92150..29d4cc6 100644
--- a/charm/turnip-pack-backend/templates/turnip-pack-backend.service.j2
+++ b/charm/turnip-pack-backend/templates/turnip-pack-backend.service.j2
@@ -19,6 +19,7 @@ Environment=VIRTINFO_TIMEOUT={{ virtinfo_timeout }}
 Environment=STATSD_HOST={{ statsd_host }}
 Environment=STATSD_PORT={{ statsd_port }}
 Environment=STATSD_PREFIX={{ statsd_prefix }}
+Environment=STATSD_ENVIRONMENT={{ statsd_environment }}
 ExecStart={{ venv_dir }}/bin/twistd --nodaemon --pidfile= --logfile={{ base_dir }}/logs/turnip-pack-backend.log --python=packbackendserver.tac
 ExecReload=/bin/kill -s HUP $MAINPID
 LimitNOFILE=1048576
diff --git a/config.yaml b/config.yaml
index af7daa5..0273840 100644
--- a/config.yaml
+++ b/config.yaml
@@ -23,4 +23,5 @@ main_site_root: https://launchpad.test/
 celery_broker: pyamqp://guest@localhost//
 statsd_host: 127.0.0.1
 statsd_port: 8125
-statsd_prefix: turnip.test
+statsd_prefix: turnip
+statsd_environment: local
diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 0d8292f..b50966d 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -327,24 +327,31 @@ class GitProcessProtocol(protocol.ProcessProtocol, object):
                 # can't be used in statsd
                 repository = re.sub('[^0-9a-zA-Z]+', '-',
                                     self.peer.raw_pathname)
-                gauge_name = ("git,operation={},repo={},metric=max_rss"
-                              .format(
-                                  self.peer.command,
-                                  repository))
+                environment = config.get("statsd_environment")
+                gauge_name = (
+                    "statsd,operation={},repo={},env={},metric=max_rss"
+                    .format(
+                        self.peer.command,
+                        repository,
+                        environment))
 
                 self.statsd_client.gauge(gauge_name, resource_usage['max_rss'])
 
-                gauge_name = ("git,operation={},repo={},metric=system_time"
-                              .format(
-                                  self.peer.command,
-                                  repository))
+                gauge_name = (
+                    "statsd,operation={},repo={},env={},metric=system_time"
+                    .format(
+                        self.peer.command,
+                        repository,
+                        environment))
                 self.statsd_client.gauge(gauge_name,
                                          resource_usage['system_time'])
 
-                gauge_name = ("git,operation={},repo={},metric=user_time"
-                              .format(
-                                  self.peer.command,
-                                  repository))
+                gauge_name = (
+                    "statsd,operation={},repo={},env={},metric=user_time"
+                    .format(
+                        self.peer.command,
+                        repository,
+                        environment))
                 self.statsd_client.gauge(gauge_name,
                                          resource_usage['user_time'])
 
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index fb168e7..7e38543 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -106,6 +106,7 @@ class FunctionalTestMixin(WithScenarios):
         self.virtinfo.ref_permissions = {
             b'refs/heads/master': ['create', 'push']}
         config.defaults['virtinfo_endpoint'] = self.virtinfo_url
+        config.defaults['statsd_environment'] = 'local'
 
     def startHookRPC(self):
         self.hookrpc_handler = HookRPCHandler(self.virtinfo_url, 15)
@@ -189,8 +190,10 @@ class FunctionalTestMixin(WithScenarios):
         metrics = ['max_rss', 'system_time', 'user_time']
         repository = re.sub('[^0-9a-zA-Z]+', '-', repo)
         self.assertThat(self.statsd_client.vals, MatchesDict({
-            u'git,operation={},repo={},metric={}'.format(
-                command, repository, metric): Not(Is(None))
+            u'statsd,operation={},repo={},env={},metric={}'
+            .format(
+                command, repository,
+                config.get('statsd_environment'), metric): Not(Is(None))
             for metric in metrics}))
 
     @defer.inlineCallbacks
diff --git a/turnipserver.py b/turnipserver.py
index 08fc987..8c91f1f 100644
--- a/turnipserver.py
+++ b/turnipserver.py
@@ -41,6 +41,7 @@ VIRTINFO_TIMEOUT = int(config.get('virtinfo_timeout'))
 STATSD_HOST = config.get('statsd_host')
 STATSD_PORT = config.get('statsd_port')
 STATSD_PREFIX = config.get('statsd_prefix')
+STATSD_ENVIRONMENT = config.get('statsd_environment')
 
 # turnipserver.py is preserved for convenience in development, services
 # in production are run in separate processes.