From 57bbcd32b2e4150903ec9617adf148993cf1b176 Mon Sep 17 00:00:00 2001
From: David Buxton <david@arachnys.com>
Date: Wed, 20 Jul 2016 12:40:27 +0100
Subject: [PATCH] Use graphite timestamps to select datapoints used in checking

* Potentially addresses #352
* Potentially addresses #129
---
 cabot/cabotapp/graphite.py          | 18 ++++++++++++++++--
 cabot/cabotapp/models.py            |  5 ++++-
 cabot/cabotapp/tests/tests_basic.py |  5 +++--
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/cabot/cabotapp/graphite.py b/cabot/cabotapp/graphite.py
index 3091bbe..3aeda5d 100644
--- a/cabot/cabotapp/graphite.py
+++ b/cabot/cabotapp/graphite.py
@@ -1,6 +1,7 @@
 from django.conf import settings
 import requests
 import logging
+import time
 
 graphite_api = settings.GRAPHITE_API
 user = settings.GRAPHITE_USER
@@ -52,7 +53,7 @@ def get_all_metrics(limit=None):
     return metrics
 
 
-def parse_metric(metric, mins_to_check=5):
+def parse_metric(metric, mins_to_check=5, utcnow=None):
     """
     Returns dict with:
     - num_series_with_data: Number of series with data
@@ -61,6 +62,8 @@ def parse_metric(metric, mins_to_check=5):
     - min
     - average_value
     """
+    if utcnow is None:
+        utcnow = time.time()
     ret = {
         'num_series_with_data': 0,
         'num_series_no_data': 0,
@@ -78,7 +81,7 @@ def parse_metric(metric, mins_to_check=5):
     all_values = []
     for target in data:
         series = {'values': [
-            float(t[0]) for t in target['datapoints'][-mins_to_check:] if t[0] is not None]}
+            float(t[0]) for t in target['datapoints'] if validate_datapoint(t, mins_to_check, utcnow)]}
         series["target"] = target["target"]
         all_values.extend(series['values'])
         if series['values']:
@@ -95,3 +98,14 @@ def parse_metric(metric, mins_to_check=5):
     ret['raw'] = data
     return ret
 
+def validate_datapoint(datapoint, mins_to_check, utcnow):
+    val, timestamp = datapoint
+    secs_to_check = 60 * mins_to_check
+    if val is None:
+        return False
+    if timestamp > (utcnow - secs_to_check):
+        return True
+    else:
+        return False
+
+
diff --git a/cabot/cabotapp/models.py b/cabot/cabotapp/models.py
index 09776ae..4f2b87d 100644
--- a/cabot/cabotapp/models.py
+++ b/cabot/cabotapp/models.py
@@ -612,6 +612,7 @@ def minimize_targets(targets):
 
 
 class GraphiteStatusCheck(StatusCheck):
+
     class Meta(StatusCheck.Meta):
         proxy = True
 
@@ -634,10 +635,12 @@ class GraphiteStatusCheck(StatusCheck):
             return "%s %s %0.1f" % (value, self.check_type, float(self.value))
 
     def _run(self):
+        if not hasattr(self, 'utcnow'):
+            self.utcnow = None
         result = StatusCheckResult(check=self)
 
         failures = []
-        graphite_output = parse_metric(self.metric, mins_to_check=self.frequency)
+        graphite_output = parse_metric(self.metric, mins_to_check=self.frequency, utcnow=self.utcnow)
 
         try:
             result.raw_data = json.dumps(graphite_output['raw'])
diff --git a/cabot/cabotapp/tests/tests_basic.py b/cabot/cabotapp/tests/tests_basic.py
index 5bf3e1f..a83e00c 100644
--- a/cabot/cabotapp/tests/tests_basic.py
+++ b/cabot/cabotapp/tests/tests_basic.py
@@ -242,6 +242,7 @@ class TestCheckRun(LocalTestCase):
     def test_graphite_run(self):
         checkresults = self.graphite_check.statuscheckresult_set.all()
         self.assertEqual(len(checkresults), 2)
+        self.graphite_check.utcnow = 1387818601 # see graphite_response.json for this magic timestamp
         self.graphite_check.run()
         checkresults = self.graphite_check.statuscheckresult_set.all()
         self.assertEqual(len(checkresults), 3)
@@ -291,8 +292,8 @@ class TestCheckRun(LocalTestCase):
 
     @patch('cabot.cabotapp.graphite.requests.get', fake_graphite_series_response)
     def test_graphite_series_run(self):
-        jsn = parse_metric('fake.pattern')
-        self.assertEqual(jsn['average_value'], 59.86)
+        jsn = parse_metric('fake.pattern', utcnow=1387818601)
+        self.assertLess(abs(jsn['average_value']-53.26), 0.1)
         self.assertEqual(jsn['series'][0]['max'], 151.0)
         self.assertEqual(jsn['series'][0]['min'], 0.1)