Skip to content

Commit d65500d

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 d65500d

File tree

2 files changed

+101
-0
lines changed

2 files changed

+101
-0
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
194194
from starlette.routing import Match, Route
195195
from starlette.types import ASGIApp, Receive, Scope, Send
196196

197+
from starlette.background import BackgroundTask
198+
199+
from opentelemetry import trace
200+
197201
from opentelemetry.instrumentation._semconv import (
198202
_get_schema_url,
199203
_OpenTelemetrySemanticConventionStability,
@@ -399,6 +403,16 @@ async def __call__(
399403
app,
400404
)
401405

406+
if not hasattr(BackgroundTask, "_otel_original_call"):
407+
BackgroundTask._otel_original_call = BackgroundTask.__call__
408+
409+
async def traced_call(self):
410+
span_name = f"BackgroundTask {getattr(self.func, '__name__', self.func.__class__.__name__)}"
411+
with tracer.start_as_current_span(span_name):
412+
return await BackgroundTask._otel_original_call(self)
413+
414+
BackgroundTask.__call__ = traced_call
415+
402416
app._is_instrumented_by_opentelemetry = True
403417
if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
404418
_InstrumentedFastAPI._instrumented_fastapi_apps.add(app)
@@ -416,6 +430,11 @@ def uninstrument_app(app: fastapi.FastAPI):
416430
app.build_middleware_stack = original_build_middleware_stack
417431
del app._original_build_middleware_stack
418432
app.middleware_stack = app.build_middleware_stack()
433+
434+
if hasattr(BackgroundTask, "_otel_original_call"):
435+
BackgroundTask.__call__ = BackgroundTask._otel_original_call
436+
del BackgroundTask._otel_original_call
437+
419438
app._is_instrumented_by_opentelemetry = False
420439

421440
# 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: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware
3030
from fastapi.responses import JSONResponse, PlainTextResponse
3131
from fastapi.routing import APIRoute
32+
from fastapi import BackgroundTasks
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,48 @@ 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+
async def background_notify():
507+
with tracer.start_as_current_span("inside-background-task"):
508+
pass
509+
@app.post("/checkout")
510+
async def checkout(background_tasks: fastapi.BackgroundTasks):
511+
background_tasks.add_task(background_notify)
512+
return {"status": "processing"}
513+
with TestClient(app) as client:
514+
response = client.post("/checkout")
515+
self.assertEqual(200, response.status_code)
516+
spans = self.memory_exporter.get_finished_spans()
517+
request_span = next(
518+
span for span in spans if span.name == "POST /checkout"
519+
)
520+
background_span = next(
521+
span
522+
for span in spans
523+
if span.name == "BackgroundTask background_notify"
524+
)
525+
inner_span = next(
526+
span for span in spans if span.name == "inside-background-task"
527+
)
528+
self.assertIsNotNone(background_span.parent)
529+
self.assertEqual(
530+
background_span.parent.span_id,
531+
request_span.context.span_id,
532+
)
533+
self.assertIsNotNone(inner_span.parent)
534+
self.assertEqual(
535+
inner_span.parent.span_id,
536+
background_span.context.span_id,
537+
)
538+
otel_fastapi.FastAPIInstrumentor().uninstrument_app(app)
539+
496540
def test_fastapi_route_attribute_added(self):
497541
"""Ensure that fastapi routes are used as the span name."""
498542
self._client.get("/user/123")
@@ -988,6 +1032,44 @@ def test_basic_post_request_metric_success_both_semconv(self):
9881032
if isinstance(point, NumberDataPoint):
9891033
self.assertEqual(point.value, 0)
9901034

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

0 commit comments

Comments
 (0)