Merge branch 'main' into emptyroute

Author: shalevr

Diff for: CHANGELOG.md

@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `opentelemetry-instrumentation-django` Fix empty span name when using
   `path("", ...)` ([#1788](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1788)
+- Fix falcon instrumentation's usage of Span Status to only set the description if the status code is ERROR.
+  ([#1840](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1840))
+- Instrument all httpx versions >= 0.18. ([#1748](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1748))
 
 ## Version 1.18.0/0.39b0 (2023-05-10)
 
@@ -26,9 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#1778](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1778))
 - Add `excluded_urls` functionality to `urllib` and `urllib3` instrumentations
   ([#1733](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1733))
-- Make Django request span attributes available for `start_span`. 
+- Make Django request span attributes available for `start_span`.
   ([#1730](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1730))
-- Make ASGI request span attributes available for `start_span`. 
+- Make ASGI request span attributes available for `start_span`.
   ([#1762](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1762))
 - `opentelemetry-instrumentation-celery` Add support for anonymous tasks.
   ([#1407](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1407))
@@ -43,12 +46,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Fix redis db.statements to be sanitized by default
+  ([#1778](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1778))
 - Fix elasticsearch db.statement attribute to be sanitized by default
   ([#1758](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1758))
 - Fix `AttributeError` when AWS Lambda handler receives a list event
   ([#1738](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1738))
 - Fix `None does not implement middleware` error when there are no middlewares registered
   ([#1766](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1766))
+- Fix Flask instrumentation to only close the span if it was created by the same request context.
+  ([#1692](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1692))
+
+### Changed
+- Update HTTP server/client instrumentation span names to comply with spec
+  ([#1759](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1759)
 
 ## Version 1.17.0/0.38b0 (2023-03-22)
 

Diff for: dev-requirements.txt

@@ -14,7 +14,7 @@ bleach==4.1.0 # transient dependency for readme-renderer
 grpcio-tools==1.29.0
 mypy-protobuf>=1.23
 protobuf~=3.13
-markupsafe==2.0.1
+markupsafe>=2.0.1
 codespell==2.1.0
 requests==2.28.1
 ruamel.yaml==0.17.21

Diff for: instrumentation/opentelemetry-instrumentation-aiohttp-client/pyproject.toml

@@ -38,6 +38,7 @@ instruments = [
 ]
 test = [
   "opentelemetry-instrumentation-aiohttp-client[instruments]",
+  "http-server-mock"
 ]
 
 [project.entry-points.opentelemetry_instrumentor]

Diff for: instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py

@@ -179,7 +179,7 @@ async def on_request_start(
             return
 
         http_method = params.method.upper()
-        request_span_name = f"HTTP {http_method}"
+        request_span_name = f"{http_method}"
         request_url = (
             remove_url_credentials(trace_config_ctx.url_filter(params.url))
             if callable(trace_config_ctx.url_filter)

Diff for: instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py

@@ -23,6 +23,7 @@
 import aiohttp
 import aiohttp.test_utils
 import yarl
+from http_server_mock import HttpServerMock
 from pkg_resources import iter_entry_points
 
 from opentelemetry import context
@@ -118,7 +119,7 @@ def test_status_codes(self):
                 self.assert_spans(
                     [
                         (
-                            "HTTP GET",
+                            "GET",
                             (span_status, None),
                             {
                                 SpanAttributes.HTTP_METHOD: "GET",
@@ -212,7 +213,7 @@ def strip_query_params(url: yarl.URL) -> str:
         self.assert_spans(
             [
                 (
-                    "HTTP GET",
+                    "GET",
                     (StatusCode.UNSET, None),
                     {
                         SpanAttributes.HTTP_METHOD: "GET",
@@ -246,7 +247,7 @@ async def do_request(url):
             self.assert_spans(
                 [
                     (
-                        "HTTP GET",
+                        "GET",
                         (expected_status, None),
                         {
                             SpanAttributes.HTTP_METHOD: "GET",
@@ -273,7 +274,7 @@ async def request_handler(request):
         self.assert_spans(
             [
                 (
-                    "HTTP GET",
+                    "GET",
                     (StatusCode.ERROR, None),
                     {
                         SpanAttributes.HTTP_METHOD: "GET",
@@ -300,7 +301,7 @@ async def request_handler(request):
         self.assert_spans(
             [
                 (
-                    "HTTP GET",
+                    "GET",
                     (StatusCode.ERROR, None),
                     {
                         SpanAttributes.HTTP_METHOD: "GET",
@@ -313,27 +314,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):
+        app = HttpServerMock("test_credential_removal")
 
-            async def do_request(url):
-                async with aiohttp.ClientSession(
-                    trace_configs=trace_configs,
-                ) as session:
-                    async with session.get(url):
-                        pass
+        @app.route("/status/200")
+        def index():
+            return "hello"
 
-            loop = asyncio.get_event_loop()
-            loop.run_until_complete(do_request(url))
+        url = "http://username:password@localhost:5000/status/200"
+
+        with app.run("localhost", 5000):
+            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",
+                    "GET",
                     (StatusCode.UNSET, None),
                     {
                         SpanAttributes.HTTP_METHOD: "GET",
-                        SpanAttributes.HTTP_URL: "http://httpbin.org/status/200",
+                        SpanAttributes.HTTP_URL: (
+                            "http://localhost:5000/status/200"
+                        ),
                         SpanAttributes.HTTP_STATUS_CODE: int(HTTPStatus.OK),
                     },
                 )
@@ -380,6 +391,7 @@ def test_instrument(self):
             self.get_default_request(), self.URL, self.default_handler
         )
         span = self.assert_spans(1)
+        self.assertEqual("GET", span.name)
         self.assertEqual("GET", span.attributes[SpanAttributes.HTTP_METHOD])
         self.assertEqual(
             f"http://{host}:{port}/test-path",

Diff for: instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

@@ -415,18 +415,23 @@ def set_status_code(span, status_code):
 
 
 def get_default_span_details(scope: dict) -> Tuple[str, dict]:
-    """Default implementation for get_default_span_details
+    """
+    Default span name is the HTTP method and URL path, or just the method.
+    https://github.com/open-telemetry/opentelemetry-specification/pull/3165
+    https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#name
+
     Args:
         scope: the ASGI scope dictionary
     Returns:
         a tuple of the span name, and any attributes to attach to the span.
     """
-    span_name = (
-        scope.get("path", "").strip()
-        or f"HTTP {scope.get('method', '').strip()}"
-    )
-
-    return span_name, {}
+    path = scope.get("path", "").strip()
+    method = scope.get("method", "").strip()
+    if method and path:  # http
+        return f"{method} {path}", {}
+    if path:  # websocket
+        return path, {}
+    return method, {}  # http with no path
 
 
 def _collect_target_attribute(

Diff for: instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py

@@ -142,25 +142,25 @@ def validate_outputs(self, outputs, error=None, modifiers=None):
         self.assertEqual(len(span_list), 4)
         expected = [
             {
-                "name": "/ http receive",
+                "name": "GET / http receive",
                 "kind": trace_api.SpanKind.INTERNAL,
                 "attributes": {"type": "http.request"},
             },
             {
-                "name": "/ http send",
+                "name": "GET / http send",
                 "kind": trace_api.SpanKind.INTERNAL,
                 "attributes": {
                     SpanAttributes.HTTP_STATUS_CODE: 200,
                     "type": "http.response.start",
                 },
             },
             {
-                "name": "/ http send",
+                "name": "GET / http send",
                 "kind": trace_api.SpanKind.INTERNAL,
                 "attributes": {"type": "http.response.body"},
             },
             {
-                "name": "/",
+                "name": "GET /",
                 "kind": trace_api.SpanKind.SERVER,
                 "attributes": {
                     SpanAttributes.HTTP_METHOD: "GET",
@@ -231,7 +231,7 @@ def update_expected_span_name(expected):
                     entry["name"] = span_name
                 else:
                     entry["name"] = " ".join(
-                        [span_name] + entry["name"].split(" ")[1:]
+                        [span_name] + entry["name"].split(" ")[2:]
                     )
             return expected
 
@@ -493,9 +493,9 @@ def update_expected_hook_results(expected):
             for entry in expected:
                 if entry["kind"] == trace_api.SpanKind.SERVER:
                     entry["name"] = "name from server hook"
-                elif entry["name"] == "/ http receive":
+                elif entry["name"] == "GET / http receive":
                     entry["name"] = "name from client request hook"
-                elif entry["name"] == "/ http send":
+                elif entry["name"] == "GET / http send":
                     entry["attributes"].update({"attr-from-hook": "value"})
             return expected
 
@@ -705,11 +705,11 @@ def test_response_attributes_invalid_status_code(self):
         self.assertEqual(self.span.set_status.call_count, 1)
 
     def test_credential_removal(self):
-        self.scope["server"] = ("username:password@httpbin.org", 80)
+        self.scope["server"] = ("username:password@mock", 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"
+            attrs[SpanAttributes.HTTP_URL], "http://mock/status/200"
         )
 
     def test_collect_target_attribute_missing(self):

Diff for: instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py

@@ -173,18 +173,12 @@ def _get_span_name(request):
                 match = resolve(request.path)
 
             if hasattr(match, "route") and match.route:
-                return match.route
+                return f"{request.method} {match.route}"
 
-            # Instead of using `view_name`, better to use `_func_name` as some applications can use similar
-            # view names in different modules
-            if hasattr(match, "_func_name"):
-                return match._func_name  # pylint: disable=protected-access
-
-            # Fallback for safety as `_func_name` private field
-            return match.view_name
+            return request.method
 
         except Resolver404:
-            return f"HTTP {request.method}"
+            return request.method
 
     # pylint: disable=too-many-locals
     def process_request(self, request):

Diff for: instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py

@@ -154,9 +154,9 @@ def test_templated_route_get(self):
 
         self.assertEqual(
             span.name,
-            "^route/(?P<year>[0-9]{4})/template/$"
+            "GET ^route/(?P<year>[0-9]{4})/template/$"
             if DJANGO_2_2
-            else "tests.views.traced_template",
+            else "GET",
         )
         self.assertEqual(span.kind, SpanKind.SERVER)
         self.assertEqual(span.status.status_code, StatusCode.UNSET)
@@ -181,9 +181,7 @@ def test_traced_get(self):
 
         span = spans[0]
 
-        self.assertEqual(
-            span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced"
-        )
+        self.assertEqual(span.name, "GET ^traced/" if DJANGO_2_2 else "GET")
         self.assertEqual(span.kind, SpanKind.SERVER)
         self.assertEqual(span.status.status_code, StatusCode.UNSET)
         self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
@@ -229,9 +227,7 @@ def test_traced_post(self):
 
         span = spans[0]
 
-        self.assertEqual(
-            span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced"
-        )
+        self.assertEqual(span.name, "POST ^traced/" if DJANGO_2_2 else "POST")
         self.assertEqual(span.kind, SpanKind.SERVER)
         self.assertEqual(span.status.status_code, StatusCode.UNSET)
         self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST")
@@ -255,9 +251,7 @@ def test_error(self):
 
         span = spans[0]
 
-        self.assertEqual(
-            span.name, "^error/" if DJANGO_2_2 else "tests.views.error"
-        )
+        self.assertEqual(span.name, "GET ^error/" if DJANGO_2_2 else "GET")
         self.assertEqual(span.kind, SpanKind.SERVER)
         self.assertEqual(span.status.status_code, StatusCode.ERROR)
         self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
@@ -321,9 +315,7 @@ def test_span_name(self):
         span = span_list[0]
         self.assertEqual(
             span.name,
-            "^span_name/([0-9]{4})/$"
-            if DJANGO_2_2
-            else "tests.views.route_span_name",
+            "GET ^span_name/([0-9]{4})/$" if DJANGO_2_2 else "GET",
         )
 
     def test_span_name_for_query_string(self):
@@ -337,9 +329,7 @@ def test_span_name_for_query_string(self):
         span = span_list[0]
         self.assertEqual(
             span.name,
-            "^span_name/([0-9]{4})/$"
-            if DJANGO_2_2
-            else "tests.views.route_span_name",
+            "GET ^span_name/([0-9]{4})/$" if DJANGO_2_2 else "GET",
         )
 
     def test_span_name_404(self):
@@ -348,7 +338,7 @@ def test_span_name_404(self):
         self.assertEqual(len(span_list), 1)
 
         span = span_list[0]
-        self.assertEqual(span.name, "HTTP GET")
+        self.assertEqual(span.name, "GET")
 
     def test_traced_request_attrs(self):
         Client().get("/span_name/1234/", CONTENT_TYPE="test/ct")