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
open-telemetry · Jun 7, 2021 · bf536ad · bf536ad
1 parent 7558742
commit bf536ad
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 22 deletions.
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,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/...ion/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/...ion/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
@@ -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/...opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/...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
@@ -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/...ion/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/...ion/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
@@ -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
@@ -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]