From f18e2e8c76bb098536c667b5e0f9512f8c979d2c Mon Sep 17 00:00:00 2001
From: Ian Wienand <iwienand@redhat.com>
Date: Fri, 28 Sep 2018 11:07:16 +1000
Subject: [PATCH] Normalise more of the API stats calls

We currently have keys like "ComputePostOs-volumes_boot" for providers
using boot-from-volume and other various "os-" keys depending on the
provider.  Normalise all these to regular CamelCase.  A basic
test-case is added.

Additionally add some documentation on the API call stats, pointing
out they reflect internal details so are subject to change.  A release
note is added for the updated stats.

Change-Id: If8398906a5a7ad3d96e985263b0c841e5dcaf7b5
---
 doc/source/operation.rst                          | 15 +++++++++++++++
 nodepool/task_manager.py                          | 11 ++++++++---
 nodepool/tests/test_sdk_integration.py            | 13 +++++++++++++
 .../stats-normalisation-088eae77f7d0c9e8.yaml     |  8 ++++++++
 4 files changed, 44 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/stats-normalisation-088eae77f7d0c9e8.yaml

diff --git a/doc/source/operation.rst b/doc/source/operation.rst
index 41950d44e..bd5809fb7 100644
--- a/doc/source/operation.rst
+++ b/doc/source/operation.rst
@@ -385,3 +385,18 @@ Nodepool launcher
    duration of the launch.
 
    See :ref:`nodepool.launch <nodepool_launch>` for a list of possible results.
+
+OpenStack API stats
+~~~~~~~~~~~~~~~~~~~
+
+Low level details on the timing of OpenStack API calls will be logged
+by the API task manager.  These calls are logged under
+``nodepool.task.<provider>.<api-call>``.  The API call name is of the
+generic format ``<endpoint><method><operation>`` transformed into a
+CamelCase value with no deliminators; for example the
+``compute.GET.servers`` call becomes ``ComputeGetServers`` and
+``compute.POST.os-volumes_boot`` becomes ``ComputePostOsVolumesBoot``.
+
+Since these calls reflect the internal operations of the
+``openstacksdk``, the exact keys logged may vary across providers and
+releases.
diff --git a/nodepool/task_manager.py b/nodepool/task_manager.py
index 26ae7faf4..0e52c35b5 100644
--- a/nodepool/task_manager.py
+++ b/nodepool/task_manager.py
@@ -18,6 +18,7 @@
 
 import threading
 import logging
+import re
 import queue
 import time
 
@@ -27,10 +28,14 @@ from nodepool import stats
 
 
 def _transform_task_name(task_name):
-    # openstacksdk sets task.name to something like "compute.DELETE.servers"
-    # We want ComputeDeleteServers
+    # Transform openstacksdk internal task name to something more
+    # suitable for sending to statsd for tracking; e.g.
+    #
+    #  compute.DELETE.servers -> ComputeDeleteServers
+    #  compute.POST.os-volumes_boot -> ComputePostOsVolumesBoot
+    parts = re.split('[.\-_]', task_name)
     return "".join(
-        [part.lower().capitalize() for part in task_name.split('.')]
+        [part.lower().capitalize() for part in parts]
     )
 
 
diff --git a/nodepool/tests/test_sdk_integration.py b/nodepool/tests/test_sdk_integration.py
index 5208da0bc..6a045780f 100644
--- a/nodepool/tests/test_sdk_integration.py
+++ b/nodepool/tests/test_sdk_integration.py
@@ -20,10 +20,23 @@ import yaml
 
 from nodepool import config as nodepool_config
 from nodepool import provider_manager
+from nodepool import task_manager
 from nodepool import tests
 
 
 class TestShadeIntegration(tests.IntegrationTestCase):
+    def test_task_name_transformation(self):
+        t = task_manager._transform_task_name
+        self.assertEqual(
+            t('compute.DELETE.servers'),
+            'ComputeDeleteServers')
+        self.assertEqual(
+            t('compute.POST.os-volumes_boot'),
+            'ComputePostOsVolumesBoot')
+        self.assertEqual(
+            t('compute.GET.os-availability-zone'),
+            'ComputeGetOsAvailabilityZone')
+
     def _cleanup_cloud_config(self):
         os.remove(self.clouds_path)
 
diff --git a/releasenotes/notes/stats-normalisation-088eae77f7d0c9e8.yaml b/releasenotes/notes/stats-normalisation-088eae77f7d0c9e8.yaml
new file mode 100644
index 000000000..f03b04577
--- /dev/null
+++ b/releasenotes/notes/stats-normalisation-088eae77f7d0c9e8.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Task names are now consistently normalised to CamelCase without
+    deliminators.  Some statistics sent to statsd with ``-`` or ``_``
+    characters will have changed keys, for example
+    ``ComputePostOs-volumes_boot`` is now
+    ``ComputePostOsVolumesBoot``.