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
 

setup.py

@@ -15,15 +15,17 @@
     platforms='any',
     install_requires=[
         'tornado',
-        'opentracing>=2.0,<2.1',
+        'opentracing>=2.0,<=2.3',
         '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/_handlers_async_py35.py

@@ -0,0 +1,60 @@
+import asyncio
+
+import tornado.web
+
+from .tracing 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/_test_case_base.py

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

tests/_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_base import BaseAsyncHTTPTestCase
+
+
+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(BaseAsyncHTTPTestCase):
+
+    @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/handlers_async.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_py35 import (
+        AsyncScopeHandler,
+        DecoratedAsyncHandler,
+        DecoratedAsyncScopeHandler,
+        DecoratedAsyncErrorHandler
+    )
+else:
+    AsyncScopeHandler = noopHandler
+    DecoratedAsyncHandler = noopHandler
+    DecoratedAsyncScopeHandler = noopHandler
+    DecoratedAsyncErrorHandler = noopHandler

tests/helpers.py

@@ -0,0 +1,31 @@
+import sys
+
+import pytest
+from tornado import version_info as tornado_version
+
+
+def skip_generator_contextvars_on_tornado6(func):
+    return pytest.mark.skipif(
+        tornado_version >= (6, 0, 0),
+        reason=(
+            'tornado6 has a bug (#2716) that '
+            'prevents contextvars from working.'
+        )
+    )(func)
+
+
+def skip_generator_contextvars_on_py34(func):
+    return 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.')
+    )(func)
+
+
+def skip_no_async_await(func):
+    return 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.'
+        )
+    )(func)

tests/test_case.py

@@ -0,0 +1,7 @@
+import sys
+
+
+if sys.version_info >= (3, 3):
+    from ._test_case_gen import AsyncHTTPTestCase  # noqa
+else:
+    from ._test_case_base import BaseAsyncHTTPTestCase as AsyncHTTPTestCase  # noqa

tests/test_client.py

@@ -15,13 +15,14 @@
 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 import ScopeManager, trace_context
+
+from .test_case import AsyncHTTPTestCase
 
 
 class MainHandler(tornado.web.RequestHandler):
@@ -47,9 +48,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 +64,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 trace_context():
+                response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -84,10 +84,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 trace_context():
+            response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -110,10 +109,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 trace_context():
+            response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -135,10 +133,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 trace_context():
+            response = self.http_fetch(self.get_url('/'))
 
-        response = self.wait()
         self.assertEqual(response.code, 200)
 
         spans = self.tracer.finished_spans()
@@ -148,15 +145,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 trace_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 +168,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 trace_context():
+            response = self.http_fetch(HTTPRequest(self.get_url('/')))
 
         self.assertEqual(response.code, 200)
 
@@ -194,12 +188,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 trace_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 +212,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 trace_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()