Skip to content

Commit 7232787

Browse files
pglombardopglombardo
authored
Tornado: Polishing & Bug Fixes (#150)
* Fix module file * Support older tornado on Python 2.7 * Run tracing within stack context * Cleanup; Dont send empty baggage * Use a dedicated handle_fork method * Report url instead of separated host & path * Also report which handler is handling the request * Update client test to follow server changes. * Update server tests
1 parent 4519fed commit 7232787

File tree

10 files changed

+71
-85
lines changed

10 files changed

+71
-85
lines changed

‎instana/__init__.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ def boot_agent():
7171
from .instrumentation.aiohttp import client
7272
from .instrumentation.aiohttp import server
7373
from .instrumentation import asynqp
74-
from .instrumentation.tornado import client
75-
from .instrumentation.tornado import server
74+
from .instrumentation.tornado import client
75+
from .instrumentation.tornado import server
7676
from .instrumentation import logging
7777
from .instrumentation import mysqlpython
7878
from .instrumentation import redis

‎instana/instrumentation/tornado/__init___.py‎ renamed to ‎instana/instrumentation/tornado/__init__.py‎

File renamed without changes.

‎instana/instrumentation/tornado/client.py‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,20 @@
33
import opentracing
44
import wrapt
55
import functools
6-
import basictracer
7-
import sys
86

97
from ...log import logger
108
from ...singletons import agent, tornado_tracer
119
from ...util import strip_secrets
1210

1311
from distutils.version import LooseVersion
1412

13+
try:
14+
import tornado
1515

16-
# Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing
17-
# scope managers which we will tackle soon.
18-
# Limit Tornado version for the time being.
19-
if (('tornado' in sys.modules) and
20-
hasattr(sys.modules['tornado'], 'version') and
21-
(LooseVersion(sys.modules['tornado'].version) < LooseVersion('6.0.0'))):
22-
try:
23-
import tornado
16+
# Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing
17+
# scope managers which we will tackle soon.
18+
# Limit Tornado version for the time being.
19+
if hasattr(tornado, 'version') and (LooseVersion(tornado.version) < LooseVersion('6.0.0')):
2420

2521
@wrapt.patch_function_wrapper('tornado.httpclient', 'AsyncHTTPClient.fetch')
2622
def fetch_with_instana(wrapped, instance, argv, kwargs):
@@ -81,6 +77,6 @@ def finish_tracing(future, scope):
8177

8278

8379
logger.debug("Instrumenting tornado client")
84-
except ImportError:
85-
pass
80+
except ImportError:
81+
pass
8682

‎instana/instrumentation/tornado/server.py‎

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,46 +3,45 @@
33
import opentracing
44
from opentracing.scope_managers.tornado import tracer_stack_context
55
import wrapt
6-
import sys
76

87
from ...log import logger
98
from ...singletons import agent, tornado_tracer
109
from ...util import strip_secrets
1110

1211
from distutils.version import LooseVersion
1312

14-
# Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing
15-
# scope managers which we will tackle soon.
16-
# Limit Tornado version for the time being.
17-
if (('tornado' in sys.modules) and
18-
hasattr(sys.modules['tornado'], 'version') and
19-
(LooseVersion(sys.modules['tornado'].version) < LooseVersion('6.0.0'))):
13+
try:
14+
import tornado
2015

21-
try:
22-
import tornado
16+
# Tornado >=6.0 switched to contextvars for context management. This requires changes to the opentracing
17+
# scope managers which we will tackle soon.
18+
# Limit Tornado version for the time being.
19+
if hasattr(tornado, 'version') and (LooseVersion(tornado.version) < LooseVersion('6.0.0')):
2320

2421
@wrapt.patch_function_wrapper('tornado.web', 'RequestHandler._execute')
2522
def execute_with_instana(wrapped, instance, argv, kwargs):
2623
try:
27-
ctx = tornado_tracer.extract(opentracing.Format.HTTP_HEADERS, instance.request.headers)
28-
scope = tornado_tracer.start_active_span('tornado-server', child_of=ctx)
24+
with tracer_stack_context():
25+
ctx = tornado_tracer.extract(opentracing.Format.HTTP_HEADERS, instance.request.headers)
26+
scope = tornado_tracer.start_active_span('tornado-server', child_of=ctx)
2927

30-
# Query param scrubbing
31-
if instance.request.query is not None and len(instance.request.query) > 0:
32-
cleaned_qp = strip_secrets(instance.request.query, agent.secrets_matcher, agent.secrets_list)
33-
scope.span.set_tag("http.params", cleaned_qp)
28+
# Query param scrubbing
29+
if instance.request.query is not None and len(instance.request.query) > 0:
30+
cleaned_qp = strip_secrets(instance.request.query, agent.secrets_matcher, agent.secrets_list)
31+
scope.span.set_tag("http.params", cleaned_qp)
3432

35-
scope.span.set_tag("http.host", instance.request.host)
36-
scope.span.set_tag("http.method", instance.request.method)
37-
scope.span.set_tag("http.path", instance.request.path)
33+
url = "%s://%s%s" % (instance.request.protocol, instance.request.host, instance.request.path)
34+
scope.span.set_tag("http.url", url)
35+
scope.span.set_tag("http.method", instance.request.method)
3836

39-
# Custom header tracking support
40-
if agent.extra_headers is not None:
41-
for custom_header in agent.extra_headers:
42-
if custom_header in instance.request.headers:
43-
scope.span.set_tag("http.%s" % custom_header, instance.request.headers[custom_header])
37+
scope.span.set_tag("handler", instance.__class__.__name__)
38+
39+
# Custom header tracking support
40+
if agent.extra_headers is not None:
41+
for custom_header in agent.extra_headers:
42+
if custom_header in instance.request.headers:
43+
scope.span.set_tag("http.%s" % custom_header, instance.request.headers[custom_header])
4444

45-
with tracer_stack_context():
4645
setattr(instance.request, "_instana", scope)
4746

4847
# Set the context response headers now because tornado doesn't give us a better option to do so
@@ -82,7 +81,6 @@ def on_finish_with_instana(wrapped, instance, argv, kwargs):
8281
scope.span.set_tag("ec", ec + 1)
8382

8483
scope.span.set_tag("http.status_code", status_code)
85-
scope.span.finish()
8684
scope.close()
8785

8886
return wrapped(*argv, **kwargs)
@@ -104,6 +102,6 @@ def log_exception_with_instana(wrapped, instance, argv, kwargs):
104102
logger.debug("tornado log_exception", exc_info=True)
105103

106104
logger.debug("Instrumenting tornado server")
107-
except ImportError:
108-
pass
105+
except ImportError:
106+
pass
109107

‎instana/meter.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ def reset(self):
136136
self.last_collect = None
137137
self.last_metrics = None
138138
self.snapshot_countdown = 0
139+
140+
def handle_fork(self):
141+
self.reset()
139142
self.run()
140143

141144
def collect_and_report(self):

‎instana/recorder.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
class InstanaRecorder(SpanRecorder):
2525
registered_spans = ("aiohttp-client", "aiohttp-server", "django", "log", "memcache", "mysql",
2626
"rabbitmq", "redis", "rpc-client", "rpc-server", "sqlalchemy", "soap",
27-
"tornado-server", "tornado-client", "urllib3", "wsgi")
28-
http_spans = ("aiohttp-client", "aiohttp-server", "django", "http", "soap", "tornado-server",
29-
"tornado-client", "urllib3", "wsgi")
27+
"tornado-client", "tornado-server", "urllib3", "wsgi")
28+
http_spans = ("aiohttp-client", "aiohttp-server", "django", "http", "soap", "tornado-client",
29+
"tornado-server", "urllib3", "wsgi")
3030

3131
exit_spans = ("aiohttp-client", "log", "memcache", "mysql", "rabbitmq", "redis", "rpc-client",
3232
"sqlalchemy", "soap", "tornado-client", "urllib3")
@@ -100,7 +100,9 @@ def record_span(self, span):
100100

101101
def build_registered_span(self, span):
102102
""" Takes a BasicSpan and converts it into a registered JsonSpan """
103-
data = Data(baggage=span.context.baggage)
103+
data = Data()
104+
if len(span.context.baggage) > 0:
105+
data.baggage = span.context.baggage
104106

105107
kind = 1 # entry
106108
if span.operation_name in self.exit_spans:

‎instana/sensor.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ def set_options(self, options):
2424
self.options = Options()
2525

2626
def handle_fork(self):
27-
self.meter.reset()
27+
# Nothing to do for the Sensor; Pass onto Meter
28+
self.meter.handle_fork()
2829

2930

3031
global_sensor = None

‎instana/singletons.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import sys
22
import opentracing
33

4-
from .agent import Agent # noqa
5-
from .tracer import InstanaTracer # noqa
4+
from .agent import Agent
5+
from .tracer import InstanaTracer
6+
7+
from distutils.version import LooseVersion
8+
69

710
# The Instana Agent which carries along with it a Sensor that collects metrics.
811
agent = Agent()
@@ -20,10 +23,10 @@
2023

2124
if sys.version_info >= (3,4):
2225
from opentracing.scope_managers.asyncio import AsyncioScopeManager
23-
from opentracing.scope_managers.tornado import TornadoScopeManager
24-
2526
async_tracer = InstanaTracer(scope_manager=AsyncioScopeManager())
26-
tornado_tracer = InstanaTracer(scope_manager=TornadoScopeManager())
27+
28+
from opentracing.scope_managers.tornado import TornadoScopeManager
29+
tornado_tracer = InstanaTracer(scope_manager=TornadoScopeManager())
2730

2831
# Set ourselves as the tracer.
2932
opentracing.tracer = tracer

‎tests/test_tornado_client.py‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ async def test():
6464

6565
self.assertEqual("tornado-server", server_span.n)
6666
self.assertEqual(200, server_span.data.http.status)
67-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
68-
self.assertEqual("/", server_span.data.http.path)
67+
self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url)
6968
self.assertIsNone(server_span.data.http.params)
7069
self.assertEqual("GET", server_span.data.http.method)
7170
self.assertIsNotNone(server_span.stack)
@@ -125,8 +124,7 @@ async def test():
125124

126125
self.assertEqual("tornado-server", server_span.n)
127126
self.assertEqual(200, server_span.data.http.status)
128-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
129-
self.assertEqual("/", server_span.data.http.path)
127+
self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url)
130128
self.assertIsNone(server_span.data.http.params)
131129
self.assertEqual("POST", server_span.data.http.method)
132130
self.assertIsNotNone(server_span.stack)
@@ -190,8 +188,7 @@ async def test():
190188

191189
self.assertEqual("tornado-server", server_span.n)
192190
self.assertEqual(200, server_span.data.http.status)
193-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
194-
self.assertEqual("/", server_span.data.http.path)
191+
self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url)
195192
self.assertIsNone(server_span.data.http.params)
196193
self.assertEqual("GET", server_span.data.http.method)
197194
self.assertIsNotNone(server_span.stack)
@@ -200,8 +197,7 @@ async def test():
200197

201198
self.assertEqual("tornado-server", server301_span.n)
202199
self.assertEqual(301, server301_span.data.http.status)
203-
self.assertEqual("127.0.0.1:4133", server301_span.data.http.host)
204-
self.assertEqual("/301", server301_span.data.http.path)
200+
self.assertEqual("http://127.0.0.1:4133/301", server301_span.data.http.url)
205201
self.assertIsNone(server301_span.data.http.params)
206202
self.assertEqual("GET", server301_span.data.http.method)
207203
self.assertIsNotNone(server301_span.stack)
@@ -264,8 +260,7 @@ async def test():
264260

265261
self.assertEqual("tornado-server", server_span.n)
266262
self.assertEqual(405, server_span.data.http.status)
267-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
268-
self.assertEqual("/405", server_span.data.http.path)
263+
self.assertEqual("http://127.0.0.1:4133/405", server_span.data.http.url)
269264
self.assertIsNone(server_span.data.http.params)
270265
self.assertEqual("GET", server_span.data.http.method)
271266
self.assertIsNotNone(server_span.stack)
@@ -328,8 +323,7 @@ async def test():
328323

329324
self.assertEqual("tornado-server", server_span.n)
330325
self.assertEqual(500, server_span.data.http.status)
331-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
332-
self.assertEqual("/500", server_span.data.http.path)
326+
self.assertEqual("http://127.0.0.1:4133/500", server_span.data.http.url)
333327
self.assertIsNone(server_span.data.http.params)
334328
self.assertEqual("GET", server_span.data.http.method)
335329
self.assertIsNotNone(server_span.stack)
@@ -392,8 +386,7 @@ async def test():
392386

393387
self.assertEqual("tornado-server", server_span.n)
394388
self.assertEqual(504, server_span.data.http.status)
395-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
396-
self.assertEqual("/504", server_span.data.http.path)
389+
self.assertEqual("http://127.0.0.1:4133/504", server_span.data.http.url)
397390
self.assertIsNone(server_span.data.http.params)
398391
self.assertEqual("GET", server_span.data.http.method)
399392
self.assertIsNotNone(server_span.stack)
@@ -453,8 +446,7 @@ async def test():
453446

454447
self.assertEqual("tornado-server", server_span.n)
455448
self.assertEqual(200, server_span.data.http.status)
456-
self.assertEqual("127.0.0.1:4133", server_span.data.http.host)
457-
self.assertEqual("/", server_span.data.http.path)
449+
self.assertEqual("http://127.0.0.1:4133/", server_span.data.http.url)
458450
self.assertEqual('secret=<redacted>', server_span.data.http.params)
459451
self.assertEqual("GET", server_span.data.http.method)
460452
self.assertIsNotNone(server_span.stack)

‎tests/test_tornado_server.py‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@ async def test():
7878

7979
self.assertEqual("tornado-server", tornado_span.n)
8080
self.assertEqual(200, tornado_span.data.http.status)
81-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
82-
self.assertEqual("/", tornado_span.data.http.path)
81+
self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url)
8382
self.assertIsNone(tornado_span.data.http.params)
8483
self.assertEqual("GET", tornado_span.data.http.method)
8584
self.assertIsNotNone(tornado_span.stack)
@@ -139,8 +138,7 @@ async def test():
139138

140139
self.assertEqual("tornado-server", tornado_span.n)
141140
self.assertEqual(200, tornado_span.data.http.status)
142-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
143-
self.assertEqual("/", tornado_span.data.http.path)
141+
self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url)
144142
self.assertIsNone(tornado_span.data.http.params)
145143
self.assertEqual("POST", tornado_span.data.http.method)
146144
self.assertIsNotNone(tornado_span.stack)
@@ -205,8 +203,7 @@ async def test():
205203

206204
self.assertEqual("tornado-server", tornado_301_span.n)
207205
self.assertEqual(301, tornado_301_span.data.http.status)
208-
self.assertEqual("127.0.0.1:4133", tornado_301_span.data.http.host)
209-
self.assertEqual("/301", tornado_301_span.data.http.path)
206+
self.assertEqual("http://127.0.0.1:4133/301", tornado_301_span.data.http.url)
210207
self.assertIsNone(tornado_span.data.http.params)
211208
self.assertEqual("GET", tornado_301_span.data.http.method)
212209
self.assertIsNotNone(tornado_301_span.stack)
@@ -215,8 +212,7 @@ async def test():
215212

216213
self.assertEqual("tornado-server", tornado_span.n)
217214
self.assertEqual(200, tornado_span.data.http.status)
218-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
219-
self.assertEqual("/", tornado_span.data.http.path)
215+
self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url)
220216
self.assertEqual("GET", tornado_span.data.http.method)
221217
self.assertIsNotNone(tornado_span.stack)
222218
self.assertTrue(type(tornado_span.stack) is list)
@@ -275,8 +271,7 @@ async def test():
275271

276272
self.assertEqual("tornado-server", tornado_span.n)
277273
self.assertEqual(405, tornado_span.data.http.status)
278-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
279-
self.assertEqual("/405", tornado_span.data.http.path)
274+
self.assertEqual("http://127.0.0.1:4133/405", tornado_span.data.http.url)
280275
self.assertIsNone(tornado_span.data.http.params)
281276
self.assertEqual("GET", tornado_span.data.http.method)
282277
self.assertIsNotNone(tornado_span.stack)
@@ -336,8 +331,7 @@ async def test():
336331

337332
self.assertEqual("tornado-server", tornado_span.n)
338333
self.assertEqual(500, tornado_span.data.http.status)
339-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
340-
self.assertEqual("/500", tornado_span.data.http.path)
334+
self.assertEqual("http://127.0.0.1:4133/500", tornado_span.data.http.url)
341335
self.assertIsNone(tornado_span.data.http.params)
342336
self.assertEqual("GET", tornado_span.data.http.method)
343337
self.assertIsNotNone(tornado_span.stack)
@@ -398,8 +392,7 @@ async def test():
398392

399393
self.assertEqual("tornado-server", tornado_span.n)
400394
self.assertEqual(504, tornado_span.data.http.status)
401-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
402-
self.assertEqual("/504", tornado_span.data.http.path)
395+
self.assertEqual("http://127.0.0.1:4133/504", tornado_span.data.http.url)
403396
self.assertIsNone(tornado_span.data.http.params)
404397
self.assertEqual("GET", tornado_span.data.http.method)
405398
self.assertIsNotNone(tornado_span.stack)
@@ -460,8 +453,7 @@ async def test():
460453

461454
self.assertEqual("tornado-server", tornado_span.n)
462455
self.assertEqual(200, tornado_span.data.http.status)
463-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
464-
self.assertEqual("/", tornado_span.data.http.path)
456+
self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url)
465457
self.assertEqual("secret=<redacted>", tornado_span.data.http.params)
466458
self.assertEqual("GET", tornado_span.data.http.method)
467459
self.assertIsNotNone(tornado_span.stack)
@@ -529,8 +521,7 @@ async def test():
529521

530522
self.assertEqual("tornado-server", tornado_span.n)
531523
self.assertEqual(200, tornado_span.data.http.status)
532-
self.assertEqual("127.0.0.1:4133", tornado_span.data.http.host)
533-
self.assertEqual("/", tornado_span.data.http.path)
524+
self.assertEqual("http://127.0.0.1:4133/", tornado_span.data.http.url)
534525
self.assertEqual("secret=<redacted>", tornado_span.data.http.params)
535526
self.assertEqual("GET", tornado_span.data.http.method)
536527
self.assertIsNotNone(tornado_span.stack)

0 commit comments

Comments
 (0)