Flask: Path Templates Support (#212)

* Flask: Path Templates Support

* Py 2.7 unicode string

Author: pglombardo

instana/instrumentation/flask/vanilla.py

@@ -1,5 +1,6 @@
 from __future__ import absolute_import
 
+import re
 import flask
 
 import opentracing
@@ -10,6 +11,8 @@
 from ...singletons import agent, tracer
 from ...util import strip_secrets
 
+path_tpl_re = re.compile('<.*>')
+
 
 def before_request_with_instana(*argv, **kwargs):
     try:
@@ -37,6 +40,12 @@ def before_request_with_instana(*argv, **kwargs):
             span.set_tag("http.params", scrubbed_params)
         if 'HTTP_HOST' in env:
             span.set_tag("http.host", env['HTTP_HOST'])
+
+        if hasattr(flask.request.url_rule, 'rule') and \
+                path_tpl_re.search(flask.request.url_rule.rule) is not None:
+            path_tpl = flask.request.url_rule.rule.replace("<", "{")
+            path_tpl = path_tpl.replace(">", "}")
+            span.set_tag("http.path_tpl", path_tpl)
     except:
         logger.debug("Flask before_request", exc_info=True)
     finally:

instana/instrumentation/flask/with_blinker.py

@@ -1,5 +1,6 @@
 from __future__ import absolute_import
 
+import re
 import opentracing
 import opentracing.ext.tags as ext
 import wrapt
@@ -11,6 +12,8 @@
 import flask
 from flask import request_started, request_finished, got_request_exception
 
+path_tpl_re = re.compile('<.*>')
+
 
 def request_started_with_instana(sender, **extra):
     try:
@@ -38,6 +41,12 @@ def request_started_with_instana(sender, **extra):
             span.set_tag("http.params", scrubbed_params)
         if 'HTTP_HOST' in env:
             span.set_tag("http.host", env['HTTP_HOST'])
+
+        if hasattr(flask.request.url_rule, 'rule') and \
+                path_tpl_re.search(flask.request.url_rule.rule) is not None:
+            path_tpl = flask.request.url_rule.rule.replace("<", "{")
+            path_tpl = path_tpl.replace(">", "}")
+            span.set_tag("http.path_tpl", path_tpl)
     except:
         logger.debug("Flask before_request", exc_info=True)
 

tests/apps/flaskalino.py

@@ -44,6 +44,11 @@ def hello():
     return "<center><h1>🐍 Hello Stan! 🦄</h1></center>"
 
 
+@app.route("/users/<username>/sayhello")
+def username_hello(username):
+    return u"<center><h1>🐍 Hello %s! 🦄</h1></center>" % username
+
+
 @app.route("/complex")
 def gen_opentracing():
     with tracer.start_active_span('asteroid') as pscope:

tests/test_flask.py

@@ -94,6 +94,9 @@ def test_get_request(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_render_template(self):
         with tracer.start_active_span('test'):
             response = self.http.request('GET', testenv["wsgi_server"] + '/render')
@@ -174,6 +177,9 @@ def test_render_template(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_render_template_string(self):
         with tracer.start_active_span('test'):
             response = self.http.request('GET', testenv["wsgi_server"] + '/render_string')
@@ -254,6 +260,9 @@ def test_render_template_string(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_301(self):
         with tracer.start_active_span('test'):
             response = self.http.request('GET', testenv["wsgi_server"] + '/301', redirect=False)
@@ -322,6 +331,9 @@ def test_301(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_404(self):
         with tracer.start_active_span('test'):
             response = self.http.request('GET', testenv["wsgi_server"] + '/11111111111')
@@ -390,6 +402,9 @@ def test_404(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_500(self):
         with tracer.start_active_span('test'):
             response = self.http.request('GET', testenv["wsgi_server"] + '/500')
@@ -458,6 +473,9 @@ def test_500(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_render_error(self):
         if signals_available is True:
             raise unittest.SkipTest("Exceptions without handlers vary with blinker")
@@ -535,6 +553,9 @@ def test_render_error(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_exception(self):
         if signals_available is True:
             raise unittest.SkipTest("Exceptions without handlers vary with blinker")
@@ -604,6 +625,9 @@ def test_exception(self):
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
 
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
     def test_custom_exception_with_log(self):
         with tracer.start_active_span('test'):
             response = self.http.request('GET', testenv["wsgi_server"] + '/exception-invalid-usage')
@@ -679,3 +703,77 @@ def test_custom_exception_with_log(self):
         self.assertIsNotNone(urllib3_span.stack)
         self.assertTrue(type(urllib3_span.stack) is list)
         self.assertTrue(len(urllib3_span.stack) > 1)
+
+        # We should NOT have a path template for this route
+        self.assertIsNone(wsgi_span.data.http.path_tpl)
+
+    def test_path_templates(self):
+        with tracer.start_active_span('test'):
+            response = self.http.request('GET', testenv["wsgi_server"] + '/users/Ricky/sayhello')
+
+        spans = self.recorder.queued_spans()
+        self.assertEqual(3, len(spans))
+
+        wsgi_span = spans[0]
+        urllib3_span = spans[1]
+        test_span = spans[2]
+
+        assert response
+        self.assertEqual(200, response.status)
+
+        assert('X-Instana-T' in response.headers)
+        assert(int(response.headers['X-Instana-T'], 16))
+        self.assertEqual(response.headers['X-Instana-T'], wsgi_span.t)
+
+        assert('X-Instana-S' in response.headers)
+        assert(int(response.headers['X-Instana-S'], 16))
+        self.assertEqual(response.headers['X-Instana-S'], wsgi_span.s)
+
+        assert('X-Instana-L' in response.headers)
+        self.assertEqual(response.headers['X-Instana-L'], '1')
+
+        assert('Server-Timing' in response.headers)
+        server_timing_value = "intid;desc=%s" % wsgi_span.t
+        self.assertEqual(response.headers['Server-Timing'], server_timing_value)
+
+        self.assertIsNone(tracer.active_span)
+
+        # Same traceId
+        self.assertEqual(test_span.t, urllib3_span.t)
+        self.assertEqual(urllib3_span.t, wsgi_span.t)
+
+        # Parent relationships
+        self.assertEqual(urllib3_span.p, test_span.s)
+        self.assertEqual(wsgi_span.p, urllib3_span.s)
+
+        # Error logging
+        self.assertFalse(test_span.error)
+        self.assertIsNone(test_span.ec)
+        self.assertFalse(urllib3_span.error)
+        self.assertIsNone(urllib3_span.ec)
+        self.assertFalse(wsgi_span.error)
+        self.assertIsNone(wsgi_span.ec)
+
+        # wsgi
+        self.assertEqual("wsgi", wsgi_span.n)
+        self.assertEqual('127.0.0.1:' + str(testenv['wsgi_port']), wsgi_span.data.http.host)
+        self.assertEqual('/users/Ricky/sayhello', wsgi_span.data.http.url)
+        self.assertEqual('GET', wsgi_span.data.http.method)
+        self.assertEqual(200, wsgi_span.data.http.status)
+        self.assertIsNone(wsgi_span.data.http.error)
+        self.assertIsNotNone(wsgi_span.stack)
+        self.assertEqual(2, len(wsgi_span.stack))
+
+        # urllib3
+        self.assertEqual("test", test_span.data.sdk.name)
+        self.assertEqual("urllib3", urllib3_span.n)
+        self.assertEqual(200, urllib3_span.data.http.status)
+        self.assertEqual(testenv["wsgi_server"] + '/users/Ricky/sayhello', urllib3_span.data.http.url)
+        self.assertEqual("GET", urllib3_span.data.http.method)
+        self.assertIsNotNone(urllib3_span.stack)
+        self.assertTrue(type(urllib3_span.stack) is list)
+        self.assertTrue(len(urllib3_span.stack) > 1)
+
+        # We should have a reported path template for this route
+        self.assertEqual("/users/{username}/sayhello", wsgi_span.data.http.path_tpl)
+