more review updates

Author: majorgreys

ext/opentelemetry-ext-asgi/README.rst

@@ -37,24 +37,23 @@ Usage (Quart)
         app.run(debug=True)
 
 
-Usage (Django)
---------------
+Usage (Django 3.0)
+------------------
 
 Modify the application's ``asgi.py`` file as shown below.
 
 .. code-block:: python
 
     import os
-    import django
-    from channels.routing import get_default_application
+    from django.core.asgi import get_asgi_application
     from opentelemetry.ext.asgi import OpenTelemetryMiddleware
 
-    os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'application.settings')
-    django.setup()
+    os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'asgi_example.settings')
 
-    application = get_default_application()
+    application = get_asgi_application()
     application = OpenTelemetryMiddleware(application)
 
+
 References
 ----------
 

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

@@ -20,6 +20,7 @@
 
 import operator
 import typing
+import urllib
 from functools import wraps
 
 from asgiref.compatibility import guarantee_single_callable
@@ -80,16 +81,16 @@ def collect_request_attributes(scope):
     server = scope.get("server") or ["0.0.0.0", 80]
     port = server[1]
     server_host = server[0] + (":" + str(port) if port != 80 else "")
-    http_url = (
-        scope.get("scheme", "http") + "://" + server_host + scope.get("path", "")
-        if scope.get("scheme") and server_host and scope.get("path")
-        else None
-    )
+    full_path = scope.get("root_path", "") + scope.get("path", "")
+    http_url = scope.get("scheme", "http") + "://" + server_host + full_path
     if scope.get("query_string") and http_url:
-        http_url = http_url + ("?" + scope.get("query_string").decode("utf8"))
+        if isinstance(scope["query_string"], bytes):
+            http_url = http_url + ("?" + scope.get("query_string").decode("utf8"))
+        else:
+            http_url = http_url + ("?" + urllib.parse.unquote(scope.get("query_string")))
 
     result = {
-        "component": scope["type"]
+        "component": scope["type"],
         "http.scheme": scope.get("scheme"),
         "http.host": server_host,
         "host.port": port,
@@ -166,7 +167,7 @@ async def __call__(self, scope, receive, send):
             receive: An awaitable callable yielding dictionaries
             send: An awaitable callable taking a single dictionary as argument.
         """
-        if scope.get("type") not in ("http", "websocket"):
+        if scope["type"] not in ("http", "websocket"):
             return await self.app(scope, receive, send)
 
         token = context.attach(
@@ -187,23 +188,23 @@ async def wrapped_receive():
                         span_name + " asgi." + scope["type"] + ".receive"
                     ) as receive_span:
                         message = await receive()
-                        if payload["type"] == "websocket.receive":
+                        if message["type"] == "websocket.receive":
                             set_status_code(receive_span, 200)
-                        receive_span.set_attribute("type", payload["type"])
-                    return payload
+                        receive_span.set_attribute("type", message["type"])
+                    return message
 
                 @wraps(send)
                 async def wrapped_send(message):
                     with self.tracer.start_as_current_span(
                         span_name + " asgi." + scope["type"] + ".send"
                     ) as send_span:
-                        if payload["type"] == "http.response.start":
-                            status_code = payload["status"]
+                        if message["type"] == "http.response.start":
+                            status_code = message["status"]
                             set_status_code(send_span, status_code)
-                        elif payload["type"] == "websocket.send":
+                        elif message["type"] == "websocket.send":
                             set_status_code(send_span, 200)
-                        send_span.set_attribute("type", payload["type"])
-                        await send(payload)
+                        send_span.set_attribute("type", message["type"])
+                        await send(message)
 
                 await self.app(scope, wrapped_receive, wrapped_send)
         finally:

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

@@ -15,6 +15,7 @@
 import sys
 import unittest
 import unittest.mock as mock
+import urllib
 
 import opentelemetry.ext.asgi as otel_asgi
 from opentelemetry import trace as trace_api
@@ -25,9 +26,9 @@
 
 
 async def http_app(scope, receive, send):
-    payload = await receive()
+    message = await receive()
     assert scope["type"] == "http"
-    if payload.get("type") == "http.request":
+    if message.get("type") == "http.request":
         await send(
             {
                 "type": "http.response.start",
@@ -41,15 +42,15 @@ async def http_app(scope, receive, send):
 async def websocket_app(scope, receive, send):
     assert scope["type"] == "websocket"
     while True:
-        payload = await receive()
-        if payload.get("type") == "websocket.connect":
+        message = await receive()
+        if message.get("type") == "websocket.connect":
             await send({"type": "websocket.accept"})
 
-        if payload.get("type") == "websocket.receive":
-            if payload.get("text") == "ping":
+        if message.get("type") == "websocket.receive":
+            if message.get("text") == "ping":
                 await send({"type": "websocket.send", "text": "pong"})
 
-        if payload.get("type") == "websocket.disconnect":
+        if message.get("type") == "websocket.disconnect":
             break
 
 
@@ -63,9 +64,9 @@ async def simple_asgi(scope, receive, send):
 
 async def error_asgi(scope, receive, send):
     assert isinstance(scope, dict)
-    assert scope.get("type") == "http"
-    payload = await receive()
-    if payload.get("type") == "http.request":
+    assert scope["type"] == "http"
+    message = await receive()
+    if message.get("type") == "http.request":
         try:
             raise ValueError
         except ValueError:
@@ -294,7 +295,7 @@ def setUp(self):
         setup_testing_defaults(self.scope)
         self.span = mock.create_autospec(trace_api.Span, spec_set=True)
 
-    def test_request_attributes(self):
+    def test_query_string_utf8(self):
         self.scope["query_string"] = b"foo=bar"
 
         attrs = otel_asgi.collect_request_attributes(self.scope)
@@ -314,6 +315,26 @@ def test_request_attributes(self):
             },
         )
 
+    def test_query_string_percent_encoding(self):
+        self.scope["query_string"] = urllib.parse.quote(b"foo=bar")
+
+        attrs = otel_asgi.collect_request_attributes(self.scope)
+        self.assertDictEqual(
+            attrs,
+            {
+                "component": "http",
+                "http.method": "GET",
+                "http.host": "127.0.0.1",
+                "http.target": "/",
+                "http.url": "http://127.0.0.1/?foo=bar",
+                "host.port": 80,
+                "http.scheme": "http",
+                "http.flavor": "1.0",
+                "net.peer.ip": "127.0.0.1",
+                "net.peer.port": 32767,
+            },
+        )
+
     def test_response_attributes(self):
         otel_asgi.set_status_code(self.span, 404)
         expected = (mock.call("http.status_code", 404),)

tests/util/src/opentelemetry/test/asgitestutil.py

@@ -1,3 +1,17 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 import asyncio
 
 from asgiref.testing import ApplicationCommunicator
@@ -38,7 +52,7 @@ def tearDown(self):
     def seed_app(self, app):
         self.communicator = ApplicationCommunicator(app, self.scope)
 
-    def send(self, message):
+    def send_input(self, message):
         asyncio.get_event_loop().run_until_complete(
             self.communicator.send_input(message)
         )

tests/util/src/opentelemetry/test/spantestutil.py

@@ -1,3 +1,17 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 import unittest
 from importlib import reload
 

tests/util/src/opentelemetry/test/wsgitestutil.py

@@ -1,3 +1,17 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 import io
 import wsgiref.util as wsgiref_util