From 162a7dc5b05db832a5211f57277520316943e6f0 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Fri, 10 May 2019 12:51:38 +0200 Subject: [PATCH 1/9] Fix module file --- instana/instrumentation/tornado/{__init___.py => __init__.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename instana/instrumentation/tornado/{__init___.py => __init__.py} (100%) diff --git a/instana/instrumentation/tornado/__init___.py b/instana/instrumentation/tornado/__init__.py similarity index 100% rename from instana/instrumentation/tornado/__init___.py rename to instana/instrumentation/tornado/__init__.py From e09c22eb01481ade6b1eebae88292fafbd94b7e1 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Fri, 10 May 2019 12:52:27 +0200 Subject: [PATCH 2/9] Support older tornado on Python 2.7 --- instana/__init__.py | 4 ++-- instana/instrumentation/tornado/client.py | 20 ++++++++------------ instana/instrumentation/tornado/server.py | 18 ++++++++---------- instana/singletons.py | 13 ++++++++----- 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/instana/__init__.py b/instana/__init__.py index d84b26f1..d902d3ce 100644 --- a/instana/__init__.py +++ b/instana/__init__.py @@ -71,8 +71,8 @@ def boot_agent(): from .instrumentation.aiohttp import client from .instrumentation.aiohttp import server from .instrumentation import asynqp - from .instrumentation.tornado import client - from .instrumentation.tornado import server + from .instrumentation.tornado import client + from .instrumentation.tornado import server from .instrumentation import logging from .instrumentation import mysqlpython from .instrumentation import redis diff --git a/instana/instrumentation/tornado/client.py b/instana/instrumentation/tornado/client.py index efc73c1e..92687747 100644 --- a/instana/instrumentation/tornado/client.py +++ b/instana/instrumentation/tornado/client.py @@ -3,8 +3,6 @@ import opentracing import wrapt import functools -import basictracer -import sys from ...log import logger from ...singletons import agent, tornado_tracer @@ -12,15 +10,13 @@ from distutils.version import LooseVersion +try: + import tornado -# Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing -# scope managers which we will tackle soon. -# Limit Tornado version for the time being. -if (('tornado' in sys.modules) and - hasattr(sys.modules['tornado'], 'version') and - (LooseVersion(sys.modules['tornado'].version) < LooseVersion('6.0.0'))): - try: - import tornado + # Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing + # scope managers which we will tackle soon. + # Limit Tornado version for the time being. + if hasattr(tornado, 'version') and (LooseVersion(tornado.version) < LooseVersion('6.0.0')): @wrapt.patch_function_wrapper('tornado.httpclient', 'AsyncHTTPClient.fetch') def fetch_with_instana(wrapped, instance, argv, kwargs): @@ -81,6 +77,6 @@ def finish_tracing(future, scope): logger.debug("Instrumenting tornado client") - except ImportError: - pass +except ImportError: + pass diff --git a/instana/instrumentation/tornado/server.py b/instana/instrumentation/tornado/server.py index 452ce6a5..bf8a1fb9 100644 --- a/instana/instrumentation/tornado/server.py +++ b/instana/instrumentation/tornado/server.py @@ -11,15 +11,13 @@ from distutils.version import LooseVersion -# Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing -# scope managers which we will tackle soon. -# Limit Tornado version for the time being. -if (('tornado' in sys.modules) and - hasattr(sys.modules['tornado'], 'version') and - (LooseVersion(sys.modules['tornado'].version) < LooseVersion('6.0.0'))): +try: + import tornado - try: - import tornado + # Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing + # scope managers which we will tackle soon. + # Limit Tornado version for the time being. + if hasattr(tornado, 'version') and (LooseVersion(tornado.version) < LooseVersion('6.0.0')): @wrapt.patch_function_wrapper('tornado.web', 'RequestHandler._execute') def execute_with_instana(wrapped, instance, argv, kwargs): @@ -104,6 +102,6 @@ def log_exception_with_instana(wrapped, instance, argv, kwargs): logger.debug("tornado log_exception", exc_info=True) logger.debug("Instrumenting tornado server") - except ImportError: - pass +except ImportError: + pass diff --git a/instana/singletons.py b/instana/singletons.py index 9cdc6801..b31b62a6 100644 --- a/instana/singletons.py +++ b/instana/singletons.py @@ -1,8 +1,11 @@ import sys import opentracing -from .agent import Agent # noqa -from .tracer import InstanaTracer # noqa +from .agent import Agent +from .tracer import InstanaTracer + +from distutils.version import LooseVersion + # The Instana Agent which carries along with it a Sensor that collects metrics. agent = Agent() @@ -20,10 +23,10 @@ if sys.version_info >= (3,4): from opentracing.scope_managers.asyncio import AsyncioScopeManager - from opentracing.scope_managers.tornado import TornadoScopeManager - async_tracer = InstanaTracer(scope_manager=AsyncioScopeManager()) - tornado_tracer = InstanaTracer(scope_manager=TornadoScopeManager()) + +from opentracing.scope_managers.tornado import TornadoScopeManager +tornado_tracer = InstanaTracer(scope_manager=TornadoScopeManager()) # Set ourselves as the tracer. opentracing.tracer = tracer From b32c0a20a8a543db3c3a90161c94f9047bb72fce Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Fri, 10 May 2019 16:34:54 +0200 Subject: [PATCH 3/9] Run tracing within stack context --- instana/instrumentation/tornado/server.py | 32 +++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/instana/instrumentation/tornado/server.py b/instana/instrumentation/tornado/server.py index bf8a1fb9..33866513 100644 --- a/instana/instrumentation/tornado/server.py +++ b/instana/instrumentation/tornado/server.py @@ -22,25 +22,26 @@ @wrapt.patch_function_wrapper('tornado.web', 'RequestHandler._execute') def execute_with_instana(wrapped, instance, argv, kwargs): try: - ctx = tornado_tracer.extract(opentracing.Format.HTTP_HEADERS, instance.request.headers) - scope = tornado_tracer.start_active_span('tornado-server', child_of=ctx) + with tracer_stack_context(): - # Query param scrubbing - if instance.request.query is not None and len(instance.request.query) > 0: - cleaned_qp = strip_secrets(instance.request.query, agent.secrets_matcher, agent.secrets_list) - scope.span.set_tag("http.params", cleaned_qp) + ctx = tornado_tracer.extract(opentracing.Format.HTTP_HEADERS, instance.request.headers) + scope = tornado_tracer.start_active_span('tornado-server', child_of=ctx) - scope.span.set_tag("http.host", instance.request.host) - scope.span.set_tag("http.method", instance.request.method) - scope.span.set_tag("http.path", instance.request.path) + # Query param scrubbing + if instance.request.query is not None and len(instance.request.query) > 0: + cleaned_qp = strip_secrets(instance.request.query, agent.secrets_matcher, agent.secrets_list) + scope.span.set_tag("http.params", cleaned_qp) - # Custom header tracking support - if agent.extra_headers is not None: - for custom_header in agent.extra_headers: - if custom_header in instance.request.headers: - scope.span.set_tag("http.%s" % custom_header, instance.request.headers[custom_header]) + scope.span.set_tag("http.host", instance.request.host) + scope.span.set_tag("http.method", instance.request.method) + scope.span.set_tag("http.path", instance.request.path) + + # Custom header tracking support + if agent.extra_headers is not None: + for custom_header in agent.extra_headers: + if custom_header in instance.request.headers: + scope.span.set_tag("http.%s" % custom_header, instance.request.headers[custom_header]) - with tracer_stack_context(): setattr(instance.request, "_instana", scope) # Set the context response headers now because tornado doesn't give us a better option to do so @@ -80,7 +81,6 @@ def on_finish_with_instana(wrapped, instance, argv, kwargs): scope.span.set_tag("ec", ec + 1) scope.span.set_tag("http.status_code", status_code) - scope.span.finish() scope.close() return wrapped(*argv, **kwargs) From b5e85192cde57913bc2924820f325a064dff7b4b Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Fri, 10 May 2019 16:35:50 +0200 Subject: [PATCH 4/9] Cleanup; Dont send empty baggage --- instana/recorder.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/instana/recorder.py b/instana/recorder.py index a47f5c92..8db62525 100644 --- a/instana/recorder.py +++ b/instana/recorder.py @@ -24,9 +24,9 @@ class InstanaRecorder(SpanRecorder): registered_spans = ("aiohttp-client", "aiohttp-server", "django", "log", "memcache", "mysql", "rabbitmq", "redis", "rpc-client", "rpc-server", "sqlalchemy", "soap", - "tornado-server", "tornado-client", "urllib3", "wsgi") - http_spans = ("aiohttp-client", "aiohttp-server", "django", "http", "soap", "tornado-server", - "tornado-client", "urllib3", "wsgi") + "tornado-client", "tornado-server", "urllib3", "wsgi") + http_spans = ("aiohttp-client", "aiohttp-server", "django", "http", "soap", "tornado-client", + "tornado-server", "urllib3", "wsgi") exit_spans = ("aiohttp-client", "log", "memcache", "mysql", "rabbitmq", "redis", "rpc-client", "sqlalchemy", "soap", "tornado-client", "urllib3") @@ -100,7 +100,9 @@ def record_span(self, span): def build_registered_span(self, span): """ Takes a BasicSpan and converts it into a registered JsonSpan """ - data = Data(baggage=span.context.baggage) + data = Data() + if len(span.context.baggage) > 0: + data.baggage = span.context.baggage kind = 1 # entry if span.operation_name in self.exit_spans: From 21f20cb989f14c41b2e7f814c8bdd59b0af412ac Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Fri, 10 May 2019 16:36:44 +0200 Subject: [PATCH 5/9] Use a dedicated handle_fork method --- instana/meter.py | 3 +++ instana/sensor.py | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/instana/meter.py b/instana/meter.py index b7d2cd6d..9b619afd 100644 --- a/instana/meter.py +++ b/instana/meter.py @@ -136,6 +136,9 @@ def reset(self): self.last_collect = None self.last_metrics = None self.snapshot_countdown = 0 + + def handle_fork(self): + self.reset() self.run() def collect_and_report(self): diff --git a/instana/sensor.py b/instana/sensor.py index 1fa96af3..d417aa7a 100644 --- a/instana/sensor.py +++ b/instana/sensor.py @@ -24,7 +24,8 @@ def set_options(self, options): self.options = Options() def handle_fork(self): - self.meter.reset() + # Nothing to do for the Sensor; Pass onto Meter + self.meter.handle_fork() global_sensor = None From a402b1f4998d8239c5172bb1148a846f80b1f2f2 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Fri, 10 May 2019 17:00:05 +0200 Subject: [PATCH 6/9] Report url instead of separated host & path --- instana/instrumentation/tornado/server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instana/instrumentation/tornado/server.py b/instana/instrumentation/tornado/server.py index 33866513..be11df0a 100644 --- a/instana/instrumentation/tornado/server.py +++ b/instana/instrumentation/tornado/server.py @@ -32,9 +32,9 @@ def execute_with_instana(wrapped, instance, argv, kwargs): cleaned_qp = strip_secrets(instance.request.query, agent.secrets_matcher, agent.secrets_list) scope.span.set_tag("http.params", cleaned_qp) - scope.span.set_tag("http.host", instance.request.host) + url = "%s://%s%s" % (instance.request.protocol, instance.request.host, instance.request.path) + scope.span.set_tag("http.url", url) scope.span.set_tag("http.method", instance.request.method) - scope.span.set_tag("http.path", instance.request.path) # Custom header tracking support if agent.extra_headers is not None: From 9dd1dd52af37e62481debe2985cd5f46dca61bcc Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Mon, 13 May 2019 11:08:18 +0200 Subject: [PATCH 7/9] Also report which handler is handling the request --- instana/instrumentation/tornado/server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instana/instrumentation/tornado/server.py b/instana/instrumentation/tornado/server.py index be11df0a..8f88f298 100644 --- a/instana/instrumentation/tornado/server.py +++ b/instana/instrumentation/tornado/server.py @@ -3,7 +3,6 @@ import opentracing from opentracing.scope_managers.tornado import tracer_stack_context import wrapt -import sys from ...log import logger from ...singletons import agent, tornado_tracer @@ -23,7 +22,6 @@ def execute_with_instana(wrapped, instance, argv, kwargs): try: with tracer_stack_context(): - ctx = tornado_tracer.extract(opentracing.Format.HTTP_HEADERS, instance.request.headers) scope = tornado_tracer.start_active_span('tornado-server', child_of=ctx) @@ -36,6 +34,8 @@ def execute_with_instana(wrapped, instance, argv, kwargs): scope.span.set_tag("http.url", url) scope.span.set_tag("http.method", instance.request.method) + scope.span.set_tag("handler", instance.__class__.__name__) + # Custom header tracking support if agent.extra_headers is not None: for custom_header in agent.extra_headers: From ebe39eca879243c803f50989679d5ae44992e21c Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Mon, 13 May 2019 11:22:50 +0200 Subject: [PATCH 8/9] Update client test to follow server changes. --- tests/test_tornado_client.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/tests/test_tornado_client.py b/tests/test_tornado_client.py index a794499b..32dad785 100644 --- a/tests/test_tornado_client.py +++ b/tests/test_tornado_client.py @@ -64,8 +64,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(200, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url) self.assertIsNone(server_span.data.http.params) self.assertEqual("GET", server_span.data.http.method) self.assertIsNotNone(server_span.stack) @@ -125,8 +124,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(200, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url) self.assertIsNone(server_span.data.http.params) self.assertEqual("POST", server_span.data.http.method) self.assertIsNotNone(server_span.stack) @@ -190,8 +188,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(200, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url) self.assertIsNone(server_span.data.http.params) self.assertEqual("GET", server_span.data.http.method) self.assertIsNotNone(server_span.stack) @@ -200,8 +197,7 @@ async def test(): self.assertEqual("tornado-server", server301_span.n) self.assertEqual(301, server301_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server301_span.data.http.host) - self.assertEqual("/301", server301_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/301", server301_span.data.http.url) self.assertIsNone(server301_span.data.http.params) self.assertEqual("GET", server301_span.data.http.method) self.assertIsNotNone(server301_span.stack) @@ -264,8 +260,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(405, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/405", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/405", server_span.data.http.url) self.assertIsNone(server_span.data.http.params) self.assertEqual("GET", server_span.data.http.method) self.assertIsNotNone(server_span.stack) @@ -328,8 +323,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(500, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/500", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/500", server_span.data.http.url) self.assertIsNone(server_span.data.http.params) self.assertEqual("GET", server_span.data.http.method) self.assertIsNotNone(server_span.stack) @@ -392,8 +386,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(504, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/504", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/504", server_span.data.http.url) self.assertIsNone(server_span.data.http.params) self.assertEqual("GET", server_span.data.http.method) self.assertIsNotNone(server_span.stack) @@ -453,8 +446,7 @@ async def test(): self.assertEqual("tornado-server", server_span.n) self.assertEqual(200, server_span.data.http.status) - self.assertEqual("127.0.0.1:4133", server_span.data.http.host) - self.assertEqual("/", server_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url) self.assertEqual('secret=', server_span.data.http.params) self.assertEqual("GET", server_span.data.http.method) self.assertIsNotNone(server_span.stack) From f2880ca3ad9550315c63a67885846fade8737a7d Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Mon, 13 May 2019 11:32:11 +0200 Subject: [PATCH 9/9] Update server tests --- tests/test_tornado_server.py | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/tests/test_tornado_server.py b/tests/test_tornado_server.py index 6a5d4ef7..6678b8c0 100644 --- a/tests/test_tornado_server.py +++ b/tests/test_tornado_server.py @@ -78,8 +78,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(200, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url) self.assertIsNone(tornado_span.data.http.params) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) @@ -139,8 +138,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(200, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url) self.assertIsNone(tornado_span.data.http.params) self.assertEqual("POST", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) @@ -205,8 +203,7 @@ async def test(): self.assertEqual("tornado-server", tornado_301_span.n) self.assertEqual(301, tornado_301_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_301_span.data.http.host) - self.assertEqual("/301", tornado_301_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/301", tornado_301_span.data.http.url) self.assertIsNone(tornado_span.data.http.params) self.assertEqual("GET", tornado_301_span.data.http.method) self.assertIsNotNone(tornado_301_span.stack) @@ -215,8 +212,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(200, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) self.assertTrue(type(tornado_span.stack) is list) @@ -275,8 +271,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(405, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/405", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/405", tornado_span.data.http.url) self.assertIsNone(tornado_span.data.http.params) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) @@ -336,8 +331,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(500, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/500", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/500", tornado_span.data.http.url) self.assertIsNone(tornado_span.data.http.params) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) @@ -398,8 +392,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(504, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/504", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/504", tornado_span.data.http.url) self.assertIsNone(tornado_span.data.http.params) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) @@ -460,8 +453,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(200, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url) self.assertEqual("secret=", tornado_span.data.http.params) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack) @@ -529,8 +521,7 @@ async def test(): self.assertEqual("tornado-server", tornado_span.n) self.assertEqual(200, tornado_span.data.http.status) - self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host) - self.assertEqual("/", tornado_span.data.http.path) + self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url) self.assertEqual("secret=", tornado_span.data.http.params) self.assertEqual("GET", tornado_span.data.http.method) self.assertIsNotNone(tornado_span.stack)