From b0a428365a0ec6e6ed2f8809ccbeb185ee4f847e Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Sat, 27 Jan 2018 15:30:09 +0100 Subject: [PATCH] Hook lower in urllib3 for greater coverage * Fixes situation where requests package calls won't be traced * Add tests that include calls via requests package --- instana/instrumentation/urllib3.py | 37 +++++++++++++++++++---- test_requirements.txt | 1 + tests/test_urllib3.py | 47 ++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/instana/instrumentation/urllib3.py b/instana/instrumentation/urllib3.py index 1b17ef47..371db35c 100644 --- a/instana/instrumentation/urllib3.py +++ b/instana/instrumentation/urllib3.py @@ -1,14 +1,39 @@ from __future__ import absolute_import -import opentracing.ext.tags as ext import instana +from instana.log import logger import opentracing +import opentracing.ext.tags as ext import wrapt try: - import urllib3 + import urllib3 # noqa + + def collect(instance, args, kwargs): + try: + kvs = {} + + kvs['host'] = instance.host + kvs['port'] = instance.port - @wrapt.patch_function_wrapper('urllib3', 'PoolManager.urlopen') + if len(args) is 2: + kvs['method'] = args[0] + kvs['path'] = args[1] + else: + kvs['method'] = kwargs['method'] + kvs['path'] = kwargs['url'] + + if type(instance) is urllib3.connectionpool.HTTPSConnectionPool: + kvs['url'] = 'https://%s:%d%s' % (kvs['host'], kvs['port'], kvs['path']) + else: + kvs['url'] = 'http://%s:%d%s' % (kvs['host'], kvs['port'], kvs['path']) + except Exception as e: + logger.debug(e) + return kvs + else: + return kvs + + @wrapt.patch_function_wrapper('urllib3', 'HTTPConnectionPool.urlopen') def urlopen_with_instana(wrapped, instance, args, kwargs): context = instana.internal_tracer.current_context() @@ -18,8 +43,10 @@ def urlopen_with_instana(wrapped, instance, args, kwargs): try: span = instana.internal_tracer.start_span("urllib3", child_of=context) - span.set_tag(ext.HTTP_URL, args[1]) - span.set_tag(ext.HTTP_METHOD, args[0]) + + kvs = collect(instance, args, kwargs) + span.set_tag(ext.HTTP_URL, kvs['url']) + span.set_tag(ext.HTTP_METHOD, kvs['method']) instana.internal_tracer.inject(span.context, opentracing.Format.HTTP_HEADERS, kwargs["headers"]) rv = wrapped(*args, **kwargs) diff --git a/test_requirements.txt b/test_requirements.txt index ddef682d..695c7f06 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -5,3 +5,4 @@ basictracer>=2.2.0 autowrapt>=1.0 flask>=0.12.2 urllib3>=1.9 +requests>=2.11.1 diff --git a/tests/test_urllib3.py b/tests/test_urllib3.py index 1766f20f..91e19200 100644 --- a/tests/test_urllib3.py +++ b/tests/test_urllib3.py @@ -1,6 +1,7 @@ from __future__ import absolute_import from nose.tools import assert_equals from instana import internal_tracer as tracer +import requests import urllib3 @@ -149,3 +150,49 @@ def test_client_error(self): assert_equals(second_span.t, first_span.t) assert_equals(second_span.p, first_span.s) + + def test_requestspkg_get(self): + span = tracer.start_span("test") + r = requests.get('http://127.0.0.1:5000/', timeout=2) + span.finish() + + spans = self.recorder.queued_spans() + assert_equals(2, len(spans)) + first_span = spans[1] + second_span = spans[0] + + assert(r) + assert_equals(200, r.status_code) + assert_equals("test", first_span.data.sdk.name) + assert_equals("urllib3", second_span.n) + assert_equals(200, second_span.data.http.status) + assert_equals("http://127.0.0.1:5000/", second_span.data.http.url) + assert_equals("GET", second_span.data.http.method) + + assert_equals(None, second_span.error) + assert_equals(None, second_span.ec) + + assert_equals(second_span.t, first_span.t) + assert_equals(second_span.p, first_span.s) + + def test_requestspkg_put(self): + span = tracer.start_span("test") + r = requests.put('http://127.0.0.1:5000/notfound') + span.finish() + + spans = self.recorder.queued_spans() + assert_equals(2, len(spans)) + first_span = spans[1] + second_span = spans[0] + + assert_equals(404, r.status_code) + assert_equals("test", first_span.data.sdk.name) + assert_equals("urllib3", second_span.n) + assert_equals(404, second_span.data.http.status) + assert_equals("http://127.0.0.1:5000/notfound", second_span.data.http.url) + assert_equals("PUT", second_span.data.http.method) + assert_equals(None, second_span.error) + assert_equals(None, second_span.ec) + + assert_equals(second_span.t, first_span.t) + assert_equals(second_span.p, first_span.s)