Merge pull request #2213 from iksaif/pr-agg-key

functions: fix groupByNode() output
graphite-project · Jan 26, 2018 · 0f11d54 · 0f11d54
2 parents 2a897cf + bc6ade3
commit 0f11d54
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 5 deletions.
diff --git a/webapp/graphite/render/functions.py b/webapp/graphite/render/functions.py
@@ -192,6 +192,9 @@ def getNodeOrTag(series, n, pathExpression=None):
   return series.tags.get(str(n), '')
 
 def aggKey(series, nodes, pathExpression=None):
+  # if series.name looks like it includes a function, use the first path expression
+  if pathExpression is None and series.name[-1] == ')':
+    pathExpression = _getFirstPathExpression(series.name)
   return '.'.join([getNodeOrTag(series, n, pathExpression) for n in nodes])
 
 def normalize(seriesLists):
@@ -2376,8 +2379,7 @@ def aliasByNode(requestContext, seriesList, *nodes):
     # dc1.server1.load5, dc1.server2.load5, dc1.server1.load10, dc1.server2.load10
   """
   for series in seriesList:
-    pathExpression = _getFirstPathExpression(series.name)
-    series.name = aggKey(series, nodes, pathExpression)
+    series.name = aggKey(series, nodes)
   return seriesList
 
 aliasByNode.group = 'Alias'

diff --git a/webapp/tests/test_functions.py b/webapp/tests/test_functions.py
@@ -37,6 +37,11 @@ def return_less(series, value):
 
 class FunctionsTest(TestCase):
 
+    def setUp(self):
+      super(FunctionsTest, self).setUp()
+      # Display more diff.
+      self.maxDiff = 1024
+
     #
     # Test safeSum()
     #
@@ -2931,21 +2936,41 @@ def test_aliasByMetric(self):
     def test_groupByNode(self):
         seriesList, inputList = self._generate_mr_series()
 
-        def verify_groupByNode(expectedResult, nodeNum):
+        def verify_groupByNode(expectedResult, nodeNum, seriesList):
             results = functions.groupByNode({}, copy.deepcopy(seriesList), nodeNum, "keepLastValue")
 
             self.assertEqual(results, expectedResult)
 
         expectedResult   = [
             TimeSeries('group',0,1,1,[None]),
         ]
-        verify_groupByNode(expectedResult, 0)
+        verify_groupByNode(expectedResult, 0, seriesList)
 
         expectedResult   = [
             TimeSeries('server1',0,1,1,[None]),
             TimeSeries('server2',0,1,1,[None]),
         ]
-        verify_groupByNode(expectedResult, 1)
+        verify_groupByNode(expectedResult, 1, seriesList)
+
+        # Additiona tests
+        seriesList = []
+        names = [
+            "collectd.test-db1.load.value",
+            "sum(collectd.test-db1.load.value)",
+            "sum(sum(collectd.test-db1.load.value))",
+            "divide(collectd.test-db1.load.value, 5)",
+            "scaleToSeconds(divide(collectd.test-db1.load.value, 5),1)",
+        ]
+        expectedResult = []
+        for name in names:
+            series = TimeSeries(name, 0, 1, 1, [1])
+            seriesList.append(series)
+        for expected in ["value"] * len(names):
+            series = TimeSeries(expected, 0, 1, 1, [1])
+            expectedResult.append(series)
+        for i, series in enumerate(seriesList):
+          verify_groupByNode([expectedResult[i]], 3, [series])
+
 
     def test_groupByNodes(self):
         seriesList, inputList = self._generate_mr_series()