Implemented check on aiohttp and wrapped conversions in try catch

open-telemetry · Jun 7, 2021 · 441a0df · 441a0df
1 parent 2e8e14f
commit 441a0df
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 23 deletions.
diff --git a/...strumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/...strumentation-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/...ion/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/...ion/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/...telemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/...telemetry-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
@@ -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/...opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/...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
@@ -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
@@ -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
@@ -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]