From de5aba059c2c87d83e10622144481bb80f8fe14d Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Tue, 21 Jun 2022 17:19:02 -0400 Subject: [PATCH 01/12] Send errors metrics for 5xx reponse from API Gateway, Lambda Function URL, or ALB --- datadog_lambda/wrapper.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index cf2efaa9..b178837b 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -110,6 +110,7 @@ def __init__(self, func): os.environ.get("DD_TRACE_MANAGED_SERVICES", "true").lower() == "true" ) self.response = None + self.already_submitted_errors_metric_before = False if self.extractor_env: extractor_parts = self.extractor_env.rsplit(".", 1) @@ -143,6 +144,7 @@ def __call__(self, event, context, **kwargs): return self.response except Exception: submit_errors_metric(context) + self.already_submitted_errors_metric_before = True if self.span: self.span.set_traceback() raise @@ -190,6 +192,9 @@ def _after(self, event, context): status_code = extract_http_status_code_tag(self.trigger_tags, self.response) if status_code: self.trigger_tags["http.status_code"] = status_code + if not self.already_submitted_errors_metric_before: + if len(status_code) == 3 and status_code.startswith("5"): + submit_errors_metric(context) # Create a new dummy Datadog subsegment for function trigger tags so we # can attach them to X-Ray spans when hybrid tracing is used if self.trigger_tags: From ad09cc90874f0d911637a939cd295968ff308ea8 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Tue, 21 Jun 2022 20:16:11 -0400 Subject: [PATCH 02/12] remove self.already_submitted_errors_metric_before flag and set_traceback on span --- datadog_lambda/wrapper.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index b178837b..4033ebe6 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -110,7 +110,6 @@ def __init__(self, func): os.environ.get("DD_TRACE_MANAGED_SERVICES", "true").lower() == "true" ) self.response = None - self.already_submitted_errors_metric_before = False if self.extractor_env: extractor_parts = self.extractor_env.rsplit(".", 1) @@ -144,7 +143,6 @@ def __call__(self, event, context, **kwargs): return self.response except Exception: submit_errors_metric(context) - self.already_submitted_errors_metric_before = True if self.span: self.span.set_traceback() raise @@ -192,9 +190,10 @@ def _after(self, event, context): status_code = extract_http_status_code_tag(self.trigger_tags, self.response) if status_code: self.trigger_tags["http.status_code"] = status_code - if not self.already_submitted_errors_metric_before: - if len(status_code) == 3 and status_code.startswith("5"): - submit_errors_metric(context) + if len(status_code) == 3 and status_code.startswith("5"): + submit_errors_metric(context) + if self.span: + self.span.set_traceback() # Create a new dummy Datadog subsegment for function trigger tags so we # can attach them to X-Ray spans when hybrid tracing is used if self.trigger_tags: From f5e80a8e747a313bae94844cb947fb6cad63cba9 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 14:24:33 -0400 Subject: [PATCH 03/12] set tags and mark span.error=1 --- datadog_lambda/constants.py | 10 +++++++ datadog_lambda/wrapper.py | 11 ++++++- tests/test_wrapper.py | 58 +++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/datadog_lambda/constants.py b/datadog_lambda/constants.py index 9bd49ce3..2ade32a3 100644 --- a/datadog_lambda/constants.py +++ b/datadog_lambda/constants.py @@ -42,3 +42,13 @@ class XrayDaemon(object): XRAY_TRACE_ID_HEADER_NAME = "_X_AMZN_TRACE_ID" XRAY_DAEMON_ADDRESS = "AWS_XRAY_DAEMON_ADDRESS" FUNCTION_NAME_HEADER_NAME = "AWS_LAMBDA_FUNCTION_NAME" + + +SERVER_ERRORS_STATUS_CODES = { + "500": "500 Internal Server Error", + "501": "501 Not Implemented", + "502": "502 Bad Gateway", + "503": "503 Service Unavailable", + "504": "504 Gateway Timeout", + "505": "505 HTTP Version Not Supported", +} \ No newline at end of file diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index 4033ebe6..f52df12c 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -8,11 +8,14 @@ import traceback from importlib import import_module +from ddtrace.constants import ERROR_MSG, ERROR_STACK, ERROR_TYPE + from datadog_lambda.extension import should_use_extension, flush_extension from datadog_lambda.cold_start import set_cold_start, is_cold_start from datadog_lambda.constants import ( - XraySubsegment, + SERVER_ERRORS_STATUS_CODES, TraceContextSource, + XraySubsegment, ) from datadog_lambda.metric import ( flush_stats, @@ -194,6 +197,12 @@ def _after(self, event, context): submit_errors_metric(context) if self.span: self.span.set_traceback() + self.span.error = 1 + self.span.set_tags({ + ERROR_TYPE: "5xx Server Errors", + ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get(status_code, + "5xx Server Errors"), + }) # Create a new dummy Datadog subsegment for function trigger tags so we # can attach them to X-Ray spans when hybrid tracing is used if self.trigger_tags: diff --git a/tests/test_wrapper.py b/tests/test_wrapper.py index 32540553..4ac6558c 100644 --- a/tests/test_wrapper.py +++ b/tests/test_wrapper.py @@ -275,6 +275,64 @@ def lambda_handler(event, context): ] ) + @patch('datadog_lambda.wrapper.extract_trigger_tags') + def test_5xx_sends_errors_metric_and_set_tags(self, mock_extract_trigger_tags): + mock_extract_trigger_tags.return_value = { + "function_trigger.event_source": "api-gateway", + "function_trigger.event_source_arn": + "arn:aws:apigateway:us-west-1::/restapis/1234567890/stages/prod", + "http.url": "70ixmpl4fl.execute-api.us-east-2.amazonaws.com", + "http.url_details.path": "/prod/path/to/resource", + "http.method": "GET", + } + @datadog_lambda_wrapper + def lambda_handler(event, context): + return { + "statusCode": 500, + "body": "fake response body" + } + + lambda_event = {} + + lambda_handler(lambda_event, get_mock_context()) + + self.mock_write_metric_point_to_stdout.assert_has_calls( + [ + call( + "aws.lambda.enhanced.invocations", + 1, + tags=[ + "region:us-west-1", + "account_id:123457598159", + "functionname:python-layer-test", + "resource:python-layer-test:1", + "cold_start:true", + "memorysize:256", + "runtime:python3.9", + "datadog_lambda:v6.6.6", + "dd_lambda_layer:datadog-python39_X.X.X", + ], + timestamp=None, + ), + call( + "aws.lambda.enhanced.errors", + 1, + tags=[ + "region:us-west-1", + "account_id:123457598159", + "functionname:python-layer-test", + "resource:python-layer-test:1", + "cold_start:true", + "memorysize:256", + "runtime:python3.9", + "datadog_lambda:v6.6.6", + "dd_lambda_layer:datadog-python39_X.X.X", + ], + timestamp=None, + ), + ] + ) + def test_enhanced_metrics_cold_start_tag(self): @datadog_lambda_wrapper def lambda_handler(event, context): From 29f41175de98c96fbd9d4690c0314e9754b171e8 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 15:21:33 -0400 Subject: [PATCH 04/12] reset self.response and lint --- datadog_lambda/constants.py | 2 +- datadog_lambda/wrapper.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/datadog_lambda/constants.py b/datadog_lambda/constants.py index 2ade32a3..113293bf 100644 --- a/datadog_lambda/constants.py +++ b/datadog_lambda/constants.py @@ -51,4 +51,4 @@ class XrayDaemon(object): "503": "503 Service Unavailable", "504": "504 Gateway Timeout", "505": "505 HTTP Version Not Supported", -} \ No newline at end of file +} diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index f52df12c..7e3565e1 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -153,6 +153,7 @@ def __call__(self, event, context, **kwargs): self._after(event, context) def _before(self, event, context): + self.response = None try: set_cold_start() @@ -198,11 +199,14 @@ def _after(self, event, context): if self.span: self.span.set_traceback() self.span.error = 1 - self.span.set_tags({ - ERROR_TYPE: "5xx Server Errors", - ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get(status_code, - "5xx Server Errors"), - }) + self.span.set_tags( + { + ERROR_TYPE: "5xx Server Errors", + ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get( + status_code, + "5xx Server Errors"), + } + ) # Create a new dummy Datadog subsegment for function trigger tags so we # can attach them to X-Ray spans when hybrid tracing is used if self.trigger_tags: From 59b1f69343ae02248c0cfbdbb9218afaf7d253bc Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 15:23:29 -0400 Subject: [PATCH 05/12] tlint --- datadog_lambda/wrapper.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index 7e3565e1..a1e08088 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -203,8 +203,7 @@ def _after(self, event, context): { ERROR_TYPE: "5xx Server Errors", ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get( - status_code, - "5xx Server Errors"), + status_code, "5xx Server Errors"), } ) # Create a new dummy Datadog subsegment for function trigger tags so we From d60572b7464fbfd070cd31b9b3faa4feda90fe49 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 15:28:43 -0400 Subject: [PATCH 06/12] lint --- datadog_lambda/wrapper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index a1e08088..d1e3ab93 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -203,7 +203,8 @@ def _after(self, event, context): { ERROR_TYPE: "5xx Server Errors", ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get( - status_code, "5xx Server Errors"), + status_code, "5xx Server Errors" + ), } ) # Create a new dummy Datadog subsegment for function trigger tags so we From d02855ad1833e6a3157908c5d8c00d6afa6a9e36 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 16:09:00 -0400 Subject: [PATCH 07/12] lint and move self.response=None into try block --- datadog_lambda/wrapper.py | 3 +-- tests/test_wrapper.py | 11 ++++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index d1e3ab93..3f7a15d3 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -153,9 +153,8 @@ def __call__(self, event, context, **kwargs): self._after(event, context) def _before(self, event, context): - self.response = None try: - + self.response = None set_cold_start() submit_invocations_metric(context) self.trigger_tags = extract_trigger_tags(event, context) diff --git a/tests/test_wrapper.py b/tests/test_wrapper.py index 4ac6558c..a78cddfc 100644 --- a/tests/test_wrapper.py +++ b/tests/test_wrapper.py @@ -275,22 +275,19 @@ def lambda_handler(event, context): ] ) - @patch('datadog_lambda.wrapper.extract_trigger_tags') + @patch("datadog_lambda.wrapper.extract_trigger_tags") def test_5xx_sends_errors_metric_and_set_tags(self, mock_extract_trigger_tags): mock_extract_trigger_tags.return_value = { "function_trigger.event_source": "api-gateway", - "function_trigger.event_source_arn": - "arn:aws:apigateway:us-west-1::/restapis/1234567890/stages/prod", + "function_trigger.event_source_arn": "arn:aws:apigateway:us-west-1::/restapis/1234567890/stages/prod", "http.url": "70ixmpl4fl.execute-api.us-east-2.amazonaws.com", "http.url_details.path": "/prod/path/to/resource", "http.method": "GET", } + @datadog_lambda_wrapper def lambda_handler(event, context): - return { - "statusCode": 500, - "body": "fake response body" - } + return {"statusCode": 500, "body": "fake response body"} lambda_event = {} From a7cb6ef0cd61953d170c50cdbbb78d7a0958c615 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 16:26:58 -0400 Subject: [PATCH 08/12] lint_again --- datadog_lambda/wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index 3f7a15d3..1ce97d87 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -8,7 +8,7 @@ import traceback from importlib import import_module -from ddtrace.constants import ERROR_MSG, ERROR_STACK, ERROR_TYPE +from ddtrace.constants import ERROR_MSG, ERROR_TYPE from datadog_lambda.extension import should_use_extension, flush_extension from datadog_lambda.cold_start import set_cold_start, is_cold_start From f71679ca638010ab8d01f957782aec3868ac1a65 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 17:32:58 -0400 Subject: [PATCH 09/12] refactor into a function --- datadog_lambda/tracing.py | 20 ++++++++++++++++++++ datadog_lambda/wrapper.py | 19 +++---------------- tests/test_tracing.py | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/datadog_lambda/tracing.py b/datadog_lambda/tracing.py index b5324b1e..f39e89a3 100644 --- a/datadog_lambda/tracing.py +++ b/datadog_lambda/tracing.py @@ -10,6 +10,10 @@ from datetime import datetime, timezone from typing import Optional, Dict +from ddtrace.constants import ERROR_MSG, ERROR_TYPE + +from datadog_lambda.metric import submit_errors_metric + try: from typing import Literal except ImportError: @@ -17,6 +21,7 @@ from typing_extensions import Literal from datadog_lambda.constants import ( + SERVER_ERRORS_STATUS_CODES, SamplingPriority, TraceHeader, TraceContextSource, @@ -959,6 +964,21 @@ def create_function_execution_span( return span +def mark_trace_as_error_for_5xx_responses(context, status_code, span): + if len(status_code) == 3 and status_code.startswith("5"): + submit_errors_metric(context) + if span: + span.error = 1 + span.set_tags( + { + ERROR_TYPE: "5xx Server Errors", + ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get( + status_code, "5xx Server Errors" + ), + } + ) + + class InferredSpanInfo(object): BASE_NAME = "_inferred_span" SYNCHRONICITY = f"{BASE_NAME}.synchronicity" diff --git a/datadog_lambda/wrapper.py b/datadog_lambda/wrapper.py index 1ce97d87..0033e93c 100644 --- a/datadog_lambda/wrapper.py +++ b/datadog_lambda/wrapper.py @@ -8,12 +8,9 @@ import traceback from importlib import import_module -from ddtrace.constants import ERROR_MSG, ERROR_TYPE - from datadog_lambda.extension import should_use_extension, flush_extension from datadog_lambda.cold_start import set_cold_start, is_cold_start from datadog_lambda.constants import ( - SERVER_ERRORS_STATUS_CODES, TraceContextSource, XraySubsegment, ) @@ -29,6 +26,7 @@ create_dd_dummy_metadata_subsegment, inject_correlation_ids, dd_tracing_enabled, + mark_trace_as_error_for_5xx_responses, set_correlation_ids, set_dd_trace_py_root, create_function_execution_span, @@ -193,19 +191,8 @@ def _after(self, event, context): status_code = extract_http_status_code_tag(self.trigger_tags, self.response) if status_code: self.trigger_tags["http.status_code"] = status_code - if len(status_code) == 3 and status_code.startswith("5"): - submit_errors_metric(context) - if self.span: - self.span.set_traceback() - self.span.error = 1 - self.span.set_tags( - { - ERROR_TYPE: "5xx Server Errors", - ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get( - status_code, "5xx Server Errors" - ), - } - ) + mark_trace_as_error_for_5xx_responses(context, status_code, self.span) + # Create a new dummy Datadog subsegment for function trigger tags so we # can attach them to X-Ray spans when hybrid tracing is used if self.trigger_tags: diff --git a/tests/test_tracing.py b/tests/test_tracing.py index be52697e..fdb0adf5 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -2,12 +2,15 @@ import json import os -from unittest.mock import MagicMock, patch, call +from unittest.mock import MagicMock, Mock, patch, call +import ddtrace +from ddtrace.constants import ERROR_MSG, ERROR_TYPE from ddtrace.helpers import get_correlation_ids from ddtrace.context import Context from datadog_lambda.constants import ( + SERVER_ERRORS_STATUS_CODES, SamplingPriority, TraceHeader, XraySubsegment, @@ -18,6 +21,7 @@ create_dd_dummy_metadata_subsegment, create_function_execution_span, get_dd_trace_context, + mark_trace_as_error_for_5xx_responses, set_correlation_ids, set_dd_trace_py_root, _convert_xray_trace_id, @@ -1191,3 +1195,31 @@ def test_create_inferred_span_from_api_gateway_event_no_apiid(self): self.assertEqual(span.span_type, "http") self.assertEqual(span.get_tag(InferredSpanInfo.TAG_SOURCE), "self") self.assertEqual(span.get_tag(InferredSpanInfo.SYNCHRONICITY), "sync") + + @patch("datadog_lambda.tracing.submit_errors_metric") + def test_mark_trace_as_error_for_5xx_responses_getting_400_response_code( + self, mock_submit_errors_metric + ): + mark_trace_as_error_for_5xx_responses( + context="fake_context", status_code="400", span="empty_span" + ) + mock_submit_errors_metric.assert_not_called() + + @patch("datadog_lambda.tracing.submit_errors_metric") + def test_mark_trace_as_error_for_5xx_responses_sends_error_metric_and_set_error_tags( + self, mock_submit_errors_metric + ): + mock_span = Mock(ddtrace.span.Span) + status_code = "500" + mark_trace_as_error_for_5xx_responses( + context="fake_context", status_code=status_code, span=mock_span + ) + mock_submit_errors_metric.assert_called_once() + self.assertEqual(1, mock_span.error) + self.assertDictEqual( + { + ERROR_TYPE: "5xx Server Errors", + ERROR_MSG: "500 Internal Server Error", + }, + mock_span.set_tags.call_args[0][0], + ) From 9730be59942e662293cc2dfede934eefc244178f Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 20:11:28 -0400 Subject: [PATCH 10/12] remove set_tags logic --- datadog_lambda/tracing.py | 8 -------- tests/test_tracing.py | 7 ------- 2 files changed, 15 deletions(-) diff --git a/datadog_lambda/tracing.py b/datadog_lambda/tracing.py index f39e89a3..97cc9324 100644 --- a/datadog_lambda/tracing.py +++ b/datadog_lambda/tracing.py @@ -969,14 +969,6 @@ def mark_trace_as_error_for_5xx_responses(context, status_code, span): submit_errors_metric(context) if span: span.error = 1 - span.set_tags( - { - ERROR_TYPE: "5xx Server Errors", - ERROR_MSG: SERVER_ERRORS_STATUS_CODES.get( - status_code, "5xx Server Errors" - ), - } - ) class InferredSpanInfo(object): diff --git a/tests/test_tracing.py b/tests/test_tracing.py index fdb0adf5..dbef144e 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -1216,10 +1216,3 @@ def test_mark_trace_as_error_for_5xx_responses_sends_error_metric_and_set_error_ ) mock_submit_errors_metric.assert_called_once() self.assertEqual(1, mock_span.error) - self.assertDictEqual( - { - ERROR_TYPE: "5xx Server Errors", - ERROR_MSG: "500 Internal Server Error", - }, - mock_span.set_tags.call_args[0][0], - ) From 43fad9d6abc23fc39eccc9588bf3f46f9c5d0b94 Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Thu, 23 Jun 2022 20:20:41 -0400 Subject: [PATCH 11/12] lint --- datadog_lambda/tracing.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/datadog_lambda/tracing.py b/datadog_lambda/tracing.py index 97cc9324..2147a957 100644 --- a/datadog_lambda/tracing.py +++ b/datadog_lambda/tracing.py @@ -10,8 +10,6 @@ from datetime import datetime, timezone from typing import Optional, Dict -from ddtrace.constants import ERROR_MSG, ERROR_TYPE - from datadog_lambda.metric import submit_errors_metric try: @@ -21,7 +19,6 @@ from typing_extensions import Literal from datadog_lambda.constants import ( - SERVER_ERRORS_STATUS_CODES, SamplingPriority, TraceHeader, TraceContextSource, From abcf63c5cabb3b5f4a1170935a68fe1887f7df2c Mon Sep 17 00:00:00 2001 From: Kimi Wu Date: Fri, 24 Jun 2022 16:11:07 -0400 Subject: [PATCH 12/12] remove SERVER_ERRORS_STATUS_CODES --- datadog_lambda/constants.py | 10 ---------- tests/test_tracing.py | 1 - 2 files changed, 11 deletions(-) diff --git a/datadog_lambda/constants.py b/datadog_lambda/constants.py index 113293bf..9bd49ce3 100644 --- a/datadog_lambda/constants.py +++ b/datadog_lambda/constants.py @@ -42,13 +42,3 @@ class XrayDaemon(object): XRAY_TRACE_ID_HEADER_NAME = "_X_AMZN_TRACE_ID" XRAY_DAEMON_ADDRESS = "AWS_XRAY_DAEMON_ADDRESS" FUNCTION_NAME_HEADER_NAME = "AWS_LAMBDA_FUNCTION_NAME" - - -SERVER_ERRORS_STATUS_CODES = { - "500": "500 Internal Server Error", - "501": "501 Not Implemented", - "502": "502 Bad Gateway", - "503": "503 Service Unavailable", - "504": "504 Gateway Timeout", - "505": "505 HTTP Version Not Supported", -} diff --git a/tests/test_tracing.py b/tests/test_tracing.py index dbef144e..5fda2851 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -10,7 +10,6 @@ from ddtrace.context import Context from datadog_lambda.constants import ( - SERVER_ERRORS_STATUS_CODES, SamplingPriority, TraceHeader, XraySubsegment,