Add support for Tornado 6

This PR adds support for Tornado 6 by conditionally using different
scope manager, context manager and tracing implementation depending
on the version of Tornado and Python being used.

It does not require existing users to change anything other than upgrade
to the latest version of this package.

This package used to use the TornadoScopeManager shipped by
opentracing-python. The scope manager used `tornado.stack_context`
which was deprecated in Tornado 5 and removed in Tornado 6. Tornado
now recommends using contextvars package introduced in Python3.7.
opentracing-python already provides a ContextVarsScopeManager that
builds on top of the contextvars package. It also implements
AsyncioScopeManager which builds on top of asyncio and falls back
on thread local storage to implement context propagation. We fallback on
this for Python 3.6 and older when using Tornado 6 and newer.

The package also had seen some decay and some tests were not passing.
This PR updates the test suite and unit tests to get them working again.

Changes this PR introduces:

- Default to ContextVarsScopeManager instead of TornadoScopeManager.
Fallback on TornadoScopeManager or AsyncioScopeManager based on the
Tornado and Python version.
- Added tox support to enable easier testing across Python and Tornado
versions.
- Updated travis config to work with tox environments. Now each travis
build will run tests on every supported python version in parallel. Each
parallel test will run all tests for all versions of tornado serially.
- The PR add some code that uses the new async/await syntax. Such code
is invalid for older versions of python. To make it works for all
versions, we conditionally import modules depending on the Python
interpreter version.
- To preserve backward compatibility and to keep using common code for
all tornado versions, we've added some noop implementations that are not
to be used with newer versions of tornado.
- `tornado.gen.coroutine` was deprecated in favour of async/await but we
still support it where we can. There is a bug in Tornado 6 that prevents
us from support the deprecated feature on Python3.7 with
ContextVarsScopeManager.
(tornadoweb/tornado#2716)
- Python3.4 also does not pass the tests for `tornado.gen.coroutine` but
it is not a regression caused by this PR. Testing on master results in
the same behavior. For now, I've added skip markers to these tests on
Python3.4. If needed, we can look into supporting these in future in a
separate PR.

.gitignore

@@ -1,8 +1,14 @@
+.coverage
+.python-version
+.tox
 *.pyc
+.vscode
 dist
 bin
 eggs
 lib
 *.egg-info
 build
 env/
+venv/
+.pytest_cache/*

.travis.yml

@@ -1,11 +1,17 @@
 language: python
 python:
   - "2.7"
+  - "3.4"
+  - "3.5"
+  - "3.6"
+  - "3.7"
+  - "3.8"
 
 sudo: required
 
 install:
+  - pip install tox tox-travis
   - make bootstrap
 
 script:
-  -  make test lint
+  - make test

Makefile

@@ -38,9 +38,12 @@ clean-test:
 lint:
 	flake8 $(project) tests
 
-test:
+test-local:
 	py.test -s --cov-report term-missing:skip-covered --cov=$(project)
 
+test:
+	tox
+
 build: 
 	python setup.py build
 

README.rst

@@ -25,7 +25,7 @@ In order to implement tracing in your system (for all the requests), add the fol
 
 .. code-block:: python
 
-    from opentracing.scope_managers.tornado import TornadoScopeManager
+    from opentracing_tornado.scope_managers import TornadoScopeManager
     import tornado_opentracing
 
     # Create your opentracing tracer using TornadoScopeManager for active Span handling.

setup.py

@@ -15,15 +15,17 @@
     platforms='any',
     install_requires=[
         'tornado',
-        'opentracing>=2.0,<2.1',
+        'opentracing>=2.0,<2.4',
         'wrapt',
     ],
     extras_require={
         'tests': [
-            'flake8<3',  # see https://github.com/zheller/flake8-quotes/issues/29
+            'flake8<4',
             'flake8-quotes',
-            'pytest>=2.7,<3',
+            'pytest>=4.6.9',
             'pytest-cov',
+            'mock',
+            'tox',
         ],
     },
     classifiers=[

tests/helpers/__init__.py

@@ -0,0 +1,14 @@
+import sys
+
+import tornado_opentracing
+from opentracing.mocktracer import MockTracer
+from tornado_opentracing.scope_managers import ScopeManager
+
+
+if sys.version_info >= (3, 3):
+    from ._test_case_gen import AsyncHTTPTestCase  # noqa
+else:
+    from ._test_case import AsyncHTTPTestCase  # noqa
+
+
+tracing = tornado_opentracing.TornadoTracing(MockTracer(ScopeManager()))

tests/helpers/_test_case.py

@@ -0,0 +1,8 @@
+import tornado.testing
+
+
+class AsyncHTTPTestCase(tornado.testing.AsyncHTTPTestCase):
+
+    def http_fetch(self, url, *args, **kwargs):
+        self.http_client.fetch(url, self.stop, *args, **kwargs)
+        return self.wait()

tests/helpers/_test_case_gen.py

@@ -0,0 +1,31 @@
+import tornado.testing
+from tornado.httpclient import HTTPError
+from tornado import version_info as tornado_version
+
+from ._test_case import AsyncHTTPTestCase as BaseTestCase
+
+
+use_wait_stop = tornado_version < (5, 0, 0)
+
+if use_wait_stop:
+    def gen_test(func):
+        return func
+else:
+    gen_test = tornado.testing.gen_test
+
+
+class AsyncHTTPTestCase(BaseTestCase):
+
+    @gen_test
+    def _http_fetch_gen(self, url, *args, **kwargs):
+        try:
+            response = yield self.http_client.fetch(url, *args, **kwargs)
+        except HTTPError as exc:
+            response = exc.response
+        return response
+
+    def http_fetch(self, url, *args, **kwargs):
+        fetch = self._http_fetch_gen
+        if use_wait_stop:
+            fetch = super().http_fetch
+        return fetch(url, *args, **kwargs)

tests/helpers/handlers.py

@@ -0,0 +1,22 @@
+import sys
+
+import tornado.web
+
+
+class noopHandler(tornado.web.RequestHandler):
+    def get(self):
+        pass
+
+
+if sys.version_info > (3, 5):
+    from .handlers_async_await import (
+        AsyncScopeHandler,
+        DecoratedAsyncHandler,
+        DecoratedAsyncScopeHandler,
+        DecoratedAsyncErrorHandler
+    )
+else:
+    AsyncScopeHandler = noopHandler
+    DecoratedAsyncHandler = noopHandler
+    DecoratedAsyncScopeHandler = noopHandler
+    DecoratedAsyncErrorHandler = noopHandler

tests/helpers/handlers_async_await.py

@@ -0,0 +1,60 @@
+import asyncio
+
+import tornado.web
+
+from . import tracing
+
+
+class AsyncScopeHandler(tornado.web.RequestHandler):
+    async def do_something(self):
+        tracing = self.settings.get('opentracing_tracing')
+        with tracing.tracer.start_active_span('Child'):
+            tracing.tracer.active_span.set_tag('start', 0)
+            await asyncio.sleep(0)
+            tracing.tracer.active_span.set_tag('end', 1)
+
+    async def get(self):
+        tracing = self.settings.get('opentracing_tracing')
+        span = tracing.get_span(self.request)
+        assert span is not None
+        assert tracing.tracer.active_span is span
+
+        await self.do_something()
+
+        assert tracing.tracer.active_span is span
+        self.write('{}')
+
+
+class DecoratedAsyncHandler(tornado.web.RequestHandler):
+    @tracing.trace('protocol', 'doesntexist')
+    async def get(self):
+        await asyncio.sleep(0)
+        self.set_status(201)
+        self.write('{}')
+
+
+class DecoratedAsyncErrorHandler(tornado.web.RequestHandler):
+    @tracing.trace()
+    async def get(self):
+        await asyncio.sleep(0)
+        raise ValueError('invalid value')
+
+
+class DecoratedAsyncScopeHandler(tornado.web.RequestHandler):
+    async def do_something(self):
+        with tracing.tracer.start_active_span('Child'):
+            tracing.tracer.active_span.set_tag('start', 0)
+            await asyncio.sleep(0)
+            tracing.tracer.active_span.set_tag('end', 1)
+
+    @tracing.trace()
+    async def get(self):
+        span = tracing.get_span(self.request)
+        assert span is not None
+        assert tracing.tracer.active_span is span
+
+        await self.do_something()
+
+        assert tracing.tracer.active_span is span
+        self.set_status(201)
+        self.write('{}')

tests/helpers/markers.py

@@ -0,0 +1,28 @@
+import sys
+
+import pytest
+from tornado import version_info as tornado_version
+
+
+skip_generator_contextvars_on_tornado6 = pytest.mark.skipif(
+    tornado_version >= (6, 0, 0),
+    reason=(
+        'tornado6 has a bug (#2716) that '
+        'prevents contextvars from working.'
+    )
+)
+
+
+skip_generator_contextvars_on_py34 = pytest.mark.skipif(
+    sys.version_info.major == 3 and sys.version_info.minor == 4,
+    reason=('does not work on 3.4 with tornado context stack currently.')
+)
+
+
+skip_no_async_await = pytest.mark.skipif(
+    sys.version_info < (3, 5) or tornado_version < (5, 0),
+    reason=(
+        'async/await is not supported on python older than 3.5 '
+        'and tornado older than 5.0.'
+    )
+)

tests/test_client.py

@@ -15,13 +15,15 @@
 import mock
 
 from opentracing.mocktracer import MockTracer
-from opentracing.scope_managers.tornado import TornadoScopeManager
-from opentracing.scope_managers.tornado import tracer_stack_context
 import tornado.gen
 from tornado.httpclient import HTTPRequest
 import tornado.web
 import tornado.testing
 import tornado_opentracing
+from tornado_opentracing.scope_managers import ScopeManager
+from tornado_opentracing.context_managers import tornado_context
+
+from .helpers import AsyncHTTPTestCase
 
 
 class MainHandler(tornado.web.RequestHandler):
@@ -47,9 +49,9 @@ def make_app():
     return app
 
 
-class TestClient(tornado.testing.AsyncHTTPTestCase):
+class TestClient(AsyncHTTPTestCase):
     def setUp(self):
-        self.tracer = MockTracer(TornadoScopeManager())
+        self.tracer = MockTracer(ScopeManager())
         super(TestClient, self).setUp()
 
     def tearDown(self):
@@ -63,10 +65,9 @@ def test_no_tracer(self):
         tornado_opentracing.init_client_tracing()
 
         with mock.patch('opentracing.tracer', new=self.tracer):
-            with tracer_stack_context():
-                self.http_client.fetch(self.get_url('/'), self.stop)
+            with tornado_context():
+                response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -84,10 +85,9 @@ def test_no_tracer(self):
     def test_simple(self):
         tornado_opentracing.init_client_tracing(self.tracer)
 
-        with tracer_stack_context():
-            self.http_client.fetch(self.get_url('/'), self.stop)
+        with tornado_context():
+            response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -110,10 +110,9 @@ def test_cb(span, request):
         tornado_opentracing.init_client_tracing(self.tracer,
                                                 start_span_cb=test_cb)
 
-        with tracer_stack_context():
-            self.http_client.fetch(self.get_url('/'), self.stop)
+        with tornado_context():
+            response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -135,10 +134,9 @@ def test_cb(span, request):
         tornado_opentracing.init_client_tracing(self.tracer,
                                                 start_span_cb=test_cb)
 
-        with tracer_stack_context():
-            self.http_client.fetch(self.get_url('/'), self.stop)
+        with tornado_context():
+            response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -148,15 +146,14 @@ def test_cb(span, request):
     def test_explicit_parameters(self):
         tornado_opentracing.init_client_tracing(self.tracer)
 
-        with tracer_stack_context():
-            self.http_client.fetch(self.get_url('/error'),
-                                   self.stop,
-                                   raise_error=False,
-                                   method='POST',
-                                   body='')
-        response = self.wait()
-        self.assertEqual(response.code, 500)
+        with tornado_context():
+            response = self.http_fetch(
+                self.get_url('/error'),
+                raise_error=False,
+                method='POST',
+                body='')
 
+        self.assertEqual(response.code, 500)
         spans = self.tracer.finished_spans()
         self.assertEqual(len(spans), 1)
         self.assertTrue(spans[0].finished)
@@ -172,10 +169,8 @@ def test_explicit_parameters(self):
     def test_request_obj(self):
         tornado_opentracing.init_client_tracing(self.tracer)
 
-        with tracer_stack_context():
-            self.http_client.fetch(HTTPRequest(self.get_url('/')), self.stop)
-
-        response = self.wait()
+        with tornado_context():
+            response = self.http_fetch(HTTPRequest(self.get_url('/')))
 
         self.assertEqual(response.code, 200)
 
@@ -194,12 +189,10 @@ def test_request_obj(self):
     def test_server_error(self):
         tornado_opentracing.init_client_tracing(self.tracer)
 
-        with tracer_stack_context():
-            self.http_client.fetch(self.get_url('/error'), self.stop)
+        with tornado_context():
+            response = self.http_fetch(self.get_url('/error'))
 
-        response = self.wait()
         self.assertEqual(response.code, 500)
-
         spans = self.tracer.finished_spans()
         self.assertEqual(len(spans), 1)
         self.assertTrue(spans[0].finished)
@@ -220,10 +213,12 @@ def test_server_error(self):
     def test_server_not_found(self):
         tornado_opentracing.init_client_tracing(self.tracer)
 
-        with tracer_stack_context():
-            self.http_client.fetch(self.get_url('/doesnotexist'), self.stop)
+        with tornado_context():
+            response = self.http_fetch(
+                self.get_url('/doesnotexist'),
+                raise_error=False
+            )
 
-        response = self.wait()
         self.assertEqual(response.code, 404)
 
         spans = self.tracer.finished_spans()