From 2e8e14fd4639ae0ea8b70823d87ffba15d53fe3a Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Wed, 2 Jun 2021 14:33:19 -0700 Subject: [PATCH 01/12] Implemented username and pw check on urllib --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 5 +++-- .../tests/test_urllib_integration.py | 9 +++++++++ tox.ini | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index fcac480e02..4bd9e83d45 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -39,6 +39,7 @@ import functools import types from typing import Collection +import yarl from urllib.request import ( # pylint: disable=no-name-in-module,import-error OpenerDirector, Request, @@ -144,7 +145,7 @@ def _instrumented_open_call( labels = { SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, + SpanAttributes.HTTP_URL: str(yarl.URL(url).with_user(None)), } with tracer.start_as_current_span( @@ -153,7 +154,7 @@ def _instrumented_open_call( exception = None if span.is_recording(): span.set_attribute(SpanAttributes.HTTP_METHOD, method) - span.set_attribute(SpanAttributes.HTTP_URL, url) + span.set_attribute(SpanAttributes.HTTP_URL, str(yarl.URL(url).with_user(None))) headers = get_or_create_headers() inject(headers) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index b05fe00012..d0b21b5861 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -318,6 +318,15 @@ def test_requests_timeout_exception(self, *_, **__): span = self.assert_span() self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_credentials_url(self): + url = "http://username:password@httpbin.org/status/200" + + with self.assertRaises(Exception): + self.perform_request(url) + + span = self.assert_span() + print(span.attributes) + self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL) class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase): @staticmethod diff --git a/tox.ini b/tox.ini index bf3f289f91..905a7b5d5d 100644 --- a/tox.ini +++ b/tox.ini @@ -245,7 +245,7 @@ commands_pre = flask: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-flask[test] - urllib: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-urllib[test] + urllib: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-urllib[test] urllib3: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-urllib3[test] From 441a0df05711f16fbb5ffaebdae63cdb0aac1a3c Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Thu, 3 Jun 2021 12:54:19 -0700 Subject: [PATCH 02/12] Implemented check on aiohttp and wrapped conversions in try catch --- .../aiohttp_client/__init__.py | 12 ++++--- .../tests/test_aiohttp_client_integration.py | 33 ++++++++++--------- .../instrumentation/requests/__init__.py | 6 ++++ .../tests/test_requests_integration.py | 7 ++++ .../instrumentation/urllib/__init__.py | 9 +++-- .../tests/test_urllib_integration.py | 1 - .../tests/test_urllib3_integration.py | 6 ++++ tox.ini | 2 +- 8 files changed, 53 insertions(+), 23 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 9597297b65..b28d2e32e5 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -65,6 +65,7 @@ def strip_query_params(url: yarl.URL) -> str: import types import typing from typing import Collection +import yarl import aiohttp import wrapt @@ -171,13 +172,16 @@ async def on_request_start( ) if trace_config_ctx.span.is_recording(): + try: + url = yarl.URL(params.url).with_user(None) + except ValueError: # invalid url was passed + pass + attributes = { SpanAttributes.HTTP_METHOD: http_method, - SpanAttributes.HTTP_URL: trace_config_ctx.url_filter( - params.url - ) + SpanAttributes.HTTP_URL: trace_config_ctx.url_filter(url) if callable(trace_config_ctx.url_filter) - else str(params.url), + else str(url), } for key, value in attributes.items(): trace_config_ctx.span.set_attribute(key, value) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index 530245bab1..f2b2cfaa54 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -263,34 +263,37 @@ async def do_request(url): ] ) self.memory_exporter.clear() + + def test_credentials(self): + trace_configs = [aiohttp_client.create_trace_config()] - def test_timeout(self): - async def request_handler(request): - await asyncio.sleep(1) - assert "traceparent" in request.headers - return aiohttp.web.Response() + url = "http://username:password@httpbin.org/" + with self.subTest(url=url): - host, port = self._http_request( - trace_config=aiohttp_client.create_trace_config(), - url="/test_timeout", - request_handler=request_handler, - timeout=aiohttp.ClientTimeout(sock_read=0.01), - ) + async def do_request(url): + async with aiohttp.ClientSession( + trace_configs=trace_configs, + ) as session: + async with session.get(url): + pass + + loop = asyncio.get_event_loop() + loop.run_until_complete(do_request(url)) self.assert_spans( [ ( "HTTP GET", - (StatusCode.ERROR, None), + (StatusCode.UNSET, None), { SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: "http://{}:{}/test_timeout".format( - host, port - ), + SpanAttributes.HTTP_URL: "http://httpbin.org/", + SpanAttributes.HTTP_STATUS_CODE: int(HTTPStatus.OK), }, ) ] ) + self.memory_exporter.clear() def test_too_many_redirects(self): async def request_handler(request): diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 0731106449..c3c52e7d44 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -36,6 +36,7 @@ import functools import types from typing import Collection +import yarl from requests.models import Response from requests.sessions import Session @@ -124,6 +125,11 @@ def _instrumented_requests_call( if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) + try: + url = str(yarl.URL(url).with_user(None)) + except ValueError: # invalid url was passed + pass + labels = {} labels[SpanAttributes.HTTP_METHOD] = method labels[SpanAttributes.HTTP_URL] = url diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 08562f6a36..66cab03fcf 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -357,6 +357,13 @@ def test_invalid_url(self): ) self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_url_credentials(self): + new_url = "http://username:password@httpbin.org/status/200" + self.perform_request(new_url) + span = self.assert_span() + + self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL) + def test_if_headers_equals_none(self): result = requests.get(self.URL, headers=None) self.assertEqual(result.text, "Hello!") diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index 4bd9e83d45..f821d5a11f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -143,9 +143,14 @@ def _instrumented_open_call( if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) + try: + url = str(yarl.URL(url).with_user(None)) + except ValueError: # invalid url was passed + pass + labels = { SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: str(yarl.URL(url).with_user(None)), + SpanAttributes.HTTP_URL: url } with tracer.start_as_current_span( @@ -154,7 +159,7 @@ def _instrumented_open_call( exception = None if span.is_recording(): span.set_attribute(SpanAttributes.HTTP_METHOD, method) - span.set_attribute(SpanAttributes.HTTP_URL, str(yarl.URL(url).with_user(None))) + span.set_attribute(SpanAttributes.HTTP_URL, url) headers = get_or_create_headers() inject(headers) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index d0b21b5861..86195b0ab1 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -325,7 +325,6 @@ def test_credentials_url(self): self.perform_request(url) span = self.assert_span() - print(span.attributes) self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL) class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase): diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 843679f932..b3149e610f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -287,3 +287,9 @@ def url_filter(url): response = self.perform_request(self.HTTP_URL + "?e=mcc") self.assert_success_span(response, self.HTTP_URL) + + def test_url_credential_removal(self): + url = "http://username:password@httpbin.org/status/200" + + response = self.perform_request(url) + self.assert_success_span(response, self.HTTP_URL) diff --git a/tox.ini b/tox.ini index 905a7b5d5d..ae636c8eb1 100644 --- a/tox.ini +++ b/tox.ini @@ -273,7 +273,7 @@ commands_pre = redis: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-redis[test] - requests: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-requests[test] + requests: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-requests[test] starlette: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-starlette[test] From 75587429d6244b5e4a9db2ac810ae6e88f243fb7 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Thu, 3 Jun 2021 13:14:15 -0700 Subject: [PATCH 03/12] Made function name uniform --- .../tests/test_aiohttp_client_integration.py | 2 +- .../tests/test_requests_integration.py | 2 +- .../tests/test_instrumentation.py | 3 +++ .../tests/test_urllib_integration.py | 2 +- .../tests/test_urllib3_integration.py | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index f2b2cfaa54..91d588d8b8 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -264,7 +264,7 @@ async def do_request(url): ) self.memory_exporter.clear() - def test_credentials(self): + def test_credential_removal(self): trace_configs = [aiohttp_client.create_trace_config()] url = "http://username:password@httpbin.org/" diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 66cab03fcf..95006d0602 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -357,7 +357,7 @@ def test_invalid_url(self): ) self.assertEqual(span.status.status_code, StatusCode.ERROR) - def test_url_credentials(self): + def test_credential_removal(self): new_url = "http://username:password@httpbin.org/status/200" self.perform_request(new_url) span = self.assert_span() diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index 58c0127647..6edc3df2d5 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -455,6 +455,9 @@ def test_response_headers(self): self.memory_exporter.clear() set_global_response_propagator(orig) + def test_credential_removal(self): + pass + class TornadoHookTest(TornadoTest): _client_request_hook = None diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 86195b0ab1..ef09fbd41a 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -318,7 +318,7 @@ def test_requests_timeout_exception(self, *_, **__): span = self.assert_span() self.assertEqual(span.status.status_code, StatusCode.ERROR) - def test_credentials_url(self): + def test_credential_removal(self): url = "http://username:password@httpbin.org/status/200" with self.assertRaises(Exception): diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index b3149e610f..658044887b 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -288,7 +288,7 @@ def url_filter(url): response = self.perform_request(self.HTTP_URL + "?e=mcc") self.assert_success_span(response, self.HTTP_URL) - def test_url_credential_removal(self): + def test_credential_removal(self): url = "http://username:password@httpbin.org/status/200" response = self.perform_request(url) From bf536ad1d422cde8de0b9bb83434eff0dc894d39 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Thu, 3 Jun 2021 17:04:31 -0700 Subject: [PATCH 04/12] Finished implementations in tornado, wsgi, and asgi Restored aiohttp test due to overwritten test Readded aiohttp url credential test Addressed yarl not found in tornado client in some tests --- .../tests/test_aiohttp_client_integration.py | 64 +++++++++++++------ .../instrumentation/asgi/__init__.py | 6 ++ .../tests/test_asgi_middleware.py | 8 +++ .../instrumentation/tornado/client.py | 9 ++- .../tests/test_instrumentation.py | 20 +++++- .../instrumentation/wsgi/__init__.py | 8 ++- .../tests/test_wsgi_middleware.py | 12 ++++ tox.ini | 6 +- 8 files changed, 111 insertions(+), 22 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index 91d588d8b8..a9f82202d8 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -263,37 +263,34 @@ async def do_request(url): ] ) self.memory_exporter.clear() - - def test_credential_removal(self): - trace_configs = [aiohttp_client.create_trace_config()] - - url = "http://username:password@httpbin.org/" - with self.subTest(url=url): - async def do_request(url): - async with aiohttp.ClientSession( - trace_configs=trace_configs, - ) as session: - async with session.get(url): - pass + def test_timeout(self): + async def request_handler(request): + await asyncio.sleep(1) + assert "traceparent" in request.headers + return aiohttp.web.Response() - loop = asyncio.get_event_loop() - loop.run_until_complete(do_request(url)) + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config(), + url="/test_timeout", + request_handler=request_handler, + timeout=aiohttp.ClientTimeout(sock_read=0.01), + ) self.assert_spans( [ ( "HTTP GET", - (StatusCode.UNSET, None), + (StatusCode.ERROR, None), { SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: "http://httpbin.org/", - SpanAttributes.HTTP_STATUS_CODE: int(HTTPStatus.OK), + SpanAttributes.HTTP_URL: "http://{}:{}/test_timeout".format( + host, port + ), }, ) ] ) - self.memory_exporter.clear() def test_too_many_redirects(self): async def request_handler(request): @@ -324,6 +321,37 @@ async def request_handler(request): ] ) + def test_credential_removal(self): + trace_configs = [aiohttp_client.create_trace_config()] + + url = "http://username:password@httpbin.org/status/200" + with self.subTest(url=url): + + async def do_request(url): + async with aiohttp.ClientSession( + trace_configs=trace_configs, + ) as session: + async with session.get(url): + pass + + loop = asyncio.get_event_loop() + loop.run_until_complete(do_request(url)) + + self.assert_spans( + [ + ( + "HTTP GET", + (StatusCode.UNSET, None), + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: "http://httpbin.org/status/200", + SpanAttributes.HTTP_STATUS_CODE: int(HTTPStatus.OK), + }, + ) + ] + ) + self.memory_exporter.clear() + class TestAioHttpClientInstrumentor(TestBase): URL = "/test-path" diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 6d8f0793ca..4901f20364 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -20,6 +20,7 @@ import typing import urllib +import yarl from functools import wraps from typing import Tuple @@ -80,6 +81,11 @@ def collect_request_attributes(scope): query_string = query_string.decode("utf8") http_url = http_url + ("?" + urllib.parse.unquote(query_string)) + try: + http_url = str(yarl.URL(http_url).with_user(None)) + except ValueError: # invalid url was passed + pass + result = { SpanAttributes.HTTP_SCHEME: scope.get("scheme"), SpanAttributes.HTTP_HOST: server_host, diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 3ed438aa02..9c39b278dc 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -429,6 +429,14 @@ def test_response_attributes(self): def test_response_attributes_invalid_status_code(self): otel_asgi.set_status_code(self.span, "Invalid Status Code") self.assertEqual(self.span.set_status.call_count, 1) + + def test_credential_removal(self): + self.scope["server"] = ("username:password@httpbin.org", 80) + self.scope["path"] = "/status/200" + attrs = otel_asgi.collect_request_attributes(self.scope) + self.assertEqual( + attrs[SpanAttributes.HTTP_URL], "http://httpbin.org/status/200" + ) if __name__ == "__main__": diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py index 6bdcd58b7e..d2eb215367 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py @@ -14,6 +14,8 @@ import functools +import yarl + from tornado.httpclient import HTTPError, HTTPRequest from opentelemetry import trace @@ -60,8 +62,13 @@ def fetch_async(tracer, request_hook, response_hook, func, _, args, kwargs): request_hook(span, request) if span.is_recording(): + try: + url = str(yarl.URL(request.url).with_user(None)) + except ValueError: # invalid url was passed + pass + attributes = { - SpanAttributes.HTTP_URL: request.url, + SpanAttributes.HTTP_URL: url, SpanAttributes.HTTP_METHOD: request.method, } for key, value in attributes.items(): diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index 6edc3df2d5..599b9646a4 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -456,7 +456,25 @@ def test_response_headers(self): set_global_response_propagator(orig) def test_credential_removal(self): - pass + response = self.fetch("http://username:password@httpbin.org/status/200") + self.assertEqual(response.code, 200) + + spans = self.sorted_spans(self.memory_exporter.get_finished_spans()) + self.assertEqual(len(spans), 1) + client = spans[0] + + self.assertEqual(client.name, "GET") + self.assertEqual(client.kind, SpanKind.CLIENT) + self.assert_span_has_attributes( + client, + { + SpanAttributes.HTTP_URL: "http://httpbin.org/status/200", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_STATUS_CODE: 200, + }, + ) + + self.memory_exporter.clear() class TornadoHookTest(TornadoTest): diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 24303a8ac6..a74d086863 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -57,6 +57,7 @@ def hello(): import functools import typing import wsgiref.util as wsgiref_util +import yarl from opentelemetry import context, trace from opentelemetry.instrumentation.utils import http_status_to_status_code @@ -128,7 +129,12 @@ def collect_request_attributes(environ): if target is not None: result[SpanAttributes.HTTP_TARGET] = target else: - result[SpanAttributes.HTTP_URL] = wsgiref_util.request_uri(environ) + try: + url = str(yarl.URL(wsgiref_util.request_uri(environ)).with_user(None)) + except ValueError: # invalid url was passed + pass + + result[SpanAttributes.HTTP_URL] = url remote_addr = environ.get("REMOTE_ADDR") if remote_addr: diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 6652f11ebb..38bdfa2614 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -364,6 +364,18 @@ def test_response_attributes(self): self.assertEqual(self.span.set_attribute.call_count, len(expected)) self.span.set_attribute.assert_has_calls(expected, any_order=True) + def test_credential_removal(self): + self.environ["HTTP_HOST"] = "username:password@httpbin.com" + self.environ["PATH_INFO"] = "/status/200" + expected = { + SpanAttributes.HTTP_URL: "http://httpbin.com/status/200", + SpanAttributes.NET_HOST_PORT: 80, + } + self.assertGreaterEqual( + otel_wsgi.collect_request_attributes(self.environ).items(), + expected.items(), + ) + class TestWsgiMiddlewareWithTracerProvider(WsgiTestBase): def validate_response( diff --git a/tox.ini b/tox.ini index ae636c8eb1..c19aa9bce2 100644 --- a/tox.ini +++ b/tox.ini @@ -277,7 +277,7 @@ commands_pre = starlette: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-starlette[test] - tornado: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-tornado[test] + tornado: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-tornado[test] jinja2: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-jinja2[test] @@ -287,6 +287,10 @@ commands_pre = aiopg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-dbapi pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-aiopg[test] + asgi: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] + + wsgi: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + datadog: pip install flaky {toxinidir}/exporter/opentelemetry-exporter-datadog[test] sklearn: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn[test] From c8be726b96d903e8ad4c215ee606eb6470a67ee5 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Thu, 3 Jun 2021 19:22:01 -0700 Subject: [PATCH 05/12] Fixed dependencies --- tox.ini | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tox.ini b/tox.ini index c19aa9bce2..6e859c063a 100644 --- a/tox.ini +++ b/tox.ini @@ -233,8 +233,8 @@ commands_pre = grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test] falcon,flask,django,pyramid,tornado,starlette,fastapi: pip install {toxinidir}/util/opentelemetry-util-http[test] - wsgi,falcon,flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] - asgi,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] + wsgi,falcon,flask,django,pyramid: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + asgi,starlette,fastapi: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] asyncpg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asyncpg[test] @@ -287,9 +287,9 @@ commands_pre = aiopg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-dbapi pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-aiopg[test] - asgi: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] + asgi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] - wsgi: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + wsgi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] datadog: pip install flaky {toxinidir}/exporter/opentelemetry-exporter-datadog[test] From 9aba340139e881fe832766553ddda7c605536b3b Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Thu, 3 Jun 2021 20:17:25 -0700 Subject: [PATCH 06/12] Moved common function into utils Readded removed dependency Added dependency for docker test Fixed linting errors Placed yarl dependency and moved common function into utils --- dev-requirements.txt | 1 + .../instrumentation/aiohttp_client/__init__.py | 7 ++----- .../instrumentation/asgi/__init__.py | 13 +++++-------- .../tests/test_asgi_middleware.py | 2 +- .../instrumentation/requests/__init__.py | 11 +++++------ .../instrumentation/tornado/client.py | 14 +++++--------- .../tests/test_instrumentation.py | 4 +++- .../instrumentation/urllib/__init__.py | 13 ++++++------- .../tests/test_urllib_integration.py | 1 + .../instrumentation/wsgi/__init__.py | 15 +++++++-------- .../src/opentelemetry/instrumentation/utils.py | 11 +++++++++++ tox.ini | 14 +++++--------- 12 files changed, 52 insertions(+), 54 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 75b9ed793b..2427aa9f4b 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -13,3 +13,4 @@ readme-renderer~=24.0 grpcio-tools==1.29.0 mypy-protobuf>=1.23 protobuf>=3.13.0 +yarl~=1.6 diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index b28d2e32e5..088fd6bb85 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -65,7 +65,6 @@ def strip_query_params(url: yarl.URL) -> str: import types import typing from typing import Collection -import yarl import aiohttp import wrapt @@ -77,6 +76,7 @@ def strip_query_params(url: yarl.URL) -> str: from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.utils import ( http_status_to_status_code, + remove_url_credentials, unwrap, ) from opentelemetry.propagate import inject @@ -172,10 +172,7 @@ async def on_request_start( ) if trace_config_ctx.span.is_recording(): - try: - url = yarl.URL(params.url).with_user(None) - except ValueError: # invalid url was passed - pass + url = remove_url_credentials(params.url) attributes = { SpanAttributes.HTTP_METHOD: http_method, diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 4901f20364..ae3063e3ea 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -20,7 +20,6 @@ import typing import urllib -import yarl from functools import wraps from typing import Tuple @@ -28,7 +27,10 @@ from opentelemetry import context, trace from opentelemetry.instrumentation.asgi.version import __version__ # noqa -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + remove_url_credentials, +) from opentelemetry.propagate import extract from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.trace import SpanAttributes @@ -81,18 +83,13 @@ def collect_request_attributes(scope): query_string = query_string.decode("utf8") http_url = http_url + ("?" + urllib.parse.unquote(query_string)) - try: - http_url = str(yarl.URL(http_url).with_user(None)) - except ValueError: # invalid url was passed - pass - result = { SpanAttributes.HTTP_SCHEME: scope.get("scheme"), SpanAttributes.HTTP_HOST: server_host, SpanAttributes.NET_HOST_PORT: port, SpanAttributes.HTTP_FLAVOR: scope.get("http_version"), SpanAttributes.HTTP_TARGET: scope.get("path"), - SpanAttributes.HTTP_URL: http_url, + SpanAttributes.HTTP_URL: str(remove_url_credentials(http_url)), } http_method = scope.get("method") if http_method: diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 9c39b278dc..90910f7fe6 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -429,7 +429,7 @@ def test_response_attributes(self): def test_response_attributes_invalid_status_code(self): otel_asgi.set_status_code(self.span, "Invalid Status Code") self.assertEqual(self.span.set_status.call_count, 1) - + def test_credential_removal(self): self.scope["server"] = ("username:password@httpbin.org", 80) self.scope["path"] = "/status/200" diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index c3c52e7d44..b758bd993e 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -36,7 +36,6 @@ import functools import types from typing import Collection -import yarl from requests.models import Response from requests.sessions import Session @@ -46,7 +45,10 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.requests.package import _instruments from opentelemetry.instrumentation.requests.version import __version__ -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + remove_url_credentials, +) from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, get_tracer @@ -125,10 +127,7 @@ def _instrumented_requests_call( if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) - try: - url = str(yarl.URL(url).with_user(None)) - except ValueError: # invalid url was passed - pass + url = str(remove_url_credentials(url)) labels = {} labels[SpanAttributes.HTTP_METHOD] = method diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py index d2eb215367..11af0db9ef 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py @@ -14,12 +14,13 @@ import functools -import yarl - from tornado.httpclient import HTTPError, HTTPRequest from opentelemetry import trace -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + remove_url_credentials, +) from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status @@ -62,13 +63,8 @@ def fetch_async(tracer, request_hook, response_hook, func, _, args, kwargs): request_hook(span, request) if span.is_recording(): - try: - url = str(yarl.URL(request.url).with_user(None)) - except ValueError: # invalid url was passed - pass - attributes = { - SpanAttributes.HTTP_URL: url, + SpanAttributes.HTTP_URL: str(remove_url_credentials(request.url)), SpanAttributes.HTTP_METHOD: request.method, } for key, value in attributes.items(): diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index 599b9646a4..338876f614 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -456,7 +456,9 @@ def test_response_headers(self): set_global_response_propagator(orig) def test_credential_removal(self): - response = self.fetch("http://username:password@httpbin.org/status/200") + response = self.fetch( + "http://username:password@httpbin.org/status/200" + ) self.assertEqual(response.code, 200) spans = self.sorted_spans(self.memory_exporter.get_finished_spans()) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index f821d5a11f..f4963b8def 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -39,7 +39,6 @@ import functools import types from typing import Collection -import yarl from urllib.request import ( # pylint: disable=no-name-in-module,import-error OpenerDirector, Request, @@ -49,7 +48,10 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib.package import _instruments from opentelemetry.instrumentation.urllib.version import __version__ -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + remove_url_credentials, +) from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, get_tracer @@ -143,14 +145,11 @@ def _instrumented_open_call( if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) - try: - url = str(yarl.URL(url).with_user(None)) - except ValueError: # invalid url was passed - pass + url = str(remove_url_credentials(url)) labels = { SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url + SpanAttributes.HTTP_URL: url, } with tracer.start_as_current_span( diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index ef09fbd41a..654fb3f8c1 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -327,6 +327,7 @@ def test_credential_removal(self): span = self.assert_span() self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL) + class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase): @staticmethod def perform_request(url: str, opener: OpenerDirector = None): diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index a74d086863..a386991036 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -57,10 +57,12 @@ def hello(): import functools import typing import wsgiref.util as wsgiref_util -import yarl from opentelemetry import context, trace -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + remove_url_credentials, +) from opentelemetry.instrumentation.wsgi.version import __version__ from opentelemetry.propagate import extract from opentelemetry.propagators.textmap import Getter @@ -129,12 +131,9 @@ def collect_request_attributes(environ): if target is not None: result[SpanAttributes.HTTP_TARGET] = target else: - try: - url = str(yarl.URL(wsgiref_util.request_uri(environ)).with_user(None)) - except ValueError: # invalid url was passed - pass - - result[SpanAttributes.HTTP_URL] = url + result[SpanAttributes.HTTP_URL] = str( + remove_url_credentials(wsgiref_util.request_uri(environ)) + ) remote_addr = environ.get("REMOTE_ADDR") if remote_addr: diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index dec070570f..a7aa840e32 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -16,6 +16,8 @@ from wrapt import ObjectProxy +from yarl import URL + from opentelemetry.trace import StatusCode @@ -60,3 +62,12 @@ def unwrap(obj, attr: str): func = getattr(obj, attr, None) if func and isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): setattr(obj, attr, func.__wrapped__) + + +def remove_url_credentials(url: str) -> URL: + """Given a string url, attempt to remove the username and password""" + try: + url = URL(url).with_user(None) + except ValueError: # invalid url was passed + pass + return url diff --git a/tox.ini b/tox.ini index 6e859c063a..bf3f289f91 100644 --- a/tox.ini +++ b/tox.ini @@ -233,8 +233,8 @@ commands_pre = grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test] falcon,flask,django,pyramid,tornado,starlette,fastapi: pip install {toxinidir}/util/opentelemetry-util-http[test] - wsgi,falcon,flask,django,pyramid: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] - asgi,starlette,fastapi: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] + wsgi,falcon,flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + asgi,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] asyncpg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asyncpg[test] @@ -245,7 +245,7 @@ commands_pre = flask: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-flask[test] - urllib: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-urllib[test] + urllib: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-urllib[test] urllib3: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-urllib3[test] @@ -273,11 +273,11 @@ commands_pre = redis: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-redis[test] - requests: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-requests[test] + requests: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-requests[test] starlette: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-starlette[test] - tornado: pip install yarl {toxinidir}/instrumentation/opentelemetry-instrumentation-tornado[test] + tornado: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-tornado[test] jinja2: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-jinja2[test] @@ -287,10 +287,6 @@ commands_pre = aiopg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-dbapi pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-aiopg[test] - asgi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] - - wsgi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] - datadog: pip install flaky {toxinidir}/exporter/opentelemetry-exporter-datadog[test] sklearn: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn[test] From 358aaa01bb34070e5bb74df16ade348e48e5951e Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Fri, 4 Jun 2021 13:56:22 -0700 Subject: [PATCH 07/12] Ran isort to follow pep8 import conversions Rearranged location of required install Fixed dependency location and linting --- dev-requirements.txt | 1 - opentelemetry-instrumentation/setup.cfg | 1 + .../src/opentelemetry/instrumentation/utils.py | 1 - tox.ini | 4 +++- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 2427aa9f4b..75b9ed793b 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -13,4 +13,3 @@ readme-renderer~=24.0 grpcio-tools==1.29.0 mypy-protobuf>=1.23 protobuf>=3.13.0 -yarl~=1.6 diff --git a/opentelemetry-instrumentation/setup.cfg b/opentelemetry-instrumentation/setup.cfg index a00673cca8..421e4497bb 100644 --- a/opentelemetry-instrumentation/setup.cfg +++ b/opentelemetry-instrumentation/setup.cfg @@ -43,6 +43,7 @@ include_package_data = True install_requires = opentelemetry-api == 1.4.0.dev0 wrapt >= 1.0.0, < 2.0.0 + yarl ~= 1.6 [options.packages.find] where = src diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index a7aa840e32..1488a1e2b8 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -15,7 +15,6 @@ from typing import Dict, Sequence from wrapt import ObjectProxy - from yarl import URL from opentelemetry.trace import StatusCode diff --git a/tox.ini b/tox.ini index bf3f289f91..d3fb98718f 100644 --- a/tox.ini +++ b/tox.ini @@ -405,6 +405,8 @@ deps = protobuf>=3.13.0 requests==2.25.0 pyodbc~=4.0.30 + yarl ~= 1.6 + changedir = tests/opentelemetry-docker-tests/tests @@ -440,4 +442,4 @@ deps = commands = {toxinidir}/scripts/generate_setup.py - {toxinidir}/scripts/generate_instrumentation_bootstrap.py \ No newline at end of file + {toxinidir}/scripts/generate_instrumentation_bootstrap.py From f1fb94201b20e755254013f259619910947a0303 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Mon, 7 Jun 2021 10:33:14 -0700 Subject: [PATCH 08/12] Disabled pylint too many public msg in test file and fixed spacing --- .../tests/test_urllib_integration.py | 2 +- tox.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 654fb3f8c1..1f11c58b3b 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -35,7 +35,7 @@ from opentelemetry.test.test_base import TestBase from opentelemetry.trace import StatusCode - +# pylint: disable=too-many-public-methods class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=no-member diff --git a/tox.ini b/tox.ini index d3fb98718f..b665a914bb 100644 --- a/tox.ini +++ b/tox.ini @@ -429,13 +429,13 @@ commands_pre = -e {toxinidir}/opentelemetry-python-core/exporter/opentelemetry-exporter-opencensus docker-compose up -d python check_availability.py + commands = pytest {posargs} commands_post = docker-compose down -v - [testenv:generate] deps = -r {toxinidir}/gen-requirements.txt From b70049d60a455ff30b50865e86a7640c145a9cb9 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Mon, 7 Jun 2021 11:27:42 -0700 Subject: [PATCH 09/12] resolve pylint and changed return type to str resolve pylint Fixed linting and changed return type to str --- .../instrumentation/aiohttp_client/__init__.py | 8 ++++---- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 +- .../opentelemetry/instrumentation/requests/__init__.py | 2 +- .../src/opentelemetry/instrumentation/tornado/client.py | 2 +- .../src/opentelemetry/instrumentation/urllib/__init__.py | 2 +- .../tests/test_urllib_integration.py | 2 ++ .../src/opentelemetry/instrumentation/wsgi/__init__.py | 4 ++-- .../src/opentelemetry/instrumentation/utils.py | 4 ++-- 8 files changed, 14 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 088fd6bb85..4ee758f9d1 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -172,13 +172,13 @@ async def on_request_start( ) if trace_config_ctx.span.is_recording(): - url = remove_url_credentials(params.url) - attributes = { SpanAttributes.HTTP_METHOD: http_method, - SpanAttributes.HTTP_URL: trace_config_ctx.url_filter(url) + SpanAttributes.HTTP_URL: remove_url_credentials( + trace_config_ctx.url_filter(params.url) + ) if callable(trace_config_ctx.url_filter) - else str(url), + else remove_url_credentials(str(params.url)), } for key, value in attributes.items(): trace_config_ctx.span.set_attribute(key, value) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index ae3063e3ea..d64968f7f5 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -89,7 +89,7 @@ def collect_request_attributes(scope): SpanAttributes.NET_HOST_PORT: port, SpanAttributes.HTTP_FLAVOR: scope.get("http_version"), SpanAttributes.HTTP_TARGET: scope.get("path"), - SpanAttributes.HTTP_URL: str(remove_url_credentials(http_url)), + SpanAttributes.HTTP_URL: remove_url_credentials(http_url), } http_method = scope.get("method") if http_method: diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index b758bd993e..fc0eb3e30d 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -127,7 +127,7 @@ def _instrumented_requests_call( if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) - url = str(remove_url_credentials(url)) + url = remove_url_credentials(url) labels = {} labels[SpanAttributes.HTTP_METHOD] = method diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py index 11af0db9ef..cf33803b53 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py @@ -64,7 +64,7 @@ def fetch_async(tracer, request_hook, response_hook, func, _, args, kwargs): if span.is_recording(): attributes = { - SpanAttributes.HTTP_URL: str(remove_url_credentials(request.url)), + SpanAttributes.HTTP_URL: remove_url_credentials(request.url), SpanAttributes.HTTP_METHOD: request.method, } for key, value in attributes.items(): diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index f4963b8def..af3636bc1b 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -145,7 +145,7 @@ def _instrumented_open_call( if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) - url = str(remove_url_credentials(url)) + url = remove_url_credentials(url) labels = { SpanAttributes.HTTP_METHOD: method, diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 1f11c58b3b..d35ceb4c9e 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -36,6 +36,8 @@ from opentelemetry.trace import StatusCode # pylint: disable=too-many-public-methods + + class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=no-member diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index a386991036..011acd5772 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -131,8 +131,8 @@ def collect_request_attributes(environ): if target is not None: result[SpanAttributes.HTTP_TARGET] = target else: - result[SpanAttributes.HTTP_URL] = str( - remove_url_credentials(wsgiref_util.request_uri(environ)) + result[SpanAttributes.HTTP_URL] = remove_url_credentials( + wsgiref_util.request_uri(environ) ) remote_addr = environ.get("REMOTE_ADDR") diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 1488a1e2b8..541400bd67 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -63,10 +63,10 @@ def unwrap(obj, attr: str): setattr(obj, attr, func.__wrapped__) -def remove_url_credentials(url: str) -> URL: +def remove_url_credentials(url: str) -> str: """Given a string url, attempt to remove the username and password""" try: - url = URL(url).with_user(None) + url = str(URL(url).with_user(None)) except ValueError: # invalid url was passed pass return url From 063759b045e7bfedf5c53eb1bfcbe95f8eaa013d Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Mon, 7 Jun 2021 13:45:24 -0700 Subject: [PATCH 10/12] Created changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index feb865a13c..70c756db91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#530](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/530)) - Fix weak reference error for pyodbc cursor in SQLAlchemy instrumentation. ([#469](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/469)) +- Implemented specification that HTTP span attributes must not contain username and password. + ([#538](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/538)) ## [0.22b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.3.0-0.22b0) - 2021-06-01 From 62c11fc56a19d7bf5bfb883143a339eb3ba02e08 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Tue, 8 Jun 2021 12:43:02 -0700 Subject: [PATCH 11/12] Changed implementation to use urllib parse to limit dependencies --- opentelemetry-instrumentation/setup.cfg | 1 - .../opentelemetry/instrumentation/utils.py | 33 +++++++++++++++---- tox.ini | 1 - 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/opentelemetry-instrumentation/setup.cfg b/opentelemetry-instrumentation/setup.cfg index 421e4497bb..a00673cca8 100644 --- a/opentelemetry-instrumentation/setup.cfg +++ b/opentelemetry-instrumentation/setup.cfg @@ -43,7 +43,6 @@ include_package_data = True install_requires = opentelemetry-api == 1.4.0.dev0 wrapt >= 1.0.0, < 2.0.0 - yarl ~= 1.6 [options.packages.find] where = src diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 541400bd67..6ea36869a3 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -13,9 +13,9 @@ # limitations under the License. from typing import Dict, Sequence +from urllib.parse import urlparse, urlunparse from wrapt import ObjectProxy -from yarl import URL from opentelemetry.trace import StatusCode @@ -64,9 +64,30 @@ def unwrap(obj, attr: str): def remove_url_credentials(url: str) -> str: - """Given a string url, attempt to remove the username and password""" - try: - url = str(URL(url).with_user(None)) - except ValueError: # invalid url was passed - pass + """Given a string url, remove the username and password only if it is a valid url""" + + def validate_url(url): + try: + parsed = urlparse(url) + return all([parsed.scheme, parsed.netloc]) + except ValueError: # invalid url was passed + return False + + if validate_url(url): + parsed_url = urlparse(url) + netloc = ( + (":".join(((parsed_url.hostname or ""), str(parsed_url.port)))) + if parsed_url.port + else (parsed_url.hostname or "") + ) + return urlunparse( + ( + parsed_url.scheme, + netloc, + parsed_url.path, + parsed_url.params, + parsed_url.query, + parsed_url.fragment, + ) + ) return url diff --git a/tox.ini b/tox.ini index b665a914bb..57286c5e7c 100644 --- a/tox.ini +++ b/tox.ini @@ -405,7 +405,6 @@ deps = protobuf>=3.13.0 requests==2.25.0 pyodbc~=4.0.30 - yarl ~= 1.6 changedir = tests/opentelemetry-docker-tests/tests From 685302ab036c72efdc19498df8f470c75b7cfd32 Mon Sep 17 00:00:00 2001 From: Ryo Kather Date: Wed, 9 Jun 2021 18:22:10 -0700 Subject: [PATCH 12/12] Moved remove credential function to http utils --- .../setup.cfg | 1 + .../aiohttp_client/__init__.py | 2 +- .../setup.cfg | 1 + .../instrumentation/asgi/__init__.py | 6 ++-- .../setup.cfg | 1 + .../instrumentation/requests/__init__.py | 6 ++-- .../instrumentation/tornado/client.py | 6 ++-- .../setup.cfg | 1 + .../instrumentation/urllib/__init__.py | 6 ++-- .../setup.cfg | 1 + .../instrumentation/wsgi/__init__.py | 6 ++-- .../opentelemetry/instrumentation/utils.py | 31 ------------------- tox.ini | 2 +- .../src/opentelemetry/util/http/__init__.py | 28 +++++++++++++++++ 14 files changed, 45 insertions(+), 53 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/setup.cfg b/instrumentation/opentelemetry-instrumentation-aiohttp-client/setup.cfg index 21a3d1160a..4de609a95b 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/setup.cfg +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/setup.cfg @@ -41,6 +41,7 @@ install_requires = opentelemetry-api == 1.4.0.dev0 opentelemetry-semantic-conventions == 0.23.dev0 opentelemetry-instrumentation == 0.23.dev0 + opentelemetry-util-http == 0.23.dev0 wrapt >= 1.0.0, < 2.0.0 [options.packages.find] diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 4ee758f9d1..b9441ca108 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -76,13 +76,13 @@ def strip_query_params(url: yarl.URL) -> str: from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.utils import ( http_status_to_status_code, - remove_url_credentials, unwrap, ) from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, TracerProvider, get_tracer from opentelemetry.trace.status import Status, StatusCode +from opentelemetry.util.http import remove_url_credentials _UrlFilterT = typing.Optional[typing.Callable[[str], str]] _SpanNameT = typing.Optional[ diff --git a/instrumentation/opentelemetry-instrumentation-asgi/setup.cfg b/instrumentation/opentelemetry-instrumentation-asgi/setup.cfg index 48c7247503..a47609d423 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/setup.cfg +++ b/instrumentation/opentelemetry-instrumentation-asgi/setup.cfg @@ -41,6 +41,7 @@ install_requires = opentelemetry-api == 1.4.0.dev0 opentelemetry-semantic-conventions == 0.23.dev0 opentelemetry-instrumentation == 0.23.dev0 + opentelemetry-util-http == 0.23.dev0 [options.extras_require] test = diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index d64968f7f5..88835674a2 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -27,14 +27,12 @@ from opentelemetry import context, trace from opentelemetry.instrumentation.asgi.version import __version__ # noqa -from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, - remove_url_credentials, -) +from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.propagate import extract from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status, StatusCode +from opentelemetry.util.http import remove_url_credentials class ASGIGetter(Getter): diff --git a/instrumentation/opentelemetry-instrumentation-requests/setup.cfg b/instrumentation/opentelemetry-instrumentation-requests/setup.cfg index fcac4718fe..2decc00bac 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/setup.cfg +++ b/instrumentation/opentelemetry-instrumentation-requests/setup.cfg @@ -41,6 +41,7 @@ install_requires = opentelemetry-api == 1.4.0.dev0 opentelemetry-semantic-conventions == 0.23.dev0 opentelemetry-instrumentation == 0.23.dev0 + opentelemetry-util-http == 0.23.dev0 [options.extras_require] test = diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index fc0eb3e30d..451dca22a8 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -45,14 +45,12 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.requests.package import _instruments from opentelemetry.instrumentation.requests.version import __version__ -from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, - remove_url_credentials, -) +from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, get_tracer from opentelemetry.trace.status import Status +from opentelemetry.util.http import remove_url_credentials # A key to a context variable to avoid creating duplicate spans when instrumenting # both, Session.request and Session.send, since Session.request calls into Session.send diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py index cf33803b53..7fe85288ab 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py @@ -17,14 +17,12 @@ from tornado.httpclient import HTTPError, HTTPRequest from opentelemetry import trace -from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, - remove_url_credentials, -) +from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status from opentelemetry.util._time import _time_ns +from opentelemetry.util.http import remove_url_credentials def _normalize_request(args, kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-urllib/setup.cfg b/instrumentation/opentelemetry-instrumentation-urllib/setup.cfg index 3830431f04..87fba0b355 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/setup.cfg +++ b/instrumentation/opentelemetry-instrumentation-urllib/setup.cfg @@ -41,6 +41,7 @@ install_requires = opentelemetry-api == 1.4.0.dev0 opentelemetry-semantic-conventions == 0.23.dev0 opentelemetry-instrumentation == 0.23.dev0 + opentelemetry-util-http == 0.23.dev0 [options.extras_require] test = diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index af3636bc1b..1081a67d13 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -48,14 +48,12 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib.package import _instruments from opentelemetry.instrumentation.urllib.version import __version__ -from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, - remove_url_credentials, -) +from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, get_tracer from opentelemetry.trace.status import Status +from opentelemetry.util.http import remove_url_credentials # A key to a context variable to avoid creating duplicate spans when instrumenting _SUPPRESS_HTTP_INSTRUMENTATION_KEY = "suppress_http_instrumentation" diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/setup.cfg b/instrumentation/opentelemetry-instrumentation-wsgi/setup.cfg index 31462974f8..0a924f6484 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/setup.cfg +++ b/instrumentation/opentelemetry-instrumentation-wsgi/setup.cfg @@ -41,6 +41,7 @@ install_requires = opentelemetry-api == 1.4.0.dev0 opentelemetry-semantic-conventions == 0.23.dev0 opentelemetry-instrumentation == 0.23.dev0 + opentelemetry-util-http == 0.23.dev0 [options.extras_require] test = diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 011acd5772..25fbc831f0 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -59,15 +59,13 @@ def hello(): import wsgiref.util as wsgiref_util from opentelemetry import context, trace -from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, - remove_url_credentials, -) +from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.instrumentation.wsgi.version import __version__ from opentelemetry.propagate import extract from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status, StatusCode +from opentelemetry.util.http import remove_url_credentials _HTTP_VERSION_PREFIX = "HTTP/" _CARRIER_KEY_PREFIX = "HTTP_" diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 6ea36869a3..dec070570f 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -13,7 +13,6 @@ # limitations under the License. from typing import Dict, Sequence -from urllib.parse import urlparse, urlunparse from wrapt import ObjectProxy @@ -61,33 +60,3 @@ def unwrap(obj, attr: str): func = getattr(obj, attr, None) if func and isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): setattr(obj, attr, func.__wrapped__) - - -def remove_url_credentials(url: str) -> str: - """Given a string url, remove the username and password only if it is a valid url""" - - def validate_url(url): - try: - parsed = urlparse(url) - return all([parsed.scheme, parsed.netloc]) - except ValueError: # invalid url was passed - return False - - if validate_url(url): - parsed_url = urlparse(url) - netloc = ( - (":".join(((parsed_url.hostname or ""), str(parsed_url.port)))) - if parsed_url.port - else (parsed_url.hostname or "") - ) - return urlunparse( - ( - parsed_url.scheme, - netloc, - parsed_url.path, - parsed_url.params, - parsed_url.query, - parsed_url.fragment, - ) - ) - return url diff --git a/tox.ini b/tox.ini index 57286c5e7c..501f127918 100644 --- a/tox.ini +++ b/tox.ini @@ -232,7 +232,7 @@ commands_pre = grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test] - falcon,flask,django,pyramid,tornado,starlette,fastapi: pip install {toxinidir}/util/opentelemetry-util-http[test] + falcon,flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test] wsgi,falcon,flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] asgi,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index 068511010d..dfa186bf16 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -15,6 +15,7 @@ from os import environ from re import compile as re_compile from re import search +from urllib.parse import urlparse, urlunparse class ExcludeList: @@ -57,3 +58,30 @@ def get_excluded_urls(instrumentation): ] return ExcludeList(excluded_urls) + + +def remove_url_credentials(url: str) -> str: + """Given a string url, remove the username and password only if it is a valid url""" + + try: + parsed = urlparse(url) + if all([parsed.scheme, parsed.netloc]): # checks for valid url + parsed_url = urlparse(url) + netloc = ( + (":".join(((parsed_url.hostname or ""), str(parsed_url.port)))) + if parsed_url.port + else (parsed_url.hostname or "") + ) + return urlunparse( + ( + parsed_url.scheme, + netloc, + parsed_url.path, + parsed_url.params, + parsed_url.query, + parsed_url.fragment, + ) + ) + except ValueError: # an unparseable url was passed + pass + return url