chore(asgi): revert regression [backport #5849 to 1.13] (#5851)

Backports #5849 to 1.13.

Addresses:

#5818 (comment)
Partially reverts: #5818

We will need to backport this fix to 1.9, 1.10, 1.11, 1.12 and 1.13
release lines.

This regression is unreleased so a release note is not required.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines)
are followed.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] OPTIONAL: PR description includes explicit acknowledgement of the
performance implications of the change as reported in the benchmarks PR
comment.

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.

Co-authored-by: Munir Abdinur <[email protected]>

Author: Yun-Kim

ddtrace/contrib/asgi/middleware.py

@@ -213,7 +213,20 @@ async def wrapped_send(message):
                 trace_utils.set_http_meta(
                     span, self.integration_config, status_code=status_code, response_headers=response_headers
                 )
-                return await send(message)
+                try:
+                    return await send(message)
+                finally:
+                    # Per asgi spec, "more_body" is used if there is still data to send
+                    # Close the span if "http.response.body" has no more data left to send in the
+                    # response.
+                    if (
+                        message.get("type") == "http.response.body"
+                        and not message.get("more_body", False)
+                        # If the span has an error status code delay finishing the span until the
+                        # traceback and exception message is available
+                        and span.error == 0
+                    ):
+                        span.finish()
 
             async def wrapped_blocked_send(message):
                 accept_header = (
@@ -235,7 +248,11 @@ async def wrapped_blocked_send(message):
                     message["body"] = bytes(appsec_utils._get_blocked_template(accept_header), "utf-8")
                     message["more_body"] = False
 
-                return await send(message)
+                try:
+                    return await send(message)
+                finally:
+                    if message.get("type") == "http.response.body" and span.error == 0:
+                        span.finish()
 
             try:
                 if _request_blocked(span):

tests/contrib/asgi/test_asgi.py

@@ -127,10 +127,9 @@ async def tasks_app_with_more_body(scope, receive, send):
         await send({"type": "http.response.body", "body": b"*", "more_body": True})
         assert not request_span.finished
 
-        # assert that the span has not finished after more_body. Span should be finished after
-        # the ddtrace trace middleware is exited.
+        # assert that the span has finished after more_body is False
         await send({"type": "http.response.body", "body": b"*", "more_body": False})
-        assert not request_span.finished
+        assert request_span.finished
         await asyncio.sleep(1)
 
 
@@ -525,7 +524,7 @@ async def test_tasks_asgi_without_more_body(scope, tracer, test_spans):
     assert request_span.span_type == "web"
     # typical duration without background task should be in less than 10 ms
     # duration with background task will take approximately 1.1s
-    assert request_span.duration < 1.1
+    assert request_span.duration < 1
 
 
 @pytest.mark.asyncio
@@ -548,7 +547,7 @@ async def test_tasks_asgi_with_more_body(scope, tracer, test_spans):
     assert request_span.span_type == "web"
     # typical duration without background task should be in less than 10 ms
     # duration with background task will take approximately 1.1s
-    assert request_span.duration < 1.1
+    assert request_span.duration < 1
 
 
 @pytest.mark.asyncio

tests/contrib/fastapi/test_fastapi.py

@@ -629,7 +629,7 @@ def test_background_task(client, tracer, test_spans):
     assert request_span.resource == "GET /asynctask"
     # typical duration without background task should be in less than 10 ms
     # duration with background task will take approximately 1.1s
-    assert request_span.duration < 1.1
+    assert request_span.duration < 1
 
 
 @pytest.mark.parametrize("host", ["hostserver", "hostserver:5454"])

tests/contrib/starlette/test_starlette.py

@@ -534,4 +534,4 @@ def test_background_task(client, tracer, test_spans):
     assert request_span.resource == "GET /backgroundtask"
     # typical duration without background task should be in less than 10ms
     # duration with background task will take approximately 1.1s
-    assert request_span.duration < 1.1
+    assert request_span.duration < 1