Skip to content

Commit 07222fb

Browse files
authored
fix(aws_lambda): warn when impending timeout but all traces have been finished [backport #5224] (#5265)
Backport of #5224 to 1.9 ## Description The root cause might be that customer is finishing all its spans, even the one provided by `datadog-lambda`. Then hitting an impending timeout, which makes the contrib `aws_lambda` to try to tag an error in the root span. After this, the contrib might try to close all unfinished spans too. Which in turn, if there's no root span, there will not be current span, therefore, adding a conditional for both root span and current span to act only if they are not `None`. Discovered in issue #5220 ## Testing strategy Added a unit test which finishes `aws_lambda` root span and then times out. Expecting not to crash, and to not tag the error. ## Risk User will not be getting an Impending Timeout error tagged in their root span, but they will be receiving a warning message in their logs. ## 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] Author is aware of the performance implications of this PR 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 is aware of, and discussed the performance implications of this PR as reported in the benchmarks PR comment.
1 parent e62b427 commit 07222fb

File tree

5 files changed

+65
-5
lines changed

5 files changed

+65
-5
lines changed

ddtrace/contrib/aws_lambda/patch.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@ def _crash_flush(_, __):
2020
Tags the current root span with an Impending Timeout error.
2121
Finishes spans with ancestors from the current span.
2222
"""
23-
2423
root_span = tracer.current_root_span()
25-
root_span.error = 1
26-
root_span.set_tag_str(ERROR_MSG, "Datadog detected an Impending Timeout")
27-
root_span.set_tag_str(ERROR_TYPE, "Impending Timeout")
24+
if root_span is not None:
25+
root_span.error = 1
26+
root_span.set_tag_str(ERROR_MSG, "Datadog detected an Impending Timeout")
27+
root_span.set_tag_str(ERROR_TYPE, "Impending Timeout")
28+
else:
29+
log.warning("An impending timeout was reached, but no root span was found. No error will be tagged.")
2830

2931
current_span = tracer.current_span()
30-
current_span.finish_with_ancestors()
32+
if current_span is not None:
33+
current_span.finish_with_ancestors()
3134

3235

3336
def _handle_signal(sig, f):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
aws_lambda: Resolves an exception not being handled, which occurs when no root span is found before a lambda times out.

tests/contrib/aws_lambda/handlers.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ def timeout():
2828
return {"success": False}
2929

3030

31+
def finishing_spans_early_handler(event, context):
32+
root_span = tracer.current_root_span()
33+
root_span.finish()
34+
35+
time.sleep(0.5)
36+
37+
return {"success": False}
38+
39+
3140
def datadog(_handler):
3241
@tracer.wrap("aws.lambda")
3342
def wrapped(*args):

tests/contrib/aws_lambda/test_aws_lambda.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from ddtrace.contrib.aws_lambda import patch
66
from ddtrace.contrib.aws_lambda import unpatch
77
from tests.contrib.aws_lambda.handlers import datadog
8+
from tests.contrib.aws_lambda.handlers import finishing_spans_early_handler
89
from tests.contrib.aws_lambda.handlers import handler
910
from tests.contrib.aws_lambda.handlers import manually_wrapped_handler
1011
from tests.contrib.aws_lambda.handlers import timeout_handler
@@ -55,6 +56,24 @@ def test_timeout_traces(context):
5556
assert e.type is Exception
5657

5758

59+
@pytest.mark.snapshot()
60+
def test_continue_on_early_trace_ending(context):
61+
"""
62+
These scenario expects no timeout error being tagged on the root span
63+
when closing all spans in the customers code and reaching a timeout.
64+
"""
65+
os.environ.update(
66+
{
67+
"AWS_LAMBDA_FUNCTION_NAME": "finishing_spans_early_handler",
68+
"DD_LAMBDA_HANDLER": "tests.contrib.aws_lambda.handlers.finishing_spans_early_handler",
69+
}
70+
)
71+
72+
patch()
73+
74+
datadog(finishing_spans_early_handler)({}, context())
75+
76+
5877
@pytest.mark.snapshot
5978
async def test_file_patching(context):
6079
os.environ.update(
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
[[
2+
{
3+
"name": "aws.lambda",
4+
"service": "",
5+
"resource": "aws.lambda",
6+
"trace_id": 0,
7+
"span_id": 1,
8+
"parent_id": 0,
9+
"type": "",
10+
"error": 0,
11+
"meta": {
12+
"_dd.p.dm": "-0",
13+
"language": "python",
14+
"runtime-id": "2a38d844990949379810ed84bb6e71d7"
15+
},
16+
"metrics": {
17+
"_dd.agent_psr": 1.0,
18+
"_dd.top_level": 1,
19+
"_dd.tracer_kr": 1.0,
20+
"_sampling_priority_v1": 1,
21+
"process_id": 55112
22+
},
23+
"duration": 6433000,
24+
"start": 1677616801872185000
25+
}]]

0 commit comments

Comments
 (0)