Skip to content

Commit bd52a15

Browse files
pglombardopglombardo
authored
Django & WSGI Improvments; More Safeties, Precision & Tests (#90)
* Add an in-test-suite background django server * Remove unused overrides; linter fixes * Drop tests on Py 3.3 * ec defaults to None * Better recording of errors in json spans * Linter fixups; Better path capture. OT 2.0 improvements * Test Flask app - use gevent pywsgi * Language/comment fixups. * New WSGI tests * Linter fixups; Add safeties; OT 2.0 improvements * Fix how Flask is run in the background * Update mysql ec tests * Remove unused package * Stronger check; Linter updates * Use same tracer everywhere
1 parent 13fb496 commit bd52a15

19 files changed

+898
-275
lines changed

‎.travis.yml

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ language: python
22

33
python:
44
- "2.7"
5-
- "3.3"
65
- "3.4"
76
- "3.5"
87
- "3.6"

‎instana/__init__.py

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
11
from __future__ import absolute_import
22

33
import os
4-
import opentracing
54
from pkg_resources import get_distribution
65

7-
from . import sensor
8-
from . import tracer
9-
10-
116
"""
127
The Instana package has two core components: the sensor and the tracer.
138
@@ -37,6 +32,7 @@ def load(module):
3732
print("Instana: Loading...")
3833
print("==========================================================")
3934

35+
4036
# Optional application wide service name.
4137
# Can be configured via environment variable or via code:
4238
#

‎instana/instrumentation/django/middleware.py

+31-26
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import wrapt
88

99
from ...log import logger
10-
from ...tracer import internal_tracer
10+
from ...tracer import internal_tracer as tracer
1111

1212
DJ_INSTANA_MIDDLEWARE = 'instana.instrumentation.django.middleware.InstanaMiddleware'
1313

@@ -26,44 +26,48 @@ def __init__(self, get_response=None):
2626
def process_request(self, request):
2727
try:
2828
env = request.environ
29+
ctx = None
2930
if 'HTTP_X_INSTANA_T' in env and 'HTTP_X_INSTANA_S' in env:
30-
ctx = internal_tracer.extract(ot.Format.HTTP_HEADERS, env)
31-
span = internal_tracer.start_span("django", child_of=ctx)
32-
else:
33-
span = internal_tracer.start_span("django")
31+
ctx = tracer.extract(ot.Format.HTTP_HEADERS, env)
32+
33+
self.scope = tracer.start_active_span('django', child_of=ctx)
3434

35-
span.set_tag(ext.HTTP_URL, env['PATH_INFO'])
36-
span.set_tag("http.params", env['QUERY_STRING'])
37-
span.set_tag(ext.HTTP_METHOD, request.method)
38-
span.set_tag("http.host", env['HTTP_HOST'])
39-
self.span = span
35+
self.scope.span.set_tag(ext.HTTP_METHOD, request.method)
36+
if 'PATH_INFO' in env:
37+
self.scope.span.set_tag(ext.HTTP_URL, env['PATH_INFO'])
38+
if 'QUERY_STRING' in env:
39+
self.scope.span.set_tag("http.params", env['QUERY_STRING'])
40+
if 'HTTP_HOST' in env:
41+
self.scope.span.set_tag("http.host", env['HTTP_HOST'])
4042
except Exception as e:
4143
logger.debug("Instana middleware @ process_response: ", e)
4244

4345
def process_response(self, request, response):
4446
try:
45-
if self.span:
47+
if self.scope is not None:
4648
if 500 <= response.status_code <= 511:
47-
self.span.set_tag("error", True)
48-
ec = self.span.tags.get('ec', 0)
49+
self.scope.span.set_tag("error", True)
50+
ec = self.scope.span.tags.get('ec', 0)
4951
if ec is 0:
50-
self.span.set_tag("ec", ec+1)
52+
self.scope.span.set_tag("ec", ec+1)
5153

52-
self.span.set_tag(ext.HTTP_STATUS_CODE, response.status_code)
53-
internal_tracer.inject(self.span.context, ot.Format.HTTP_HEADERS, response)
54-
self.span.finish()
55-
self.span = None
54+
self.scope.span.set_tag(ext.HTTP_STATUS_CODE, response.status_code)
55+
tracer.inject(self.scope.span.context, ot.Format.HTTP_HEADERS, response)
5656
except Exception as e:
5757
logger.debug("Instana middleware @ process_response: ", e)
5858
finally:
59+
self.scope.close()
60+
self.scope = None
5961
return response
6062

6163
def process_exception(self, request, exception):
62-
if self.span:
63-
self.span.log_kv({'message': exception})
64-
self.span.set_tag("error", True)
65-
ec = self.span.tags.get('ec', 0)
66-
self.span.set_tag("ec", ec+1)
64+
if self.scope is not None:
65+
self.scope.span.set_tag(ext.HTTP_STATUS_CODE, 500)
66+
self.scope.span.set_tag('http.error', str(exception))
67+
self.scope.span.set_tag("error", True)
68+
ec = self.scope.span.tags.get('ec', 0)
69+
self.scope.span.set_tag("ec", ec+1)
70+
self.scope.close()
6771

6872

6973
def load_middleware_wrapper(wrapped, instance, args, kwargs):
@@ -77,7 +81,7 @@ def load_middleware_wrapper(wrapped, instance, args, kwargs):
7781
return wrapped(*args, **kwargs)
7882

7983
# Save the list of middleware for Snapshot reporting
80-
internal_tracer.sensor.meter.djmw = settings.MIDDLEWARE
84+
tracer.sensor.meter.djmw = settings.MIDDLEWARE
8185

8286
if type(settings.MIDDLEWARE) is tuple:
8387
settings.MIDDLEWARE = (DJ_INSTANA_MIDDLEWARE,) + settings.MIDDLEWARE
@@ -91,7 +95,7 @@ def load_middleware_wrapper(wrapped, instance, args, kwargs):
9195
return wrapped(*args, **kwargs)
9296

9397
# Save the list of middleware for Snapshot reporting
94-
internal_tracer.sensor.meter.djmw = settings.MIDDLEWARE_CLASSES
98+
tracer.sensor.meter.djmw = settings.MIDDLEWARE_CLASSES
9599

96100
if type(settings.MIDDLEWARE_CLASSES) is tuple:
97101
settings.MIDDLEWARE_CLASSES = (DJ_INSTANA_MIDDLEWARE,) + settings.MIDDLEWARE_CLASSES
@@ -107,8 +111,9 @@ def load_middleware_wrapper(wrapped, instance, args, kwargs):
107111
except Exception as e:
108112
logger.warn("Instana: Couldn't add InstanaMiddleware to Django: ", e)
109113

114+
110115
try:
111116
if 'django' in sys.modules:
112117
wrapt.wrap_function_wrapper('django.core.handlers.base', 'BaseHandler.load_middleware', load_middleware_wrapper)
113-
except:
118+
except Exception:
114119
pass

‎instana/instrumentation/urllib3.py

+12-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
from __future__ import absolute_import
22

3-
import instana
43
import opentracing
54
import opentracing.ext.tags as ext
65
import wrapt
76

87
from ..log import logger
9-
from ..tracer import internal_tracer
8+
from ..tracer import internal_tracer as tracer
109

1110
try:
1211
import urllib3 # noqa
@@ -23,36 +22,40 @@ def collect(instance, args, kwargs):
2322
kvs['method'] = args[0]
2423
kvs['path'] = args[1]
2524
else:
26-
kvs['method'] = kwargs['method']
27-
kvs['path'] = kwargs['url']
25+
kvs['method'] = kwargs.get('method')
26+
kvs['path'] = kwargs.get('path')
27+
if kvs['path'] is None:
28+
kvs['path'] = kwargs.get('url')
2829

2930
if type(instance) is urllib3.connectionpool.HTTPSConnectionPool:
3031
kvs['url'] = 'https://%s:%d%s' % (kvs['host'], kvs['port'], kvs['path'])
3132
else:
3233
kvs['url'] = 'http://%s:%d%s' % (kvs['host'], kvs['port'], kvs['path'])
33-
except Exception as e:
34-
logger.debug(e)
34+
except Exception:
35+
logger.debug("urllib3 collect error", exc_info=True)
3536
return kvs
3637
else:
3738
return kvs
3839

3940
@wrapt.patch_function_wrapper('urllib3', 'HTTPConnectionPool.urlopen')
4041
def urlopen_with_instana(wrapped, instance, args, kwargs):
41-
parent_span = internal_tracer.active_span
42+
parent_span = tracer.active_span
4243

4344
# If we're not tracing, just return
4445
if parent_span is None:
4546
return wrapped(*args, **kwargs)
4647

47-
with internal_tracer.start_active_span("urllib3", child_of=parent_span) as scope:
48+
with tracer.start_active_span("urllib3", child_of=parent_span) as scope:
4849
try:
4950
kvs = collect(instance, args, kwargs)
5051
if 'url' in kvs:
5152
scope.span.set_tag(ext.HTTP_URL, kvs['url'])
5253
if 'method' in kvs:
5354
scope.span.set_tag(ext.HTTP_METHOD, kvs['method'])
5455

55-
internal_tracer.inject(scope.span.context, opentracing.Format.HTTP_HEADERS, kwargs["headers"])
56+
if 'headers' in kwargs:
57+
tracer.inject(scope.span.context, opentracing.Format.HTTP_HEADERS, kwargs['headers'])
58+
5659
rv = wrapped(*args, **kwargs)
5760

5861
scope.span.set_tag(ext.HTTP_STATUS_CODE, rv.status)

‎instana/json_span.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class JsonSpan(object):
77
d = 0
88
n = None
99
f = None
10-
ec = 0
10+
ec = None
1111
error = None
1212
data = None
1313

@@ -44,6 +44,7 @@ class HttpData(object):
4444
url = None
4545
status = 0
4646
method = None
47+
error = None
4748

4849
def __init__(self, **kwds):
4950
self.__dict__.update(kwds)

‎instana/meter.py

+5-5
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
class Snapshot(object):
1818
name = None
1919
version = None
20-
f = None # flavor: CPython, Jython, IronPython, PyPy
21-
a = None # architecture: i386, x86, x86_64, AMD64
20+
f = None # flavor: CPython, Jython, IronPython, PyPy
21+
a = None # architecture: i386, x86, x86_64, AMD64
2222
versions = None
2323
djmw = []
2424

@@ -29,8 +29,8 @@ def to_dict(self):
2929
kvs = dict()
3030
kvs['name'] = self.name
3131
kvs['version'] = self.version
32-
kvs['f'] = self.f # flavor
33-
kvs['a'] = self.a # architecture
32+
kvs['f'] = self.f # flavor
33+
kvs['a'] = self.a # architecture
3434
kvs['versions'] = self.versions
3535
kvs['djmw'] = list(self.djmw)
3636
return kvs
@@ -157,7 +157,7 @@ def collect_snapshot(self):
157157
appname = os.environ["FLASK_APP"]
158158
elif "DJANGO_SETTINGS_MODULE" in os.environ:
159159
appname = os.environ["DJANGO_SETTINGS_MODULE"].split('.')[0]
160-
elif hasattr(sys, '__interactivehook__') or hasattr(sys, 'ps1'):
160+
elif hasattr(sys, '__interactivehook__') or (hasattr(sys, 'ps1') and hasattr(sys, 'ps2')):
161161
appname = "Interactive Console"
162162
else:
163163
appname = os.path.basename(sys.argv[0])

‎instana/recorder.py

+37-23
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ def build_registered_span(self, span):
9292
data.http = HttpData(host=self.get_host_name(span),
9393
url=self.get_string_tag(span, ext.HTTP_URL),
9494
method=self.get_string_tag(span, ext.HTTP_METHOD),
95-
status=self.get_tag(span, ext.HTTP_STATUS_CODE))
95+
status=self.get_tag(span, ext.HTTP_STATUS_CODE),
96+
error=self.get_tag(span, 'http.error'))
9697

9798
if span.operation_name == "soap":
9899
data.soap = SoapData(action=self.get_tag(span, 'soap.action'))
@@ -109,17 +110,23 @@ def build_registered_span(self, span):
109110
entityFrom = {'e': self.sensor.agent.from_.pid,
110111
'h': self.sensor.agent.from_.agentUuid}
111112

112-
return JsonSpan(
113-
n=span.operation_name,
114-
t=span.context.trace_id,
115-
p=span.parent_id,
116-
s=span.context.span_id,
117-
ts=int(round(span.start_time * 1000)),
118-
d=int(round(span.duration * 1000)),
119-
f=entityFrom,
120-
ec=self.get_tag(span, "ec", 0),
121-
error=self.get_tag(span, "error"),
122-
data=data)
113+
json_span = JsonSpan(n=span.operation_name,
114+
t=span.context.trace_id,
115+
p=span.parent_id,
116+
s=span.context.span_id,
117+
ts=int(round(span.start_time * 1000)),
118+
d=int(round(span.duration * 1000)),
119+
f=entityFrom,
120+
data=data)
121+
122+
error = self.get_tag(span, "error", False)
123+
ec = self.get_tag(span, "ec", None)
124+
125+
if error and ec:
126+
json_span.error = error
127+
json_span.ec = ec
128+
129+
return json_span
123130

124131
def build_sdk_span(self, span):
125132
""" Takes a BasicSpan and converts into an SDK type JsonSpan """
@@ -135,17 +142,24 @@ def build_sdk_span(self, span):
135142
entityFrom = {'e': self.sensor.agent.from_.pid,
136143
'h': self.sensor.agent.from_.agentUuid}
137144

138-
return JsonSpan(
139-
t=span.context.trace_id,
140-
p=span.parent_id,
141-
s=span.context.span_id,
142-
ts=int(round(span.start_time * 1000)),
143-
d=int(round(span.duration * 1000)),
144-
n="sdk",
145-
f=entityFrom,
146-
ec=self.get_tag(span, "ec"),
147-
error=self.get_tag(span, "error"),
148-
data=data)
145+
json_span = JsonSpan(
146+
t=span.context.trace_id,
147+
p=span.parent_id,
148+
s=span.context.span_id,
149+
ts=int(round(span.start_time * 1000)),
150+
d=int(round(span.duration * 1000)),
151+
n="sdk",
152+
f=entityFrom,
153+
data=data)
154+
155+
error = self.get_tag(span, "error", False)
156+
ec = self.get_tag(span, "ec", None)
157+
158+
if error and ec:
159+
json_span.error = error
160+
json_span.ec = ec
161+
162+
return json_span
149163

150164
def get_tag(self, span, tag, default=None):
151165
if tag in span.tags:

‎instana/wsgi.py

+20-16
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import opentracing as ot
44
import opentracing.ext.tags as tags
55

6-
from .tracer import internal_tracer
6+
from .tracer import internal_tracer as tracer
77

88

99
class iWSGIMiddleware(object):
@@ -18,29 +18,33 @@ def __call__(self, environ, start_response):
1818

1919
def new_start_response(status, headers, exc_info=None):
2020
"""Modified start response with additional headers."""
21-
internal_tracer.inject(span.context, ot.Format.HTTP_HEADERS, headers)
21+
tracer.inject(self.scope.span.context, ot.Format.HTTP_HEADERS, headers)
2222
res = start_response(status, headers, exc_info)
2323

2424
sc = status.split(' ')[0]
2525
if 500 <= int(sc) <= 511:
26-
span.set_tag("error", True)
27-
ec = span.tags.get('ec', 0)
28-
span.set_tag("ec", ec+1)
26+
self.scope.span.set_tag("error", True)
27+
ec = self.scope.span.tags.get('ec', 0)
28+
self.scope.span.set_tag("ec", ec+1)
2929

30-
span.set_tag(tags.HTTP_STATUS_CODE, sc)
31-
span.finish()
30+
self.scope.span.set_tag(tags.HTTP_STATUS_CODE, sc)
31+
self.scope.close()
3232
return res
3333

34+
ctx = None
3435
if 'HTTP_X_INSTANA_T' in env and 'HTTP_X_INSTANA_S' in env:
35-
ctx = internal_tracer.extract(ot.Format.HTTP_HEADERS, env)
36-
span = internal_tracer.start_span("wsgi", child_of=ctx)
37-
else:
38-
span = internal_tracer.start_span("wsgi")
39-
40-
span.set_tag(tags.HTTP_URL, env['PATH_INFO'])
41-
span.set_tag("http.params", env['QUERY_STRING'])
42-
span.set_tag(tags.HTTP_METHOD, env['REQUEST_METHOD'])
43-
span.set_tag("http.host", env['HTTP_HOST'])
36+
ctx = tracer.extract(ot.Format.HTTP_HEADERS, env)
37+
38+
self.scope = tracer.start_active_span("wsgi", child_of=ctx)
39+
40+
if 'PATH_INFO' in env:
41+
self.scope.span.set_tag(tags.HTTP_URL, env['PATH_INFO'])
42+
if 'QUERY_STRING' in env:
43+
self.scope.span.set_tag("http.params", env['QUERY_STRING'])
44+
if 'REQUEST_METHOD' in env:
45+
self.scope.span.set_tag(tags.HTTP_METHOD, env['REQUEST_METHOD'])
46+
if 'HTTP_HOST' in env:
47+
self.scope.span.set_tag("http.host", env['HTTP_HOST'])
4448

4549
return self.app(environ, new_start_response)
4650

‎setup.py

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def check_setuptools():
4444
},
4545
extras_require={
4646
'test': [
47+
'django>=1.11',
4748
'nose>=1.0',
4849
'flask>=0.12.2',
4950
'lxml>=3.4',

0 commit comments

Comments
 (0)