Skip to content

Commit 64c6c26

Browse files
authored
fix(starlette): don't crash on functions without __name__ (#8812)
Don't crash when encountering functions without `__name__` or `__module__` attributes. Starlette background tasks can be started with any `callable`, and not all `callable`s (annoyingly) are guaranteed to have `__name__` attributes. Specifically, `functools.partial` generates a function without a `__name__`. There is some debate (see python/cpython#91002) about whether this is the right behavior (it's clearly confusing, since it caused a bug here!) but right now, this makes the whole background task crash due to an attribute error. This PR just tries to get the name/module, and if not available, replaces them with the string `<unknown>` rather than crashing the whole task. (Happy to switch to a different string here, `<unknown>` is just a placeholder that kind of matches python's `__name__` for lambdas (`<lambda>`) ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. - [x] If change touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent 907b7e8 commit 64c6c26

File tree

4 files changed

+51
-14
lines changed

4 files changed

+51
-14
lines changed

ddtrace/contrib/starlette/patch.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,12 @@ def traced_handler(wrapped, instance, args, kwargs):
177177
def _trace_background_tasks(module, pin, wrapped, instance, args, kwargs):
178178
task = get_argument_value(args, kwargs, 0, "func")
179179
current_span = pin.tracer.current_span()
180+
module_name = getattr(module, "__name__", "<unknown>")
181+
task_name = getattr(task, "__name__", "<unknown>")
180182

181183
async def traced_task(*args, **kwargs):
182184
with pin.tracer.start_span(
183-
f"{module.__name__}.background_task", resource=task.__name__, child_of=None, activate=True
185+
f"{module_name}.background_task", resource=task_name, child_of=None, activate=True
184186
) as span:
185187
if current_span:
186188
span.link_span(current_span.context)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
starlette: Fix a bug that crashed background tasks started from functions without a `__name__` attribute

tests/contrib/starlette/app.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import asyncio
2+
import functools
23
from tempfile import NamedTemporaryFile
34
import time
45

@@ -118,6 +119,10 @@ async def background_task(request):
118119

119120
tasks = BackgroundTasks()
120121
tasks.add_task(custom_task, task_arg="hi")
122+
123+
anonymous = functools.partial(custom_task, task_arg="hi")
124+
tasks.add_task(anonymous)
125+
121126
return JSONResponse(jsonmsg, background=tasks)
122127

123128
routes = [

tests/snapshots/tests.contrib.starlette.test_starlette.test_background_task.json

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,42 +2,68 @@
22
{
33
"name": "starlette.background_task",
44
"service": "",
5-
"resource": "custom_task",
5+
"resource": "<unknown>",
66
"trace_id": 0,
77
"span_id": 1,
88
"parent_id": 0,
99
"type": "",
1010
"error": 0,
1111
"meta": {
1212
"_dd.p.dm": "-0",
13-
"_dd.p.tid": "65e7e28000000000",
14-
"_dd.span_links": "[{\"trace_id\": \"65e7e27f0000000002d19f6cebbac350\", \"span_id\": \"61fccfc1f9d153b9\", \"tracestate\": \"dd=s:1;t.dm:-0\", \"flags\": 1}]",
13+
"_dd.p.tid": "660a332900000000",
14+
"_dd.span_links": "[{\"trace_id\": \"660a3327000000008449256363f198a8\", \"span_id\": \"92db06edf6ed92a6\", \"tracestate\": \"dd=s:1;t.dm:-0\", \"flags\": 1}]",
15+
"language": "python",
16+
"runtime-id": "2c92a35bf98a4b02af66c1340b9fc3bc"
17+
},
18+
"metrics": {
19+
"_dd.top_level": 1,
20+
"_dd.tracer_kr": 1.0,
21+
"_sampling_priority_v1": 1,
22+
"process_id": 7548
23+
},
24+
"duration": 2004714626,
25+
"start": 1711944489796649052
26+
}],
27+
[
28+
{
29+
"name": "starlette.background_task",
30+
"service": "",
31+
"resource": "custom_task",
32+
"trace_id": 1,
33+
"span_id": 1,
34+
"parent_id": 0,
35+
"type": "",
36+
"error": 0,
37+
"meta": {
38+
"_dd.p.dm": "-0",
39+
"_dd.p.tid": "660a332700000000",
40+
"_dd.span_links": "[{\"trace_id\": \"660a3327000000008449256363f198a8\", \"span_id\": \"92db06edf6ed92a6\", \"tracestate\": \"dd=s:1;t.dm:-0\", \"flags\": 1}]",
1541
"language": "python",
16-
"runtime-id": "5444022027b74fcead7ac4f888aaa621"
42+
"runtime-id": "2c92a35bf98a4b02af66c1340b9fc3bc"
1743
},
1844
"metrics": {
1945
"_dd.top_level": 1,
2046
"_dd.tracer_kr": 1.0,
2147
"_sampling_priority_v1": 1,
22-
"process_id": 85074
48+
"process_id": 7548
2349
},
24-
"duration": 2001259000,
25-
"start": 1709695616000325000
50+
"duration": 2003761042,
51+
"start": 1711944487791773926
2652
}],
2753
[
2854
{
2955
"name": "starlette.request",
3056
"service": "starlette",
3157
"resource": "GET /backgroundtask",
32-
"trace_id": 1,
58+
"trace_id": 2,
3359
"span_id": 1,
3460
"parent_id": 0,
3561
"type": "web",
3662
"error": 0,
3763
"meta": {
3864
"_dd.base_service": "",
3965
"_dd.p.dm": "-0",
40-
"_dd.p.tid": "65e7e27f00000000",
66+
"_dd.p.tid": "660a332700000000",
4167
"component": "starlette",
4268
"http.method": "GET",
4369
"http.route": "/backgroundtask",
@@ -46,15 +72,15 @@
4672
"http.useragent": "testclient",
4773
"http.version": "1.1",
4874
"language": "python",
49-
"runtime-id": "5444022027b74fcead7ac4f888aaa621",
75+
"runtime-id": "2c92a35bf98a4b02af66c1340b9fc3bc",
5076
"span.kind": "server"
5177
},
5278
"metrics": {
5379
"_dd.top_level": 1,
5480
"_dd.tracer_kr": 1.0,
5581
"_sampling_priority_v1": 1,
56-
"process_id": 85074
82+
"process_id": 7548
5783
},
58-
"duration": 314000,
59-
"start": 1709695615999912000
84+
"duration": 501334,
85+
"start": 1711944487791160467
6086
}]]

0 commit comments

Comments
 (0)