From 1f15deecfc5c3ff360b53d0e3e6368a317606bc0 Mon Sep 17 00:00:00 2001 From: Will Gittoes Date: Thu, 8 Mar 2018 17:31:46 -0500 Subject: [PATCH 1/4] [falcon] Adds distributed tracing. --- ddtrace/contrib/falcon/__init__.py | 14 +++++++++++++- ddtrace/contrib/falcon/middleware.py | 12 +++++++++++- ddtrace/contrib/falcon/patch.py | 5 ++++- tests/contrib/falcon/app/app.py | 5 +++-- tests/contrib/falcon/test_suite.py | 7 +++++++ tox.ini | 2 +- 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/ddtrace/contrib/falcon/__init__.py b/ddtrace/contrib/falcon/__init__.py index 580764cf213..4a7112b85ea 100644 --- a/ddtrace/contrib/falcon/__init__.py +++ b/ddtrace/contrib/falcon/__init__.py @@ -5,8 +5,20 @@ from ddtrace import tracer from ddtrace.contrib.falcon import TraceMiddleware - mw = TraceMiddleware(tracer, 'my-falcon-app') + mw = TraceMiddleware(tracer, 'my-falcon-app', distributed_tracing=True) falcon.API(middleware=[mw]) + +You can also use the autopatching functionality: + + import falcon + from ddtrace import tracer, patch + + patch(falcon=True) + + app = falcon.API() + +To enable distributed tracing when using autopatching, set the +DATADOG_FALCON_DISTRIBUTED_TRACING environment variable to true. """ from ..util import require_modules diff --git a/ddtrace/contrib/falcon/middleware.py b/ddtrace/contrib/falcon/middleware.py index ff7e3f55a48..8280c4a40f4 100644 --- a/ddtrace/contrib/falcon/middleware.py +++ b/ddtrace/contrib/falcon/middleware.py @@ -1,15 +1,18 @@ import sys +from ddtrace import tracer from ddtrace.ext import http as httpx +from ddtrace.propagation.http import HTTPPropagator from ...ext import AppTypes class TraceMiddleware(object): - def __init__(self, tracer, service="falcon"): + def __init__(self, tracer, service="falcon", distributed_tracing=False): # store tracing references self.tracer = tracer self.service = service + self._distributed_tracing = distributed_tracing # configure Falcon service self.tracer.set_service_info( @@ -19,6 +22,13 @@ def __init__(self, tracer, service="falcon"): ) def process_request(self, req, resp): + if self._distributed_tracing: + # Falcon uppercases all header names. + headers = dict((k.lower(), v) for k, v in req.headers.items()) + propagator = HTTPPropagator() + context = propagator.extract(headers) + self.tracer.context_provider.activate(context) + span = self.tracer.trace( "falcon.request", service=self.service, diff --git a/ddtrace/contrib/falcon/patch.py b/ddtrace/contrib/falcon/patch.py index 1840b03edfc..55f892b0aa1 100644 --- a/ddtrace/contrib/falcon/patch.py +++ b/ddtrace/contrib/falcon/patch.py @@ -5,6 +5,7 @@ from ddtrace import tracer from .middleware import TraceMiddleware +from ...util import asbool def patch(): @@ -21,8 +22,10 @@ def patch(): def traced_init(wrapped, instance, args, kwargs): mw = kwargs.pop('middleware', []) service = os.environ.get('DATADOG_SERVICE_NAME') or 'falcon' + distributed_tracing = asbool(os.environ.get( + 'DATADOG_FALCON_DISTRIBUTED_TRACING')) or False - mw.insert(0, TraceMiddleware(tracer, service)) + mw.insert(0, TraceMiddleware(tracer, service, distributed_tracing)) kwargs['middleware'] = mw wrapped(*args, **kwargs) diff --git a/tests/contrib/falcon/app/app.py b/tests/contrib/falcon/app/app.py index dd1f0937633..c0e1eac35bf 100644 --- a/tests/contrib/falcon/app/app.py +++ b/tests/contrib/falcon/app/app.py @@ -5,9 +5,10 @@ from . import resources -def get_app(tracer=None): +def get_app(tracer=None, distributed_tracing=False): # initialize a traced Falcon application - middleware = [TraceMiddleware(tracer)] if tracer else [] + middleware = [TraceMiddleware( + tracer, distributed_tracing=distributed_tracing)] if tracer else [] app = falcon.API(middleware=middleware) # add resource routing diff --git a/tests/contrib/falcon/test_suite.py b/tests/contrib/falcon/test_suite.py index 8f8b1c46cc7..f87354a97ed 100644 --- a/tests/contrib/falcon/test_suite.py +++ b/tests/contrib/falcon/test_suite.py @@ -29,6 +29,7 @@ def test_404(self): eq_(span.resource, 'GET 404') eq_(span.get_tag(httpx.STATUS_CODE), '404') eq_(span.get_tag(httpx.URL), 'http://falconframework.org/fake_endpoint') + eq_(span.parent_id, None) def test_exception(self): try: @@ -47,6 +48,7 @@ def test_exception(self): eq_(span.resource, 'GET tests.contrib.falcon.app.resources.ResourceException') eq_(span.get_tag(httpx.STATUS_CODE), '500') eq_(span.get_tag(httpx.URL), 'http://falconframework.org/exception') + eq_(span.parent_id, None) def test_200(self): out = self.simulate_get('/200') @@ -62,6 +64,7 @@ def test_200(self): eq_(span.resource, 'GET tests.contrib.falcon.app.resources.Resource200') eq_(span.get_tag(httpx.STATUS_CODE), '200') eq_(span.get_tag(httpx.URL), 'http://falconframework.org/200') + eq_(span.parent_id, None) def test_201(self): out = self.simulate_post('/201') @@ -77,6 +80,7 @@ def test_201(self): eq_(span.resource, 'POST tests.contrib.falcon.app.resources.Resource201') eq_(span.get_tag(httpx.STATUS_CODE), '201') eq_(span.get_tag(httpx.URL), 'http://falconframework.org/201') + eq_(span.parent_id, None) def test_500(self): out = self.simulate_get('/500') @@ -92,6 +96,7 @@ def test_500(self): eq_(span.resource, 'GET tests.contrib.falcon.app.resources.Resource500') eq_(span.get_tag(httpx.STATUS_CODE), '500') eq_(span.get_tag(httpx.URL), 'http://falconframework.org/500') + eq_(span.parent_id, None) def test_404_exception(self): out = self.simulate_get('/not_found') @@ -106,6 +111,7 @@ def test_404_exception(self): eq_(span.resource, 'GET tests.contrib.falcon.app.resources.ResourceNotFound') eq_(span.get_tag(httpx.STATUS_CODE), '404') eq_(span.get_tag(httpx.URL), 'http://falconframework.org/not_found') + eq_(span.parent_id, None) def test_404_exception_no_stacktracer(self): # it should not have the stacktrace when a 404 exception is raised @@ -120,3 +126,4 @@ def test_404_exception_no_stacktracer(self): eq_(span.service, self._service) eq_(span.get_tag(httpx.STATUS_CODE), '404') ok_(span.get_tag(errx.ERROR_TYPE) is None) + eq_(span.parent_id, None) diff --git a/tox.ini b/tox.ini index 8bcc83f6ad0..4d804e99d46 100644 --- a/tox.ini +++ b/tox.ini @@ -253,7 +253,7 @@ commands = flaskcache{012,013}: nosetests {posargs} tests/contrib/flask_cache flask{010,011,012}: nosetests {posargs} tests/contrib/flask flask-autopatch{010,011,012}: ddtrace-run nosetests {posargs} tests/contrib/flask_autopatch - falcon{10,11,12}: nosetests {posargs} tests/contrib/falcon/test_middleware.py + falcon{10,11,12}: nosetests {posargs} tests/contrib/falcon/test_middleware.py tests/contrib/falcon/test_distributed_tracing.py falcon-autopatch{10,11,12}: ddtrace-run nosetests {posargs} tests/contrib/falcon/test_autopatch.py gevent{11,12}: nosetests {posargs} tests/contrib/gevent gevent{10}: nosetests {posargs} tests/contrib/gevent From d97830251b06b39d07dce512dd2ac1fb9d8e0704 Mon Sep 17 00:00:00 2001 From: Will Gittoes Date: Thu, 8 Mar 2018 17:36:12 -0500 Subject: [PATCH 2/4] [falcon] Forgot to add test file! --- .../falcon/test_distributed_tracing.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/contrib/falcon/test_distributed_tracing.py diff --git a/tests/contrib/falcon/test_distributed_tracing.py b/tests/contrib/falcon/test_distributed_tracing.py new file mode 100644 index 00000000000..d5ae73bcd33 --- /dev/null +++ b/tests/contrib/falcon/test_distributed_tracing.py @@ -0,0 +1,39 @@ +from ddtrace.propagation.http import HTTPPropagator +from ddtrace.ext import errors as errx, http as httpx, AppTypes +from falcon import testing +from nose.tools import eq_, ok_ +from tests.test_tracer import get_dummy_tracer + +from .app import get_app + + +class DistributedTracingTestCase(testing.TestCase): + """Executes tests using the manual instrumentation so a middleware + is explicitly added. + """ + + def setUp(self): + super(DistributedTracingTestCase, self).setUp() + self._service = 'falcon' + self.tracer = get_dummy_tracer() + self.api = get_app(tracer=self.tracer, distributed_tracing=True) + + def test_has_parent_span(self): + headers = {} + root_tracer = get_dummy_tracer() + root_tracer.set_service_info('root', 'root', AppTypes.web) + with root_tracer.trace('root') as root: + propagator = HTTPPropagator() + propagator.inject(root.context, headers) + out = self.simulate_get('/200', headers=headers) + eq_(out.status_code, 200) + eq_(out.content.decode('utf-8'), 'Success') + + traces = self.tracer.writer.pop_traces() + + eq_(len(traces), 1) + eq_(len(traces[0]), 1) + + eq_(traces[0][0].parent_id, root.span_id) + eq_(traces[0][0].trace_id, root.trace_id) + From cb0efb0d12284015a6cdefdf744e1ad666e33fc7 Mon Sep 17 00:00:00 2001 From: Will Gittoes Date: Mon, 19 Mar 2018 12:00:29 +1100 Subject: [PATCH 3/4] [falcon] Use iteritems in py2 and py3. Hardcode headers in tests to reduce fragility --- ddtrace/contrib/falcon/middleware.py | 3 +- .../falcon/test_distributed_tracing.py | 36 +++++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/ddtrace/contrib/falcon/middleware.py b/ddtrace/contrib/falcon/middleware.py index 8280c4a40f4..b110dc371e6 100644 --- a/ddtrace/contrib/falcon/middleware.py +++ b/ddtrace/contrib/falcon/middleware.py @@ -3,6 +3,7 @@ from ddtrace import tracer from ddtrace.ext import http as httpx from ddtrace.propagation.http import HTTPPropagator +from ...compat import iteritems from ...ext import AppTypes @@ -24,7 +25,7 @@ def __init__(self, tracer, service="falcon", distributed_tracing=False): def process_request(self, req, resp): if self._distributed_tracing: # Falcon uppercases all header names. - headers = dict((k.lower(), v) for k, v in req.headers.items()) + headers = dict((k.lower(), v) for k, v in iteritems(req.headers)) propagator = HTTPPropagator() context = propagator.extract(headers) self.tracer.context_provider.activate(context) diff --git a/tests/contrib/falcon/test_distributed_tracing.py b/tests/contrib/falcon/test_distributed_tracing.py index d5ae73bcd33..02fb870a3e1 100644 --- a/tests/contrib/falcon/test_distributed_tracing.py +++ b/tests/contrib/falcon/test_distributed_tracing.py @@ -18,22 +18,38 @@ def setUp(self): self.tracer = get_dummy_tracer() self.api = get_app(tracer=self.tracer, distributed_tracing=True) - def test_has_parent_span(self): - headers = {} - root_tracer = get_dummy_tracer() - root_tracer.set_service_info('root', 'root', AppTypes.web) - with root_tracer.trace('root') as root: - propagator = HTTPPropagator() - propagator.inject(root.context, headers) + def test_distributred_tracing(self): + headers = { + 'x-datadog-trace-id': '100', + 'x-datadog-parent-id': '42', + } out = self.simulate_get('/200', headers=headers) eq_(out.status_code, 200) eq_(out.content.decode('utf-8'), 'Success') traces = self.tracer.writer.pop_traces() - + eq_(len(traces), 1) eq_(len(traces[0]), 1) - eq_(traces[0][0].parent_id, root.span_id) - eq_(traces[0][0].trace_id, root.trace_id) + eq_(traces[0][0].parent_id, 42) + eq_(traces[0][0].trace_id, 100) + + def test_distributred_tracing_disabled(self): + self.tracer = get_dummy_tracer() + self.api = get_app(tracer=self.tracer) + headers = { + 'x-datadog-trace-id': '100', + 'x-datadog-parent-id': '42', + } + out = self.simulate_get('/200', headers=headers) + eq_(out.status_code, 200) + eq_(out.content.decode('utf-8'), 'Success') + + traces = self.tracer.writer.pop_traces() + + eq_(len(traces), 1) + eq_(len(traces[0]), 1) + ok_(traces[0][0].parent_id is not 42) + ok_(traces[0][0].trace_id is not 100) From 3f5b0a342dd472cb4ce2b5d330dcd498a3994e7b Mon Sep 17 00:00:00 2001 From: Will Gittoes Date: Tue, 20 Mar 2018 10:19:57 +1100 Subject: [PATCH 4/4] [falcon] Fix lint errors --- ddtrace/contrib/falcon/middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ddtrace/contrib/falcon/middleware.py b/ddtrace/contrib/falcon/middleware.py index b110dc371e6..d614e9125b8 100644 --- a/ddtrace/contrib/falcon/middleware.py +++ b/ddtrace/contrib/falcon/middleware.py @@ -1,6 +1,5 @@ import sys -from ddtrace import tracer from ddtrace.ext import http as httpx from ddtrace.propagation.http import HTTPPropagator from ...compat import iteritems