Repository: graphite-project/graphite-web
Issue: carbonlink query doesn't work for tagged series

When fetching data from a tagged series, graphite-web as evidenced by the carbon-cache query.log uses the encoded series path (something like `_tagged.301.220.3012209b6ac3b582a8fd1a3317a5f40407fa446943af8f24ed32beff3f13b620` in my case with `TAG_HASH_FILENAMES=True`) to query the carbon-cache for in-memory data. However, AFAICT carbon-cache organizes its memory cache not by the encoded path, but by the path it received from the metric sender (after normalizing it, ie. sorting the tags and converting OpenMetrics tag syntax to Carbon syntax), something like `my.metric;bar=a;foo=b`. Hashing to get the filename is only done when data is finally written to disk. Therefore, the carbonlink query will fail to retrieve any data currently floating in the carbon-cache.

In theory this could be fixed on either end by changing the behaviour to match the other end. However, if you were to change this in carbon-cache, you would run into a related issue, namely that the relay doesn't take tagging into account when consistent-hashing (I'm going to open another bug report on carbon for that). Since it is essential that the hash used for consistent-hashing is the same in both graphite-web and carbon-relay, changing carbon-cache would mean that carbon-relay would also have to be aware of the TAG_HASH_FILENAMES setting for one thing, and it would have to calculate the final encoded path for the series in order to decide to which carbon-cache the metric should be sent.

OTOH, fixing it in graphite-web should be straightforward, since at the point where carbonlink is queried the human-friendly name of the series must have already been queried from the TAGDB (otherwise the series wouldn't have been found in the first place).

Changed files:
webapp/graphite/finders/standard.py, webapp/tests/test_finders.py

diff --git a/webapp/graphite/finders/standard.py b/webapp/graphite/finders/standard.py
index a9e28727..117338e4 100644
--- a/webapp/graphite/finders/standard.py
+++ b/webapp/graphite/finders/standard.py
@@ -69,13 +69,14 @@ class StandardFinder(BaseFinder):
                     datasource_pattern = None
 
                 absolute_path = join(root_dir, relative_path)
-                metric_path = fs_to_metric(relative_path)
-                real_metric_path = get_real_metric_path(absolute_path, metric_path)
 
                 # if we're finding by tag, return the proper metric path
                 if tagged:
                   metric_path = query.pattern
+                  real_metric_path = query.pattern
                 else:
+                  metric_path = fs_to_metric(relative_path)
+                  real_metric_path = get_real_metric_path(absolute_path, metric_path)
                   metric_path_parts = metric_path.split('.')
                   for field_index in find_escaped_pattern_fields(query.pattern):
                       metric_path_parts[field_index] = pattern_parts[field_index].replace('\\\\', '')
diff --git a/webapp/tests/test_finders.py b/webapp/tests/test_finders.py
index 9f025948..b6ee5ff4 100644
--- a/webapp/tests/test_finders.py
+++ b/webapp/tests/test_finders.py
@@ -247,6 +247,25 @@ class StandardFinderTest(TestCase):
             scandir_mock.call_count = 0
             self.wipe_whisper()
 
+    def test_standard_finder_tagged_whisper_carbonlink(self):
+        try:
+            self.create_whisper(join(
+              '_tagged',
+              'b34',
+              '2de',
+              # foo;bar=baz2
+              'b342defa10cb579981c63ef78be5ac248f681f4bd2c35bc0209d3a7b9eb99346.wsp'
+            ))
+
+            finder = get_finders('graphite.finders.standard.StandardFinder')[0]
+
+            nodes = list(finder.find_nodes(FindQuery('foo;bar=baz2', None, None)))
+            self.assertEqual(len(nodes), 1)
+            self.assertEqual(nodes[0].reader.real_metric_path, 'foo;bar=baz2')
+
+        finally:
+            self.wipe_whisper()
+
     def test_globstar(self):
         self.addCleanup(self.wipe_whisper)
         store  = Store(finders=get_finders('graphite.finders.standard.StandardFinder'))