diff --git a/CHANGELOG.md b/CHANGELOG.md index b2bd4680..2508029f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixed - Exclude python library paths from backtraces. ([PR #514](https://github.com/scoutapp/scout_apm_python/issues/514)) +- Capture the full URL in `urllib3` spans. ## [2.18.0] 2021-02-09 diff --git a/src/scout_apm/instruments/urllib3.py b/src/scout_apm/instruments/urllib3.py index 67b69859..db2763d8 100644 --- a/src/scout_apm/instruments/urllib3.py +++ b/src/scout_apm/instruments/urllib3.py @@ -43,21 +43,22 @@ def ensure_installed(): @wrapt.decorator def wrapped_urlopen(wrapped, instance, args, kwargs): - def _extract_method(method, *args, **kwargs): - return method + def _extract_method_url(method, url, *args, **kwargs): + return method, url try: - method = _extract_method(*args, **kwargs) + method, url = _extract_method_url(*args, **kwargs) except TypeError: method = "Unknown" - - try: - url = text_type(instance._absolute_url("/")) - except Exception: - logger.exception("Could not get URL for HTTPConnectionPool") url = "Unknown" + else: + try: + url = text_type(instance._absolute_url(url)) + except Exception: + logger.exception("Could not get URL for HTTPConnectionPool") + url = "Unknown" tracked_request = TrackedRequest.instance() with tracked_request.span(operation="HTTP/{}".format(method)) as span: - span.tag("url", text_type(url)) + span.tag("url", url) return wrapped(*args, **kwargs) diff --git a/tests/integration/instruments/test_requests.py b/tests/integration/instruments/test_requests.py new file mode 100644 index 00000000..9fc85dd9 --- /dev/null +++ b/tests/integration/instruments/test_requests.py @@ -0,0 +1,52 @@ +# coding=utf-8 +""" +Test requests package is instrumented via urllib3 instrumentation. + +We don't instrument the package directly, but urllib3 underneath. +However, it's a good idea to verify it's working as expected. +""" +from __future__ import absolute_import, division, print_function, unicode_literals + +import httpretty +import requests + +from scout_apm.instruments.urllib3 import ensure_installed + + +def test_request(tracked_request): + ensure_installed() + with httpretty.enabled(allow_net_connect=False): + httpretty.register_uri( + httpretty.GET, "https://example.com/", body="Hello World!" + ) + + session = requests.Session() + response = session.get("https://example.com") + + assert response.status_code == 200 + assert response.content == b"Hello World!" + assert len(tracked_request.complete_spans) == 1 + span = tracked_request.complete_spans[0] + assert span.operation == "HTTP/GET" + assert span.tags["url"] == "https://example.com:443/" + + +def test_second_request(tracked_request): + ensure_installed() + with tracked_request.span("Test"), httpretty.enabled(allow_net_connect=False): + httpretty.register_uri( + httpretty.GET, "https://example.com/foo", body="Hello World!" + ) + httpretty.register_uri( + httpretty.GET, "https://example.org/bar", body="Hello World!" + ) + session = requests.Session() + session.get("https://example.com/foo") + session.get("https://example.org/bar") + assert len(tracked_request.complete_spans) == 3 + assert ( + tracked_request.complete_spans[0].tags["url"] == "https://example.com:443/foo" + ) + assert ( + tracked_request.complete_spans[1].tags["url"] == "https://example.org:443/bar" + ) diff --git a/tests/integration/instruments/test_urllib3.py b/tests/integration/instruments/test_urllib3.py index ea2ae35a..922304e6 100644 --- a/tests/integration/instruments/test_urllib3.py +++ b/tests/integration/instruments/test_urllib3.py @@ -91,6 +91,29 @@ def test_request(tracked_request): assert span.tags["url"] == "https://example.com:443/" +def test_second_request(tracked_request): + ensure_installed() + with tracked_request.span("Test"), httpretty.enabled(allow_net_connect=False): + httpretty.register_uri( + httpretty.GET, "https://example.com/foo", body="Hello World!" + ) + httpretty.register_uri( + httpretty.GET, "https://example.org/bar", body="Hello World!" + ) + + http = urllib3_cert_pool_manager() + http.request("GET", "https://example.com/foo") + http.request("GET", "https://example.org/bar") + + assert len(tracked_request.complete_spans) == 3 + assert ( + tracked_request.complete_spans[0].tags["url"] == "https://example.com:443/foo" + ) + assert ( + tracked_request.complete_spans[1].tags["url"] == "https://example.org:443/bar" + ) + + def test_request_type_error(tracked_request): ensure_installed() with pytest.raises(TypeError): @@ -101,7 +124,7 @@ def test_request_type_error(tracked_request): assert len(tracked_request.complete_spans) == 1 span = tracked_request.complete_spans[0] assert span.operation == "HTTP/Unknown" - assert span.tags["url"] == "https://example.com:443/" + assert span.tags["url"] == "Unknown" def test_request_no_absolute_url(caplog, tracked_request):