From 1b68c61da8eab9bf58a671d910026b411874bb35 Mon Sep 17 00:00:00 2001 From: Georgi Date: Wed, 29 Nov 2023 11:40:50 +0100 Subject: [PATCH 01/12] [CLOUDS-4012] Fix service tag setting in lambda forwarder If the service tag in set in dd_tags use it, otherwise use the one coming from the logGroup If none of the above, check if the tag is set on the lambda forwarder and use it, otherwise use the function name. Finally, check if the service tag is set in the application level and override --- aws/logs_monitoring/lambda_cache.py | 7 +++ aws/logs_monitoring/lambda_function.py | 60 +++++++++++++++++++++++--- aws/logs_monitoring/parsing.py | 23 ++++++---- 3 files changed, 75 insertions(+), 15 deletions(-) diff --git a/aws/logs_monitoring/lambda_cache.py b/aws/logs_monitoring/lambda_cache.py index 5bf6481b1..cd887d3d0 100644 --- a/aws/logs_monitoring/lambda_cache.py +++ b/aws/logs_monitoring/lambda_cache.py @@ -78,5 +78,12 @@ def get(self, key): logger.debug("Local cache expired, fetching cache from S3") self._refresh() + if not self.should_fetch_tags(): + logger.debug( + "Not fetching lambda function tags because the env variable DD_FETCH_LAMBDA_TAGS is " + "not set to true" + ) + return [] function_tags = self.tags_by_id.get(key, []) + return function_tags diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index ed4933398..fc74af3e9 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -257,14 +257,23 @@ def add_metadata_to_lambda_log(event): # Get custom tags of the Lambda function custom_lambda_tags = get_enriched_lambda_log_tags(event) + # If not set during parsing or set with a default value # Set the `service` tag and metadata field. If the Lambda function is # tagged with a `service` tag, use it, otherwise use the function name. - service_tag = next( - (tag for tag in custom_lambda_tags if tag.startswith("service:")), - f"service:{function_name}", - ) - tags.append(service_tag) - event[DD_SERVICE] = service_tag.split(":")[1] + # Otherwise, remove the `service` tag from the Lambda function if it exists + if not event[DD_SERVICE] or event[DD_SERVICE] == event[DD_SOURCE]: + service_tag = next( + (tag for tag in custom_lambda_tags if tag.startswith("service:")), + f"service:{function_name}", + ) + tags.append(service_tag) + event[DD_SERVICE] = service_tag.split(":")[1] + else: + # remove the service tag from the cusotm lambda tags if it exists + # as we don't want to add it again + custom_lambda_tags = [ + tag for tag in custom_lambda_tags if not tag.startswith("service:") + ] # Check if one of the Lambda's custom tags is env # If an env tag exists, remove the env:none placeholder @@ -319,7 +328,44 @@ def extract_ddtags_from_message(event): if logger.isEnabledFor(logging.DEBUG): logger.debug(f"Failed to extract ddtags from: {event}") return - event[DD_CUSTOM_TAGS] = f"{event[DD_CUSTOM_TAGS]},{extracted_ddtags}" + + event[DD_CUSTOM_TAGS] = _merge_custom_and_application_tags(event[DD_CUSTOM_TAGS], extracted_ddtags) + + # Extract service tag from message.ddtags if exists + if extracted_ddtags.contains("service:"): + event[DD_SERVICE] = next(tag[8:] for tag in extracted_ddtags.split(",") if tag.startswith("service:")) + + +def _merge_custom_and_application_tags(custom_tags, application_tags): + """Merge the custom tags added by the forwarder and the application. + + The custom tags added by the forwarder are added to the top-level `ddtags` + field, while the custom tags added by the application are added to the + `message.ddtags` field. + + Args: + custom_tags (str): the custom tags added by the forwarder + application_tags (str): the custom tags added by the application + + Returns: + str: the merged custom tags + """ + if not custom_tags: + return application_tags + + if not application_tags: + return custom_tags + + custom_tags_set = set(custom_tags.split(",")) + application_tags_set = set(application_tags.split(",")) + application_tags_keys = [tag.split(":")[0] for tag in application_tags_set] + + for application_tag_key in application_tags_keys: + custom_tags_set = set( + [tag for tag in custom_tags_set if not tag.startswith(f"{application_tag_key}:")] + ) + + return ",".join(list(custom_tags_set.union(application_tags_set))) def extract_host_from_cloudtrails(event): diff --git a/aws/logs_monitoring/parsing.py b/aws/logs_monitoring/parsing.py index bb61ba211..a6efc220d 100644 --- a/aws/logs_monitoring/parsing.py +++ b/aws/logs_monitoring/parsing.py @@ -186,7 +186,7 @@ def s3_handler(event, context, metadata): source = "transitgateway" metadata[DD_SOURCE] = source - metadata[DD_SERVICE] = get_service_from_tags(metadata) + metadata[DD_SERVICE] = get_service_from_tags_and_remove_duplicates(metadata) ##Get the ARN of the service and set it as the hostname hostname = parse_service_arn(source, key, bucket, context) @@ -242,15 +242,21 @@ def s3_handler(event, context, metadata): yield structured_line -def get_service_from_tags(metadata): - # Get service from dd_custom_tags if it exists +def get_service_from_tags_and_remove_duplicates(metadata): + service = "" tagsplit = metadata[DD_CUSTOM_TAGS].split(",") - for tag in tagsplit: + for i, tag in enumerate(tagsplit): if tag.startswith("service:"): - return tag[8:] + if service: + # remove duplicate entry from the tags + del tagsplit[i] + else: + service = tag[8:] + + metadata[DD_CUSTOM_TAGS] = ",".join(tagsplit) # Default service to source value - return metadata[DD_SOURCE] + return service if service else metadata[DD_SOURCE] def parse_event_source(event, key): @@ -530,7 +536,8 @@ def awslogs_handler(event, context, metadata): # Set service from custom tags, which may include the tags set on the log group # Returns DD_SOURCE by default - metadata[DD_SERVICE] = get_service_from_tags(metadata) + metadata[DD_SERVICE] = get_service_from_tags_and_remove_duplicates(metadata) + # Set host as log group where cloudwatch is source if metadata[DD_SOURCE] == "cloudwatch" or metadata.get(DD_HOST, None) == None: @@ -640,7 +647,7 @@ def cwevent_handler(event, metadata): else: metadata[DD_SOURCE] = "cloudwatch" - metadata[DD_SERVICE] = get_service_from_tags(metadata) + metadata[DD_SERVICE] = get_service_from_tags_and_remove_duplicates(metadata) yield data From 8529e903b4a2ab7fc02bdeb022317716bc982a1a Mon Sep 17 00:00:00 2001 From: Georgi Date: Wed, 29 Nov 2023 13:10:42 +0100 Subject: [PATCH 02/12] Fix Unit tests, Integration tests, python formatting --- aws/logs_monitoring/lambda_function.py | 25 +++++++++++++------ aws/logs_monitoring/parsing.py | 1 - aws/logs_monitoring/tests/test_parsing.py | 14 ++++++++--- .../cloudwatch_log_custom_tags.json~snapshot | 2 +- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index fc74af3e9..ae11ed708 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -258,7 +258,7 @@ def add_metadata_to_lambda_log(event): custom_lambda_tags = get_enriched_lambda_log_tags(event) # If not set during parsing or set with a default value - # Set the `service` tag and metadata field. If the Lambda function is + # Try to set the `service` tag and metadata field. If the Lambda function is # tagged with a `service` tag, use it, otherwise use the function name. # Otherwise, remove the `service` tag from the Lambda function if it exists if not event[DD_SERVICE] or event[DD_SERVICE] == event[DD_SOURCE]: @@ -266,8 +266,9 @@ def add_metadata_to_lambda_log(event): (tag for tag in custom_lambda_tags if tag.startswith("service:")), f"service:{function_name}", ) - tags.append(service_tag) - event[DD_SERVICE] = service_tag.split(":")[1] + if service_tag: + tags.append(service_tag) + event[DD_SERVICE] = service_tag.split(":")[1] else: # remove the service tag from the cusotm lambda tags if it exists # as we don't want to add it again @@ -329,11 +330,17 @@ def extract_ddtags_from_message(event): logger.debug(f"Failed to extract ddtags from: {event}") return - event[DD_CUSTOM_TAGS] = _merge_custom_and_application_tags(event[DD_CUSTOM_TAGS], extracted_ddtags) + event[DD_CUSTOM_TAGS] = _merge_custom_and_application_tags( + event[DD_CUSTOM_TAGS], extracted_ddtags + ) - # Extract service tag from message.ddtags if exists + ## Extract service tag from message.ddtags if exists if extracted_ddtags.contains("service:"): - event[DD_SERVICE] = next(tag[8:] for tag in extracted_ddtags.split(",") if tag.startswith("service:")) + event[DD_SERVICE] = next( + tag[8:] + for tag in extracted_ddtags.split(",") + if tag.startswith("service:") + ) def _merge_custom_and_application_tags(custom_tags, application_tags): @@ -362,7 +369,11 @@ def _merge_custom_and_application_tags(custom_tags, application_tags): for application_tag_key in application_tags_keys: custom_tags_set = set( - [tag for tag in custom_tags_set if not tag.startswith(f"{application_tag_key}:")] + [ + tag + for tag in custom_tags_set + if not tag.startswith(f"{application_tag_key}:") + ] ) return ",".join(list(custom_tags_set.union(application_tags_set))) diff --git a/aws/logs_monitoring/parsing.py b/aws/logs_monitoring/parsing.py index a6efc220d..b1ba7f23b 100644 --- a/aws/logs_monitoring/parsing.py +++ b/aws/logs_monitoring/parsing.py @@ -538,7 +538,6 @@ def awslogs_handler(event, context, metadata): # Returns DD_SOURCE by default metadata[DD_SERVICE] = get_service_from_tags_and_remove_duplicates(metadata) - # Set host as log group where cloudwatch is source if metadata[DD_SOURCE] == "cloudwatch" or metadata.get(DD_HOST, None) == None: metadata[DD_HOST] = aws_attributes["aws"]["awslogs"]["logGroup"] diff --git a/aws/logs_monitoring/tests/test_parsing.py b/aws/logs_monitoring/tests/test_parsing.py index 84f85d159..ce7f820d3 100644 --- a/aws/logs_monitoring/tests/test_parsing.py +++ b/aws/logs_monitoring/tests/test_parsing.py @@ -28,7 +28,7 @@ parse_service_arn, separate_security_hub_findings, parse_aws_waf_logs, - get_service_from_tags, + get_service_from_tags_and_remove_duplicates, get_state_machine_arn, get_lower_cased_lambda_function_name, ) @@ -734,14 +734,22 @@ def test_get_service_from_tags(self): DD_SOURCE: "ecs", DD_CUSTOM_TAGS: "env:dev,tag,stack:aws:ecs,service:web,version:v1", } - self.assertEqual(get_service_from_tags(metadata), "web") + self.assertEqual(get_service_from_tags_and_remove_duplicates(metadata), "web") def test_get_service_from_tags_default_to_source(self): metadata = { DD_SOURCE: "ecs", DD_CUSTOM_TAGS: "env:dev,tag,stack:aws:ecs,version:v1", } - self.assertEqual(get_service_from_tags(metadata), "ecs") + self.assertEqual(get_service_from_tags_and_remove_duplicates(metadata), "ecs") + + def test_get_service_from_tags_removing_duplicates(self): + metadata = { + DD_SOURCE: "ecs", + DD_CUSTOM_TAGS: "env:dev,tag,stack:aws:ecs,service:web,version:v1,service:other", + } + self.assertEqual(get_service_from_tags_and_remove_duplicates(metadata), "web") + self.assertEqual(metadata[DD_CUSTOM_TAGS], "env:dev,tag,stack:aws:ecs,service:web,version:v1") class TestParsingStepFunctionLogs(unittest.TestCase): diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot index 674812320..d12ff77e3 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwardername:test,forwarder_memorysize:1536,forwarder_version:,custom_tag1:value1,custom_tag2:value2", + "ddtags": "forwardername:test,custom_tag2:value2,custom_tag1:value1,forwarder_memorysize:1536,forwarder_version:", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", From 4a6c5aeb70d956aaf5de177d0e84f5a068d67162 Mon Sep 17 00:00:00 2001 From: Georgi Date: Wed, 29 Nov 2023 15:40:04 +0100 Subject: [PATCH 03/12] Fix Formatting on test_parsing.py --- aws/logs_monitoring/tests/test_parsing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aws/logs_monitoring/tests/test_parsing.py b/aws/logs_monitoring/tests/test_parsing.py index ce7f820d3..1ff5c7a7c 100644 --- a/aws/logs_monitoring/tests/test_parsing.py +++ b/aws/logs_monitoring/tests/test_parsing.py @@ -749,7 +749,9 @@ def test_get_service_from_tags_removing_duplicates(self): DD_CUSTOM_TAGS: "env:dev,tag,stack:aws:ecs,service:web,version:v1,service:other", } self.assertEqual(get_service_from_tags_and_remove_duplicates(metadata), "web") - self.assertEqual(metadata[DD_CUSTOM_TAGS], "env:dev,tag,stack:aws:ecs,service:web,version:v1") + self.assertEqual( + metadata[DD_CUSTOM_TAGS], "env:dev,tag,stack:aws:ecs,service:web,version:v1" + ) class TestParsingStepFunctionLogs(unittest.TestCase): From 1051fb9f9f065b99a0f48805d3d17931f384c68f Mon Sep 17 00:00:00 2001 From: Georgi Date: Thu, 30 Nov 2023 11:38:03 +0100 Subject: [PATCH 04/12] Fix a bug in setting the service tag from the message ALso: - Update Itests - Add __init__.py to create a package for running utests inside the IDE --- aws/logs_monitoring/lambda_function.py | 2 +- .../cloudwatch_log_custom_tags.json~snapshot | 2 +- .../snapshots/cloudwatch_log_service_tag.json | 14 +++ .../cloudwatch_log_service_tag.json~snapshot | 107 ++++++++++++++++++ .../tester/test_snapshots.py | 5 + 5 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json create mode 100644 aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index ae11ed708..9c16874e5 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -335,7 +335,7 @@ def extract_ddtags_from_message(event): ) ## Extract service tag from message.ddtags if exists - if extracted_ddtags.contains("service:"): + if "service:" in extracted_ddtags: event[DD_SERVICE] = next( tag[8:] for tag in extracted_ddtags.split(",") diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot index d12ff77e3..6cd369faf 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwardername:test,custom_tag2:value2,custom_tag1:value1,forwarder_memorysize:1536,forwarder_version:", + "ddtags": "forwarder_memorysize:1536,forwardername:test,custom_tag2:value2,forwarder_version:,custom_tag1:value1", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json new file mode 100644 index 000000000..7c499d54e --- /dev/null +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json @@ -0,0 +1,14 @@ +{ + "messageType": "DATA_MESSAGE", + "owner": "123456789123", + "logGroup": "testLogGroup", + "logStream": "testLogStream", + "subscriptionFilters": ["testFilter"], + "logEvents": [ + { + "id": "eventId1", + "timestamp": 1440442987000, + "message": "{\"message\": \"hello world\", \"ddtags\": \"service:myservice\"}\n" + } + ] +} diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot new file mode 100644 index 000000000..957018006 --- /dev/null +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot @@ -0,0 +1,107 @@ +{ + "events": [ + { + "data": [ + { + "aws": { + "awslogs": { + "logGroup": "testLogGroup", + "logStream": "testLogStream", + "owner": "123456789123" + }, + "function_version": "$LATEST", + "invoked_function_arn": "arn:aws:lambda:us-east-1:0000000000:function:test" + }, + "ddsource": "cloudwatch", + "ddsourcecategory": "aws", + "ddtags": "forwarder_version:,service:myservice,forwarder_memorysize:1536,forwardername:test", + "host": "testLogGroup", + "id": "eventId1", + "message": "{\"message\": \"hello world\"}", + "service": "myservice", + "timestamp": 1440442987000 + } + ], + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Connection": "keep-alive", + "Content-Length": "", + "Content-type": "application/json", + "DD-API-KEY": "abcdefghijklmnopqrstuvwxyz012345", + "DD-EVP-ORIGIN": "aws_forwarder", + "DD-EVP-ORIGIN-VERSION": "", + "Host": "recorder:8080", + "User-Agent": "", + "x-datadog-parent-id": "", + "x-datadog-sampling-priority": "2", + "x-datadog-trace-id": "4842834437835386637" + }, + "path": "/api/v2/logs", + "verb": "POST" + }, + { + "data": { + "series": [ + { + "device": null, + "host": null, + "interval": 10, + "metric": "aws.dd_forwarder.incoming_events", + "points": "", + "tags": [ + "forwardername:test", + "forwarder_memorysize:1536", + "forwarder_version:", + "event_type:awslogs" + ], + "type": "distribution" + }, + { + "device": null, + "host": null, + "interval": 10, + "metric": "aws.dd_forwarder.logs_forwarded", + "points": "", + "tags": [ + "forwardername:test", + "forwarder_memorysize:1536", + "forwarder_version:", + "event_type:awslogs" + ], + "type": "distribution" + }, + { + "device": null, + "host": null, + "interval": 10, + "metric": "aws.dd_forwarder.metrics_forwarded", + "points": "", + "tags": [ + "forwardername:test", + "forwarder_memorysize:1536", + "forwarder_version:", + "event_type:awslogs" + ], + "type": "distribution" + } + ] + }, + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Connection": "keep-alive", + "Content-Encoding": "deflate", + "Content-Length": "", + "Content-Type": "application/json", + "Host": "recorder:8080", + "User-Agent": "", + "x-datadog-parent-id": "", + "x-datadog-sampling-priority": "2", + "x-datadog-trace-id": "4842834437835386637" + }, + "path": "/api/v1/distribution_points?api_key=abcdefghijklmnopqrstuvwxyz012345", + "verb": "POST" + } + ] +} \ No newline at end of file diff --git a/aws/logs_monitoring/tools/integration_tests/tester/test_snapshots.py b/aws/logs_monitoring/tools/integration_tests/tester/test_snapshots.py index bef4c6f84..9ab651a1b 100644 --- a/aws/logs_monitoring/tools/integration_tests/tester/test_snapshots.py +++ b/aws/logs_monitoring/tools/integration_tests/tester/test_snapshots.py @@ -182,3 +182,8 @@ def test_customized_log_group_lambda_log(self): ) snapshot_filename = f"{input_filename}~snapshot" self.compare_snapshot(input_filename, snapshot_filename) + + def test_cloudwatch_log_service_tag(self): + input_filename = f"{snapshot_dir}/cloudwatch_log_service_tag.json" + snapshot_filename = f"{input_filename}~snapshot" + self.compare_snapshot(input_filename, snapshot_filename) From 1fc0661a95fb65dce5750156532df3aa672281bf Mon Sep 17 00:00:00 2001 From: Georgi Date: Thu, 30 Nov 2023 15:09:02 +0100 Subject: [PATCH 05/12] Add UTests --- .../tests/test_lambda_function.py | 113 +++++++++++++++--- 1 file changed, 98 insertions(+), 15 deletions(-) diff --git a/aws/logs_monitoring/tests/test_lambda_function.py b/aws/logs_monitoring/tests/test_lambda_function.py index 086df3299..502b89108 100644 --- a/aws/logs_monitoring/tests/test_lambda_function.py +++ b/aws/logs_monitoring/tests/test_lambda_function.py @@ -8,6 +8,7 @@ from time import time from botocore.exceptions import ClientError from approvaltests.approvals import verify_as_json +from importlib import reload sys.modules["trace_forwarder.connection"] = MagicMock() sys.modules["datadog_lambda.wrapper"] = MagicMock() @@ -130,12 +131,8 @@ def create_cloudwatch_log_event_from_data(data): class TestLambdaFunctionEndToEnd(unittest.TestCase): - @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") - @patch("base_tags_cache.send_forwarder_internal_metrics") @patch("enhanced_lambda_metrics.LambdaTagsCache.get_cache_from_s3") - def test_datadog_forwarder( - self, mock_get_s3_cache, mock_forward_metrics, cw_logs_tags_get - ): + def test_datadog_forwarder(self, mock_get_s3_cache): mock_get_s3_cache.return_value = ( { "arn:aws:lambda:sa-east-1:601427279990:function:inferred-spans-python-dev-initsender": [ @@ -149,15 +146,7 @@ def test_datadog_forwarder( time(), ) context = Context() - my_path = os.path.abspath(os.path.dirname(__file__)) - path = os.path.join(my_path, "events/cloudwatch_logs.json") - - with open( - path, - "r", - ) as input_file: - input_data = input_file.read() - + input_data = self._get_input_data() event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} os.environ["DD_FETCH_LAMBDA_TAGS"] = "True" @@ -170,7 +159,7 @@ def test_datadog_forwarder( verify_as_json(transformed_events) - metrics, logs, trace_payloads = split(transformed_events) + _, _, trace_payloads = split(transformed_events) self.assertEqual(len(trace_payloads), 1) trace_payload = json.loads(trace_payloads[0]["message"]) @@ -204,6 +193,100 @@ def test_datadog_forwarder( del os.environ["DD_FETCH_LAMBDA_TAGS"] + @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") + def test_setting_service_tag_from_log_group_cache(self, cw_logs_tags_get): + reload(sys.modules["settings"]) + reload(sys.modules["parsing"]) + cw_logs_tags_get.return_value = ["service:log_group_service"] + context = Context() + input_data = self._get_input_data() + event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} + + normalized_events = parse(event, context) + enriched_events = enrich(normalized_events) + transformed_events = transform(enriched_events) + + _, logs, _ = split(transformed_events) + self.assertEqual(len(logs), 16) + for log in logs: + self.assertEqual(log["service"], "log_group_service") + + @patch("lambda_cache.LambdaTagsCache.get") + @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") + def test_overrding_service_tag_from_lambda_cache( + self, lambda_tags_get, cw_logs_tags_get + ): + reload(sys.modules["settings"]) + reload(sys.modules["parsing"]) + lambda_tags_get.return_value = ["service:lambda_service"] + cw_logs_tags_get.return_value = ["service:log_group_service"] + + context = Context() + input_data = self._get_input_data() + event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} + + normalized_events = parse(event, context) + enriched_events = enrich(normalized_events) + transformed_events = transform(enriched_events) + + _, logs, _ = split(transformed_events) + self.assertEqual(len(logs), 16) + for log in logs: + self.assertEqual(log["service"], "lambda_service") + + @patch.dict(os.environ, {"DD_TAGS": "service:dd_tag_service"}, clear=True) + @patch("lambda_cache.LambdaTagsCache.get") + @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") + def test_overrding_service_tag_from_lambda_cache_when_dd_tags_is_set( + self, lambda_tags_get, cw_logs_tags_get + ): + lambda_tags_get.return_value = ["service:lambda_service"] + cw_logs_tags_get.return_value = ["service:log_group_service"] + + context = Context() + input_data = self._get_input_data() + event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} + + normalized_events = parse(event, context) + enriched_events = enrich(normalized_events) + transformed_events = transform(enriched_events) + + _, logs, _ = split(transformed_events) + self.assertEqual(len(logs), 16) + for log in logs: + self.assertEqual(log["service"], "lambda_service") + + @patch.dict(os.environ, {"DD_TAGS": "service:dd_tag_service"}, clear=True) + @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") + def test_service_override_from_dd_tags(self, cw_logs_tags_get): + reload(sys.modules["settings"]) + reload(sys.modules["parsing"]) + cw_logs_tags_get.return_value = ["service:log_group_service"] + context = Context() + input_data = self._get_input_data() + event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} + + normalized_events = parse(event, context) + enriched_events = enrich(normalized_events) + transformed_events = transform(enriched_events) + + _, logs, _ = split(transformed_events) + self.assertEqual(len(logs), 16) + for log in logs: + self.assertEqual(log["service"], "dd_tag_service") + + def _get_input_data(self): + my_path = os.path.abspath(os.path.dirname(__file__)) + path = os.path.join(my_path, "events/cloudwatch_logs.json") + + with open( + path, + "r", + ) as input_file: + input_data = input_file.read() + + return input_data + class TestLambdaFunctionExtractTracePayload(unittest.TestCase): def test_extract_trace_payload_none_no_trace(self): From 22d3c2d259145368ef08fbf7ba7b1c7110ce3db1 Mon Sep 17 00:00:00 2001 From: Georgi Date: Thu, 30 Nov 2023 15:39:35 +0100 Subject: [PATCH 06/12] Update integrtion test files to match the new order of dd_tags keys --- aws/logs_monitoring/.gitignore | 2 +- .../snapshots/cloudwatch_log_custom_tags.json~snapshot | 2 +- .../snapshots/cloudwatch_log_service_tag.json~snapshot | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/aws/logs_monitoring/.gitignore b/aws/logs_monitoring/.gitignore index 7b1ef62a0..7ace5bde3 100644 --- a/aws/logs_monitoring/.gitignore +++ b/aws/logs_monitoring/.gitignore @@ -1,3 +1,3 @@ *.zip tools/layers -.forwarder \ No newline at end of file +.forwarder diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot index 6cd369faf..1d32b9abf 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwarder_memorysize:1536,forwardername:test,custom_tag2:value2,forwarder_version:,custom_tag1:value1", + "ddtags": "forwardername:test,custom_tag2:value2,forwarder_memorysize:1536,custom_tag1:value1,forwarder_version:", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot index 957018006..ba1ae0f80 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwarder_version:,service:myservice,forwarder_memorysize:1536,forwardername:test", + "ddtags": "forwardername:test,service:myservice,forwarder_version:,forwarder_memorysize:1536", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", From 1d4e262f10bdebd34716c887d7a571d2ca1e961d Mon Sep 17 00:00:00 2001 From: Georgi Date: Thu, 30 Nov 2023 16:46:52 +0100 Subject: [PATCH 07/12] Make tags output order consistent when extracting message tags --- aws/logs_monitoring/lambda_function.py | 6 ++-- .../tests/test_lambda_function.py | 33 +++++++++++-------- .../cloudwatch_log_custom_tags.json~snapshot | 2 +- .../cloudwatch_log_service_tag.json~snapshot | 2 +- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index 9c16874e5..e975cfee2 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -330,7 +330,7 @@ def extract_ddtags_from_message(event): logger.debug(f"Failed to extract ddtags from: {event}") return - event[DD_CUSTOM_TAGS] = _merge_custom_and_application_tags( + event[DD_CUSTOM_TAGS] = merge_custom_and_application_tags( event[DD_CUSTOM_TAGS], extracted_ddtags ) @@ -343,7 +343,7 @@ def extract_ddtags_from_message(event): ) -def _merge_custom_and_application_tags(custom_tags, application_tags): +def merge_custom_and_application_tags(custom_tags, application_tags): """Merge the custom tags added by the forwarder and the application. The custom tags added by the forwarder are added to the top-level `ddtags` @@ -376,7 +376,7 @@ def _merge_custom_and_application_tags(custom_tags, application_tags): ] ) - return ",".join(list(custom_tags_set.union(application_tags_set))) + return ",".join(sorted(list(custom_tags_set.union(application_tags_set)))) def extract_host_from_cloudtrails(event): diff --git a/aws/logs_monitoring/tests/test_lambda_function.py b/aws/logs_monitoring/tests/test_lambda_function.py index 502b89108..23fa03293 100644 --- a/aws/logs_monitoring/tests/test_lambda_function.py +++ b/aws/logs_monitoring/tests/test_lambda_function.py @@ -35,6 +35,7 @@ enrich, transform, split, + merge_custom_and_application_tags, ) from parsing import parse, parse_event_type @@ -211,16 +212,12 @@ def test_setting_service_tag_from_log_group_cache(self, cw_logs_tags_get): for log in logs: self.assertEqual(log["service"], "log_group_service") - @patch("lambda_cache.LambdaTagsCache.get") + @patch.dict(os.environ, {"DD_TAGS": "service:dd_tag_service"}, clear=True) @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") - def test_overrding_service_tag_from_lambda_cache( - self, lambda_tags_get, cw_logs_tags_get - ): + def test_service_override_from_dd_tags(self, cw_logs_tags_get): reload(sys.modules["settings"]) reload(sys.modules["parsing"]) - lambda_tags_get.return_value = ["service:lambda_service"] cw_logs_tags_get.return_value = ["service:log_group_service"] - context = Context() input_data = self._get_input_data() event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} @@ -232,12 +229,11 @@ def test_overrding_service_tag_from_lambda_cache( _, logs, _ = split(transformed_events) self.assertEqual(len(logs), 16) for log in logs: - self.assertEqual(log["service"], "lambda_service") + self.assertEqual(log["service"], "dd_tag_service") - @patch.dict(os.environ, {"DD_TAGS": "service:dd_tag_service"}, clear=True) @patch("lambda_cache.LambdaTagsCache.get") @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") - def test_overrding_service_tag_from_lambda_cache_when_dd_tags_is_set( + def test_overrding_service_tag_from_lambda_cache( self, lambda_tags_get, cw_logs_tags_get ): lambda_tags_get.return_value = ["service:lambda_service"] @@ -257,11 +253,14 @@ def test_overrding_service_tag_from_lambda_cache_when_dd_tags_is_set( self.assertEqual(log["service"], "lambda_service") @patch.dict(os.environ, {"DD_TAGS": "service:dd_tag_service"}, clear=True) + @patch("lambda_cache.LambdaTagsCache.get") @patch("cloudwatch_log_group_cache.CloudwatchLogGroupTagsCache.get") - def test_service_override_from_dd_tags(self, cw_logs_tags_get): - reload(sys.modules["settings"]) - reload(sys.modules["parsing"]) + def test_overrding_service_tag_from_lambda_cache_when_dd_tags_is_set( + self, lambda_tags_get, cw_logs_tags_get + ): + lambda_tags_get.return_value = ["service:lambda_service"] cw_logs_tags_get.return_value = ["service:log_group_service"] + context = Context() input_data = self._get_input_data() event = {"awslogs": {"data": create_cloudwatch_log_event_from_data(input_data)}} @@ -273,7 +272,7 @@ def test_service_override_from_dd_tags(self, cw_logs_tags_get): _, logs, _ = split(transformed_events) self.assertEqual(len(logs), 16) for log in logs: - self.assertEqual(log["service"], "dd_tag_service") + self.assertEqual(log["service"], "lambda_service") def _get_input_data(self): my_path = os.path.abspath(os.path.dirname(__file__)) @@ -316,6 +315,14 @@ def test_extract_trace_payload_valid_trace(self): extract_trace_payload({"message": message_json, "ddtags": tags_json}), item ) +class TestMergeMessageTags(unittest.TestCase): + def test_merge_custom_and_application_tags(self): + message_tags = "key0:value0,key1:value1" + custom_tags = "key2:value2,key0:value3" + self.assertEqual( + merge_custom_and_application_tags(custom_tags, message_tags), + "key0:value0,key1:value1,key2:value2", + ) if __name__ == "__main__": unittest.main() diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot index 1d32b9abf..42f0de013 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwardername:test,custom_tag2:value2,forwarder_memorysize:1536,custom_tag1:value1,forwarder_version:", + "ddtags": "custom_tag1:value1,custom_tag2:value2,forwarder_memorysize:1536,forwarder_version:,forwardername:test", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot index ba1ae0f80..4a4bfee20 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwardername:test,service:myservice,forwarder_version:,forwarder_memorysize:1536", + "ddtags": "forwarder_memorysize:1536,forwarder_version:,forwardername:test,service:myservice", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", From 6d19f4efb915d74eec98e267153d1cacb11c9ea9 Mon Sep 17 00:00:00 2001 From: Georgi Date: Thu, 30 Nov 2023 16:55:48 +0100 Subject: [PATCH 08/12] Fix formatting --- aws/logs_monitoring/tests/test_lambda_function.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/aws/logs_monitoring/tests/test_lambda_function.py b/aws/logs_monitoring/tests/test_lambda_function.py index 23fa03293..13615ba5e 100644 --- a/aws/logs_monitoring/tests/test_lambda_function.py +++ b/aws/logs_monitoring/tests/test_lambda_function.py @@ -315,6 +315,7 @@ def test_extract_trace_payload_valid_trace(self): extract_trace_payload({"message": message_json, "ddtags": tags_json}), item ) + class TestMergeMessageTags(unittest.TestCase): def test_merge_custom_and_application_tags(self): message_tags = "key0:value0,key1:value1" @@ -324,5 +325,6 @@ def test_merge_custom_and_application_tags(self): "key0:value0,key1:value1,key2:value2", ) + if __name__ == "__main__": unittest.main() From e41f97dd89bbc03828eb9b8b1df6c24f41e54280 Mon Sep 17 00:00:00 2001 From: Georgi Date: Fri, 1 Dec 2023 13:55:56 +0100 Subject: [PATCH 09/12] Address some review comments / fix typos --- aws/logs_monitoring/lambda_cache.py | 13 ++++++------- aws/logs_monitoring/lambda_function.py | 17 +++++++---------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/aws/logs_monitoring/lambda_cache.py b/aws/logs_monitoring/lambda_cache.py index cd887d3d0..d0bbf7cc7 100644 --- a/aws/logs_monitoring/lambda_cache.py +++ b/aws/logs_monitoring/lambda_cache.py @@ -73,17 +73,16 @@ def get(self, key): Returns: lambda_tags (str[]): the list of "key:value" Datadog tag strings """ - if self._is_expired(): - send_forwarder_internal_metrics("local_cache_expired") - logger.debug("Local cache expired, fetching cache from S3") - self._refresh() - if not self.should_fetch_tags(): logger.debug( "Not fetching lambda function tags because the env variable DD_FETCH_LAMBDA_TAGS is " "not set to true" ) return [] - function_tags = self.tags_by_id.get(key, []) - return function_tags + if self._is_expired(): + send_forwarder_internal_metrics("local_cache_expired") + logger.debug("Local cache expired, fetching cache from S3") + self._refresh() + + return self.tags_by_id.get(key, []) diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index e975cfee2..5df51fcd2 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -257,10 +257,9 @@ def add_metadata_to_lambda_log(event): # Get custom tags of the Lambda function custom_lambda_tags = get_enriched_lambda_log_tags(event) - # If not set during parsing or set with a default value - # Try to set the `service` tag and metadata field. If the Lambda function is - # tagged with a `service` tag, use it, otherwise use the function name. - # Otherwise, remove the `service` tag from the Lambda function if it exists + # If not set during parsing or has a default value + # then set the service tag from lambda tags cache or using the function name + # otherwise, remove the service tag from the custom lambda tags if exists to avoid duplication if not event[DD_SERVICE] or event[DD_SERVICE] == event[DD_SOURCE]: service_tag = next( (tag for tag in custom_lambda_tags if tag.startswith("service:")), @@ -270,7 +269,7 @@ def add_metadata_to_lambda_log(event): tags.append(service_tag) event[DD_SERVICE] = service_tag.split(":")[1] else: - # remove the service tag from the cusotm lambda tags if it exists + # remove the service tag from the custom lambda tags if it exists # as we don't want to add it again custom_lambda_tags = [ tag for tag in custom_lambda_tags if not tag.startswith("service:") @@ -330,11 +329,9 @@ def extract_ddtags_from_message(event): logger.debug(f"Failed to extract ddtags from: {event}") return - event[DD_CUSTOM_TAGS] = merge_custom_and_application_tags( - event[DD_CUSTOM_TAGS], extracted_ddtags - ) + event[DD_CUSTOM_TAGS] = merge_tags(event[DD_CUSTOM_TAGS], extracted_ddtags) - ## Extract service tag from message.ddtags if exists + # Extract service tag from message.ddtags if exists if "service:" in extracted_ddtags: event[DD_SERVICE] = next( tag[8:] @@ -343,7 +340,7 @@ def extract_ddtags_from_message(event): ) -def merge_custom_and_application_tags(custom_tags, application_tags): +def merge_tags(custom_tags, application_tags): """Merge the custom tags added by the forwarder and the application. The custom tags added by the forwarder are added to the top-level `ddtags` From 4fa4e7ceb606bc1590060e02db991c3fcf96fde3 Mon Sep 17 00:00:00 2001 From: Georgi Date: Fri, 1 Dec 2023 14:26:25 +0100 Subject: [PATCH 10/12] Fix failing utest after function rename --- aws/logs_monitoring/tests/test_lambda_function.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/logs_monitoring/tests/test_lambda_function.py b/aws/logs_monitoring/tests/test_lambda_function.py index 13615ba5e..6235c3c8d 100644 --- a/aws/logs_monitoring/tests/test_lambda_function.py +++ b/aws/logs_monitoring/tests/test_lambda_function.py @@ -35,7 +35,7 @@ enrich, transform, split, - merge_custom_and_application_tags, + merge_tags, ) from parsing import parse, parse_event_type @@ -321,7 +321,7 @@ def test_merge_custom_and_application_tags(self): message_tags = "key0:value0,key1:value1" custom_tags = "key2:value2,key0:value3" self.assertEqual( - merge_custom_and_application_tags(custom_tags, message_tags), + merge_tags(custom_tags, message_tags), "key0:value0,key1:value1,key2:value2", ) From 379dd1ed6559a1eb554af5d7785efa08a797e61d Mon Sep 17 00:00:00 2001 From: Georgi Date: Mon, 4 Dec 2023 14:52:47 +0100 Subject: [PATCH 11/12] Change tag merging behavior between message ddtags and upper level ddtags Override the service tag if it exists. Merge upper level ddtags and message ddtags in one string. --- aws/logs_monitoring/lambda_function.py | 53 ++-------- .../tests/test_lambda_function.py | 98 +++++++++++++++++-- 2 files changed, 101 insertions(+), 50 deletions(-) diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index 5df51fcd2..c4b4a26fc 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -269,8 +269,6 @@ def add_metadata_to_lambda_log(event): tags.append(service_tag) event[DD_SERVICE] = service_tag.split(":")[1] else: - # remove the service tag from the custom lambda tags if it exists - # as we don't want to add it again custom_lambda_tags = [ tag for tag in custom_lambda_tags if not tag.startswith("service:") ] @@ -329,51 +327,18 @@ def extract_ddtags_from_message(event): logger.debug(f"Failed to extract ddtags from: {event}") return - event[DD_CUSTOM_TAGS] = merge_tags(event[DD_CUSTOM_TAGS], extracted_ddtags) - # Extract service tag from message.ddtags if exists - if "service:" in extracted_ddtags: - event[DD_SERVICE] = next( - tag[8:] - for tag in extracted_ddtags.split(",") - if tag.startswith("service:") + if "service" in extracted_ddtags: + event[DD_SERVICE] = next(tag[8:] for tag in extracted_ddtags.split(",") if tag.startswith("service:")) + event[DD_CUSTOM_TAGS] = ",".join( + [ + tag + for tag in event[DD_CUSTOM_TAGS].split(",") + if not tag.startswith("service") + ] ) - -def merge_tags(custom_tags, application_tags): - """Merge the custom tags added by the forwarder and the application. - - The custom tags added by the forwarder are added to the top-level `ddtags` - field, while the custom tags added by the application are added to the - `message.ddtags` field. - - Args: - custom_tags (str): the custom tags added by the forwarder - application_tags (str): the custom tags added by the application - - Returns: - str: the merged custom tags - """ - if not custom_tags: - return application_tags - - if not application_tags: - return custom_tags - - custom_tags_set = set(custom_tags.split(",")) - application_tags_set = set(application_tags.split(",")) - application_tags_keys = [tag.split(":")[0] for tag in application_tags_set] - - for application_tag_key in application_tags_keys: - custom_tags_set = set( - [ - tag - for tag in custom_tags_set - if not tag.startswith(f"{application_tag_key}:") - ] - ) - - return ",".join(sorted(list(custom_tags_set.union(application_tags_set)))) + event[DD_CUSTOM_TAGS] = f"{event[DD_CUSTOM_TAGS]},{extracted_ddtags}" def extract_host_from_cloudtrails(event): diff --git a/aws/logs_monitoring/tests/test_lambda_function.py b/aws/logs_monitoring/tests/test_lambda_function.py index 6235c3c8d..6a573398d 100644 --- a/aws/logs_monitoring/tests/test_lambda_function.py +++ b/aws/logs_monitoring/tests/test_lambda_function.py @@ -35,7 +35,7 @@ enrich, transform, split, - merge_tags, + extract_ddtags_from_message, ) from parsing import parse, parse_event_type @@ -317,12 +317,98 @@ def test_extract_trace_payload_valid_trace(self): class TestMergeMessageTags(unittest.TestCase): - def test_merge_custom_and_application_tags(self): - message_tags = "key0:value0,key1:value1" - custom_tags = "key2:value2,key0:value3" + message_tags = ( + '{"ddtags":"service:my_application_service,custom_tag_1:value1"}' + ) + custom_tags = "custom_tag_2:value2,service:my_custom_service" + + def test_extract_ddtags_from_message_str(self): + event = { + "message": self.message_tags, + "ddtags": self.custom_tags, + "service": "my_service", + } + + extract_ddtags_from_message(event) + + self.assertEqual( + event["ddtags"], + "custom_tag_2:value2,service:my_application_service,custom_tag_1:value1", + ) + self.assertEqual( + event["service"], + "my_application_service", + ) + + def test_extract_ddtags_from_message_dict(self): + loaded_message_tags = json.loads(self.message_tags) + event = { + "message": loaded_message_tags, + "ddtags": self.custom_tags, + "service": "my_service", + } + + extract_ddtags_from_message(event) + + self.assertEqual( + event["ddtags"], + "custom_tag_2:value2,service:my_application_service,custom_tag_1:value1", + ) + self.assertEqual( + event["service"], + "my_application_service", + ) + + def test_extract_ddtags_from_message_service_tag_setting(self): + loaded_message_tags = json.loads(self.message_tags) + loaded_message_tags["ddtags"] = ",".join([tag for tag in loaded_message_tags["ddtags"].split(",") if not tag.startswith("service:")]) + event = { + "message": loaded_message_tags, + "ddtags": self.custom_tags, + "service": "my_custom_service", + } + + extract_ddtags_from_message(event) + + self.assertEqual( + event["ddtags"], + "custom_tag_2:value2,service:my_custom_service,custom_tag_1:value1", + ) + self.assertEqual( + event["service"], + "my_custom_service", + ) + + def test_extract_ddtags_from_message_multiple_service_tag_values(self): + custom_tags = self.custom_tags + ",service:my_custom_service_2" + event = {"message": self.message_tags, "ddtags": custom_tags} + + extract_ddtags_from_message(event) + + self.assertEqual( + event["ddtags"], + "custom_tag_2:value2,service:my_application_service,custom_tag_1:value1", + ) + self.assertEqual( + event["service"], + "my_application_service", + ) + + def test_extract_ddtags_from_message_multiple_values_tag(self): + loaded_message_tags = json.loads(self.message_tags) + loaded_message_tags["ddtags"] += ",custom_tag_3:value4" + custom_tags = self.custom_tags + ",custom_tag_3:value3" + event = {"message": loaded_message_tags, "ddtags": custom_tags} + + extract_ddtags_from_message(event) + + self.assertEqual( + event["ddtags"], + "custom_tag_2:value2,custom_tag_3:value3,service:my_application_service,custom_tag_1:value1,custom_tag_3:value4", + ) self.assertEqual( - merge_tags(custom_tags, message_tags), - "key0:value0,key1:value1,key2:value2", + event["service"], + "my_application_service", ) From 187bd4b11721c7551b018d5ddd16741df74e1a10 Mon Sep 17 00:00:00 2001 From: Georgi Date: Mon, 4 Dec 2023 16:01:22 +0100 Subject: [PATCH 12/12] Fix Itests results order --- aws/logs_monitoring/lambda_function.py | 6 +++++- aws/logs_monitoring/tests/test_lambda_function.py | 12 ++++++++---- .../cloudwatch_log_custom_tags.json~snapshot | 2 +- .../cloudwatch_log_service_tag.json~snapshot | 2 +- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/aws/logs_monitoring/lambda_function.py b/aws/logs_monitoring/lambda_function.py index c4b4a26fc..86df8453e 100644 --- a/aws/logs_monitoring/lambda_function.py +++ b/aws/logs_monitoring/lambda_function.py @@ -329,7 +329,11 @@ def extract_ddtags_from_message(event): # Extract service tag from message.ddtags if exists if "service" in extracted_ddtags: - event[DD_SERVICE] = next(tag[8:] for tag in extracted_ddtags.split(",") if tag.startswith("service:")) + event[DD_SERVICE] = next( + tag[8:] + for tag in extracted_ddtags.split(",") + if tag.startswith("service:") + ) event[DD_CUSTOM_TAGS] = ",".join( [ tag diff --git a/aws/logs_monitoring/tests/test_lambda_function.py b/aws/logs_monitoring/tests/test_lambda_function.py index 6a573398d..3e81bab45 100644 --- a/aws/logs_monitoring/tests/test_lambda_function.py +++ b/aws/logs_monitoring/tests/test_lambda_function.py @@ -317,9 +317,7 @@ def test_extract_trace_payload_valid_trace(self): class TestMergeMessageTags(unittest.TestCase): - message_tags = ( - '{"ddtags":"service:my_application_service,custom_tag_1:value1"}' - ) + message_tags = '{"ddtags":"service:my_application_service,custom_tag_1:value1"}' custom_tags = "custom_tag_2:value2,service:my_custom_service" def test_extract_ddtags_from_message_str(self): @@ -361,7 +359,13 @@ def test_extract_ddtags_from_message_dict(self): def test_extract_ddtags_from_message_service_tag_setting(self): loaded_message_tags = json.loads(self.message_tags) - loaded_message_tags["ddtags"] = ",".join([tag for tag in loaded_message_tags["ddtags"].split(",") if not tag.startswith("service:")]) + loaded_message_tags["ddtags"] = ",".join( + [ + tag + for tag in loaded_message_tags["ddtags"].split(",") + if not tag.startswith("service:") + ] + ) event = { "message": loaded_message_tags, "ddtags": self.custom_tags, diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot index 42f0de013..674812320 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_custom_tags.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "custom_tag1:value1,custom_tag2:value2,forwarder_memorysize:1536,forwarder_version:,forwardername:test", + "ddtags": "forwardername:test,forwarder_memorysize:1536,forwarder_version:,custom_tag1:value1,custom_tag2:value2", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}", diff --git a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot index 4a4bfee20..a5bc3bf90 100644 --- a/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot +++ b/aws/logs_monitoring/tools/integration_tests/snapshots/cloudwatch_log_service_tag.json~snapshot @@ -14,7 +14,7 @@ }, "ddsource": "cloudwatch", "ddsourcecategory": "aws", - "ddtags": "forwarder_memorysize:1536,forwarder_version:,forwardername:test,service:myservice", + "ddtags": "forwardername:test,forwarder_memorysize:1536,forwarder_version:,service:myservice", "host": "testLogGroup", "id": "eventId1", "message": "{\"message\": \"hello world\"}",