From a0942614d3701a9c7fbcd8889ae4517129f24106 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 2 Jun 2020 17:39:03 +0200 Subject: [PATCH 1/4] fix: cast dimension value to str to avoid issue where EMF silently fails to collect the metrics --- aws_lambda_powertools/metrics/base.py | 8 +++++- tests/functional/test_metrics.py | 35 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/aws_lambda_powertools/metrics/base.py b/aws_lambda_powertools/metrics/base.py index 83949ad874d..457a08a9506 100644 --- a/aws_lambda_powertools/metrics/base.py +++ b/aws_lambda_powertools/metrics/base.py @@ -203,7 +203,13 @@ def add_dimension(self, name: str, value: str): Dimension value """ logger.debug(f"Adding dimension: {name}:{value}") - self.dimension_set[name] = value + + # Cast value to str according to EMF spec + # Majority of values are expected to be string already, so checking before casting improves performance in most cases + if isinstance(value, str): + self.dimension_set[name] = value + else: + self.dimension_set[name] = str(value) def __extract_metric_unit_value(self, unit: Union[str, MetricUnit]) -> str: """Return metric value from metric unit whether that's str or MetricUnit enum diff --git a/tests/functional/test_metrics.py b/tests/functional/test_metrics.py index 71610bc0f19..7b69dc33834 100644 --- a/tests/functional/test_metrics.py +++ b/tests/functional/test_metrics.py @@ -47,6 +47,12 @@ def dimensions() -> List[Dict[str, str]]: {"name": "test_dimension_2", "value": "test"}, ] +@pytest.fixture +def non_str_dimensions() -> List[Dict[str, str]]: + return [ + {"name": "test_dimension", "value": True}, + {"name": "test_dimension_2", "value": 3}, + ] @pytest.fixture def namespace() -> Dict[str, str]: @@ -380,3 +386,32 @@ def lambda_handler(evt, context): # THEN metric set should be empty after function has been run assert my_metrics.metric_set == {} + +def test_log_metrics_non_string_dimension_values(capsys, metrics, non_str_dimensions, namespace): + # GIVEN Metrics is initialized and dimensions with non-string values are added + my_metrics = Metrics() + my_metrics.add_namespace(**namespace) + for metric in metrics: + my_metrics.add_metric(**metric) + for dimension in non_str_dimensions: + my_metrics.add_dimension(**dimension) + + # WHEN we utilize log_metrics to serialize + # and flush all metrics at the end of a function execution + @my_metrics.log_metrics + def lambda_handler(evt, ctx): + return True + + lambda_handler({}, {}) + + output = json.loads(capsys.readouterr().out.strip()) + expected = serialize_metrics(metrics=metrics, dimensions=non_str_dimensions, namespace=namespace) + + remove_timestamp(metrics=[output, expected]) # Timestamp will always be different + + # THEN we should have no exceptions + # and dimension values hould be serialized as strings + assert expected["_aws"] == output["_aws"] + for dimension in non_str_dimensions: + assert dimension["name"] in output + assert isinstance(output[dimension["name"]], str) \ No newline at end of file From 858552884fe2bf988d14f34ddebf6884b620b12a Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 2 Jun 2020 18:05:08 +0200 Subject: [PATCH 2/4] fix: correct type hint in fixture --- tests/functional/test_metrics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_metrics.py b/tests/functional/test_metrics.py index 7b69dc33834..a0894466d96 100644 --- a/tests/functional/test_metrics.py +++ b/tests/functional/test_metrics.py @@ -1,5 +1,5 @@ import json -from typing import Dict, List +from typing import Dict, List, Any import pytest @@ -48,7 +48,7 @@ def dimensions() -> List[Dict[str, str]]: ] @pytest.fixture -def non_str_dimensions() -> List[Dict[str, str]]: +def non_str_dimensions() -> List[Dict[str, Any]]: return [ {"name": "test_dimension", "value": True}, {"name": "test_dimension_2", "value": 3}, From 89a8370366bdccc796c2423bde584201612a1635 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 2 Jun 2020 18:07:25 +0200 Subject: [PATCH 3/4] fix: remove surplus assertions and logic from new test --- tests/functional/test_metrics.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/functional/test_metrics.py b/tests/functional/test_metrics.py index a0894466d96..c7bb44f16c6 100644 --- a/tests/functional/test_metrics.py +++ b/tests/functional/test_metrics.py @@ -403,15 +403,9 @@ def lambda_handler(evt, ctx): return True lambda_handler({}, {}) - output = json.loads(capsys.readouterr().out.strip()) - expected = serialize_metrics(metrics=metrics, dimensions=non_str_dimensions, namespace=namespace) - - remove_timestamp(metrics=[output, expected]) # Timestamp will always be different # THEN we should have no exceptions # and dimension values hould be serialized as strings - assert expected["_aws"] == output["_aws"] for dimension in non_str_dimensions: - assert dimension["name"] in output assert isinstance(output[dimension["name"]], str) \ No newline at end of file From c778f22f7dd90f2af1059b76c741ba7dd4db993d Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 2 Jun 2020 18:27:17 +0200 Subject: [PATCH 4/4] chore: fix formatting --- aws_lambda_powertools/metrics/base.py | 7 ++++--- tests/functional/test_metrics.py | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/aws_lambda_powertools/metrics/base.py b/aws_lambda_powertools/metrics/base.py index 457a08a9506..9d2f07b677f 100644 --- a/aws_lambda_powertools/metrics/base.py +++ b/aws_lambda_powertools/metrics/base.py @@ -205,11 +205,12 @@ def add_dimension(self, name: str, value: str): logger.debug(f"Adding dimension: {name}:{value}") # Cast value to str according to EMF spec - # Majority of values are expected to be string already, so checking before casting improves performance in most cases - if isinstance(value, str): + # Majority of values are expected to be string already, so + # checking before casting improves performance in most cases + if isinstance(value, str): self.dimension_set[name] = value else: - self.dimension_set[name] = str(value) + self.dimension_set[name] = str(value) def __extract_metric_unit_value(self, unit: Union[str, MetricUnit]) -> str: """Return metric value from metric unit whether that's str or MetricUnit enum diff --git a/tests/functional/test_metrics.py b/tests/functional/test_metrics.py index c7bb44f16c6..b5c1a3232de 100644 --- a/tests/functional/test_metrics.py +++ b/tests/functional/test_metrics.py @@ -1,5 +1,5 @@ import json -from typing import Dict, List, Any +from typing import Any, Dict, List import pytest @@ -47,6 +47,7 @@ def dimensions() -> List[Dict[str, str]]: {"name": "test_dimension_2", "value": "test"}, ] + @pytest.fixture def non_str_dimensions() -> List[Dict[str, Any]]: return [ @@ -54,6 +55,7 @@ def non_str_dimensions() -> List[Dict[str, Any]]: {"name": "test_dimension_2", "value": 3}, ] + @pytest.fixture def namespace() -> Dict[str, str]: return {"name": "test_namespace"} @@ -387,6 +389,7 @@ def lambda_handler(evt, context): # THEN metric set should be empty after function has been run assert my_metrics.metric_set == {} + def test_log_metrics_non_string_dimension_values(capsys, metrics, non_str_dimensions, namespace): # GIVEN Metrics is initialized and dimensions with non-string values are added my_metrics = Metrics() @@ -408,4 +411,4 @@ def lambda_handler(evt, ctx): # THEN we should have no exceptions # and dimension values hould be serialized as strings for dimension in non_str_dimensions: - assert isinstance(output[dimension["name"]], str) \ No newline at end of file + assert isinstance(output[dimension["name"]], str)