Ensure clean http url

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
 

instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py

@@ -76,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
@@ -173,11 +174,11 @@ async def on_request_start(
         if trace_config_ctx.span.is_recording():
             attributes = {
                 SpanAttributes.HTTP_METHOD: http_method,
-                SpanAttributes.HTTP_URL: trace_config_ctx.url_filter(
-                    params.url
+                SpanAttributes.HTTP_URL: remove_url_credentials(
+                    trace_config_ctx.url_filter(params.url)
                 )
                 if callable(trace_config_ctx.url_filter)
-                else str(params.url),
+                else remove_url_credentials(str(params.url)),
             }
             for key, value in attributes.items():
                 trace_config_ctx.span.set_attribute(key, value)

instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py

@@ -321,6 +321,37 @@ async def request_handler(request):
             ]
         )
 
+    def test_credential_removal(self):
+        trace_configs = [aiohttp_client.create_trace_config()]
+
+        url = "http://username:[email protected]/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"

instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

@@ -27,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
@@ -86,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: http_url,
+        SpanAttributes.HTTP_URL: remove_url_credentials(http_url),
     }
     http_method = scope.get("method")
     if http_method:

instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py

@@ -430,6 +430,14 @@ 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:[email protected]", 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__":
     unittest.main()

instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py

@@ -45,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
@@ -124,6 +127,8 @@ def _instrumented_requests_call(
         if not span_name or not isinstance(span_name, str):
             span_name = get_default_span_name(method)
 
+        url = remove_url_credentials(url)
+
         labels = {}
         labels[SpanAttributes.HTTP_METHOD] = method
         labels[SpanAttributes.HTTP_URL] = url

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_credential_removal(self):
+        new_url = "http://username:[email protected]/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!")

instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py

@@ -17,7 +17,10 @@
 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
@@ -61,7 +64,7 @@ def fetch_async(tracer, request_hook, response_hook, func, _, args, kwargs):
 
     if span.is_recording():
         attributes = {
-            SpanAttributes.HTTP_URL: request.url,
+            SpanAttributes.HTTP_URL: remove_url_credentials(request.url),
             SpanAttributes.HTTP_METHOD: request.method,
         }
         for key, value in attributes.items():

instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py

@@ -455,6 +455,29 @@ def test_response_headers(self):
         self.memory_exporter.clear()
         set_global_response_propagator(orig)
 
+    def test_credential_removal(self):
+        response = self.fetch(
+            "http://username:[email protected]/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):
     _client_request_hook = None

instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py

@@ -48,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
@@ -142,6 +145,8 @@ def _instrumented_open_call(
         if not span_name or not isinstance(span_name, str):
             span_name = get_default_span_name(method)
 
+        url = remove_url_credentials(url)
+
         labels = {
             SpanAttributes.HTTP_METHOD: method,
             SpanAttributes.HTTP_URL: url,

instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py

@@ -35,6 +35,8 @@
 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
@@ -318,6 +320,15 @@ def test_requests_timeout_exception(self, *_, **__):
         span = self.assert_span()
         self.assertEqual(span.status.status_code, StatusCode.ERROR)
 
+    def test_credential_removal(self):
+        url = "http://username:[email protected]/status/200"
+
+        with self.assertRaises(Exception):
+            self.perform_request(url)
+
+        span = self.assert_span()
+        self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL)
+
 
 class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase):
     @staticmethod

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_credential_removal(self):
+        url = "http://username:[email protected]/status/200"
+
+        response = self.perform_request(url)
+        self.assert_success_span(response, self.HTTP_URL)

instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py

@@ -59,7 +59,10 @@ def hello():
 import wsgiref.util as wsgiref_util
 
 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
@@ -128,7 +131,9 @@ 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)
+        result[SpanAttributes.HTTP_URL] = remove_url_credentials(
+            wsgiref_util.request_uri(environ)
+        )
 
     remote_addr = environ.get("REMOTE_ADDR")
     if remote_addr:

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:[email protected]"
+        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(

opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 from typing import Dict, Sequence
+from urllib.parse import urlparse, urlunparse
 
 from wrapt import ObjectProxy
 
@@ -60,3 +61,33 @@ 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

tox.ini

@@ -405,6 +405,7 @@ deps =
   protobuf>=3.13.0
   requests==2.25.0
   pyodbc~=4.0.30
+
 changedir =
   tests/opentelemetry-docker-tests/tests
 
@@ -427,17 +428,17 @@ 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
 
 commands =
   {toxinidir}/scripts/generate_setup.py
-  {toxinidir}/scripts/generate_instrumentation_bootstrap.py
+  {toxinidir}/scripts/generate_instrumentation_bootstrap.py