Skip to content

Commit ab9b962

Browse files
committed
fix #4251: wrap BackgroundTasks in their own span
Only the request/ASGI middleware was traced; background tasks were not. Background tasks ran after the response with no span, causing child spans to attach to a closed parent span. This change patches BackgroundTask.__call__ to wrap execution in a new span. Each background task now creates a child span of the request, ensuring a correct trace hierarchy. Add three regression tests for background task instrumentation.
1 parent e1fbc0d commit ab9b962

File tree

2 files changed

+106
-0
lines changed

2 files changed

+106
-0
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
190190

191191
import fastapi
192192
from starlette.applications import Starlette
193+
from starlette.background import BackgroundTask
193194
from starlette.middleware.errors import ServerErrorMiddleware
194195
from starlette.routing import Match, Route
195196
from starlette.types import ASGIApp, Receive, Scope, Send
@@ -399,6 +400,16 @@ async def __call__(
399400
app,
400401
)
401402

403+
if not hasattr(BackgroundTask, "_otel_original_call"):
404+
BackgroundTask._otel_original_call = BackgroundTask.__call__
405+
406+
async def traced_call(self):
407+
span_name = f"BackgroundTask {getattr(self.func, '__name__', self.func.__class__.__name__)}"
408+
with tracer.start_as_current_span(span_name):
409+
return await BackgroundTask._otel_original_call(self)
410+
411+
BackgroundTask.__call__ = traced_call
412+
402413
app._is_instrumented_by_opentelemetry = True
403414
if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
404415
_InstrumentedFastAPI._instrumented_fastapi_apps.add(app)
@@ -416,6 +427,11 @@ def uninstrument_app(app: fastapi.FastAPI):
416427
app.build_middleware_stack = original_build_middleware_stack
417428
del app._original_build_middleware_stack
418429
app.middleware_stack = app.build_middleware_stack()
430+
431+
if hasattr(BackgroundTask, "_otel_original_call"):
432+
BackgroundTask.__call__ = BackgroundTask._otel_original_call
433+
del BackgroundTask._otel_original_call
434+
419435
app._is_instrumented_by_opentelemetry = False
420436

421437
# Remove the app from the set of instrumented apps to avoid calling uninstrument twice

instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@
2525

2626
import fastapi
2727
import pytest
28+
from fastapi.background import BackgroundTasks
2829
from fastapi.middleware.asyncexitstack import AsyncExitStackMiddleware
2930
from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware
3031
from fastapi.responses import JSONResponse, PlainTextResponse
3132
from fastapi.routing import APIRoute
3233
from fastapi.testclient import TestClient
34+
from starlette.background import BackgroundTask
3335
from starlette.routing import Match
3436
from starlette.types import Receive, Scope, Send
3537

@@ -493,6 +495,51 @@ def test_basic_fastapi_call(self):
493495
for span in spans:
494496
self.assertIn("GET /foobar", span.name)
495497

498+
def test_background_task_span_parents_inner_spans(self):
499+
"""Regression test for #4251: spans created inside a FastAPI
500+
BackgroundTask must be children of a dedicated background-task span
501+
instead of the already-closed request span."""
502+
self.memory_exporter.clear()
503+
app = fastapi.FastAPI()
504+
self._instrumentor.instrument_app(app)
505+
tracer = self.tracer_provider.get_tracer(__name__)
506+
507+
async def background_notify():
508+
with tracer.start_as_current_span("inside-background-task"):
509+
pass
510+
511+
@app.post("/checkout")
512+
async def checkout(background_tasks: BackgroundTasks):
513+
background_tasks.add_task(background_notify)
514+
return {"status": "processing"}
515+
516+
with TestClient(app) as client:
517+
response = client.post("/checkout")
518+
self.assertEqual(200, response.status_code)
519+
spans = self.memory_exporter.get_finished_spans()
520+
request_span = next(
521+
span for span in spans if span.name == "POST /checkout"
522+
)
523+
background_span = next(
524+
span
525+
for span in spans
526+
if span.name == "BackgroundTask background_notify"
527+
)
528+
inner_span = next(
529+
span for span in spans if span.name == "inside-background-task"
530+
)
531+
self.assertIsNotNone(background_span.parent)
532+
self.assertEqual(
533+
background_span.parent.span_id,
534+
request_span.context.span_id,
535+
)
536+
self.assertIsNotNone(inner_span.parent)
537+
self.assertEqual(
538+
inner_span.parent.span_id,
539+
background_span.context.span_id,
540+
)
541+
otel_fastapi.FastAPIInstrumentor().uninstrument_app(app)
542+
496543
def test_fastapi_route_attribute_added(self):
497544
"""Ensure that fastapi routes are used as the span name."""
498545
self._client.get("/user/123")
@@ -988,6 +1035,49 @@ def test_basic_post_request_metric_success_both_semconv(self):
9881035
if isinstance(point, NumberDataPoint):
9891036
self.assertEqual(point.value, 0)
9901037

1038+
def test_uninstrument_app_restores_background_task_call(self):
1039+
"""Regression test for #4251: uninstrumentation must restore the
1040+
original BackgroundTask.__call__ after FastAPI patches it."""
1041+
self.assertTrue(hasattr(BackgroundTask, "_otel_original_call"))
1042+
self._instrumentor.uninstrument_app(self._app)
1043+
self.assertFalse(hasattr(BackgroundTask, "_otel_original_call"))
1044+
1045+
def test_background_task_span_not_duplicated_on_double_instrument_app(
1046+
self,
1047+
):
1048+
"""Regression test for #4251: repeated instrument_app calls must not
1049+
wrap BackgroundTask.__call__ multiple times or duplicate spans."""
1050+
self.memory_exporter.clear()
1051+
app = fastapi.FastAPI()
1052+
self._instrumentor.instrument_app(app)
1053+
self._instrumentor.instrument_app(app)
1054+
tracer = self.tracer_provider.get_tracer(__name__)
1055+
1056+
async def background_notify():
1057+
with tracer.start_as_current_span("inside-background-task"):
1058+
pass
1059+
1060+
@app.post("/checkout")
1061+
async def checkout(background_tasks: BackgroundTasks):
1062+
background_tasks.add_task(background_notify)
1063+
return {"status": "processing"}
1064+
1065+
with TestClient(app) as client:
1066+
response = client.post("/checkout")
1067+
self.assertEqual(200, response.status_code)
1068+
spans = self.memory_exporter.get_finished_spans()
1069+
background_spans = [
1070+
span
1071+
for span in spans
1072+
if span.name == "BackgroundTask background_notify"
1073+
]
1074+
inner_spans = [
1075+
span for span in spans if span.name == "inside-background-task"
1076+
]
1077+
self.assertEqual(len(background_spans), 1)
1078+
self.assertEqual(len(inner_spans), 1)
1079+
otel_fastapi.FastAPIInstrumentor().uninstrument_app(app)
1080+
9911081
def test_metric_uninstrument_app(self):
9921082
self._client.get("/foobar")
9931083
self._instrumentor.uninstrument_app(self._app)

0 commit comments

Comments
 (0)