Merge pull request #344 from bmermet/jinja-not-closing-spans

Fix flask instrumentation not closing spans

Author: Emanuele Palazzetti

ddtrace/contrib/flask/middleware.py

@@ -59,16 +59,7 @@ def __init__(self, app, tracer, service="flask", use_signals=True):
             self.app.before_request(self._before_request)
             self.app.after_request(self._after_request)
 
-        # Instrument template rendering. If it's flask >= 0.11, we can use
-        # signals, Otherwise we have to patch a global method.
-        template_signals = {
-            'before_render_template': self._template_started,  # added in 0.11
-            'template_rendered': self._template_done
-        }
-        if self.use_signals and _signals_exist(template_signals):
-            self._connect(template_signals)
-        else:
-            _patch_render(tracer)
+        _patch_render(tracer)
 
     def _flask_signals_exist(self, names):
         """ Return true if the current version of flask has all of the given
@@ -177,19 +168,6 @@ def _request_exception(self, *args, **kwargs):
         except Exception:
             self.app.logger.exception("error tracing error")
 
-    def _template_started(self, sender, template, *args, **kwargs):
-        span = self._tracer.trace('flask.template')
-        try:
-            span.span_type = http.TEMPLATE
-            span.set_tag("flask.template", template.name or "string")
-        finally:
-            g.flask_datadog_tmpl_span = span
-
-    def _template_done(self, *arg, **kwargs):
-        span = getattr(g, 'flask_datadog_tmpl_span', None)
-        if span:
-            span.finish()
-
 
 def _patch_render(tracer):
     """ patch flask's render template methods with the given tracer. """

tests/contrib/flask/test_flask.py

@@ -59,6 +59,9 @@ def tmpl():
 def tmpl_err():
     return render_template('err.html')
 
+@app.route('/tmpl/render_err')
+def tmpl_render_err():
+    return render_template('render_err.html')
 
 @app.route('/child')
 def child():
@@ -215,6 +218,36 @@ def test_template_err(self):
         eq_(s.meta.get(http.STATUS_CODE), '500')
         eq_(s.meta.get(http.METHOD), 'GET')
 
+    def test_template_render_err(self):
+        tracer.debug_logging = True
+        start = time.time()
+        try:
+            app.get('/tmpl/render_err')
+        except Exception:
+            pass
+        else:
+            assert 0
+        end = time.time()
+
+        # ensure trace worked
+        assert not tracer.current_span(), tracer.current_span().pprint()
+        spans = writer.pop()
+        eq_(len(spans), 2)
+        by_name = {s.name:s for s in spans}
+        s = by_name["flask.request"]
+        eq_(s.service, service)
+        eq_(s.resource, "tmpl_render_err")
+        assert s.start >= start
+        assert s.duration <= end - start
+        eq_(s.error, 1)
+        eq_(s.meta.get(http.STATUS_CODE), '500')
+        eq_(s.meta.get(http.METHOD), 'GET')
+        t = by_name["flask.template"]
+        eq_(t.get_tag("flask.template"), "render_err.html")
+        eq_(t.error, 1)
+        eq_(t.parent_id, s.span_id)
+        eq_(t.trace_id, s.trace_id)
+
     def test_error(self):
         start = time.time()
         rv = app.get('/error')

tests/contrib/flask/test_templates/render_err.html

@@ -0,0 +1 @@
+hello {{object.method()}}