From 1f103e47c2d49fdca31e9bbc09b47c5aaecb4254 Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Tue, 12 Nov 2024 09:55:29 +0000 Subject: [PATCH 1/3] Adding warning when dimension name or value is empty --- .../provider/cloudwatch_emf/cloudwatch.py | 16 ++++++++++++---- .../test_metrics_cloudwatch_emf.py | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py index 5da02528aab..f8f0334a94a 100644 --- a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py +++ b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py @@ -272,10 +272,18 @@ def add_dimension(self, name: str, value: str) -> None: raise SchemaValidationError( f"Maximum number of dimensions exceeded ({MAX_DIMENSIONS}): Unable to add dimension {name}.", ) - # 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 - self.dimension_set[name] = value if isinstance(value, str) else str(value) + + if not name or not value: + warnings.warn( + f"The dimension {name} doesn't meet the requirements and won't be added. " + "Ensure the dimension name and value are non empty strings", + stacklevel=2, + ) + else: + # 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 + self.dimension_set[name] = value if isinstance(value, str) else str(value) def add_metadata(self, key: str, value: Any) -> None: """Adds high cardinal metadata for metrics object diff --git a/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py b/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py index c09660b4f9a..b4cccc0bc92 100644 --- a/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py +++ b/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py @@ -1045,6 +1045,25 @@ def test_clear_default_dimensions(namespace): assert not my_metrics.default_dimensions +def test_add_dimensions_with_empty_value(namespace, capsys, metric): + # GIVEN Metrics is initialized + my_metrics = Metrics(namespace=namespace) + + my_dimension = "my_empty_dimension" + + # WHEN we try to add a dimension with empty value + with pytest.warns(UserWarning, match=f"The dimension {my_dimension} doesn't meet the requirements *"): + my_metrics.add_dimension(name="my_empty_dimension", value="") + + my_metrics.add_metric(**metric) + my_metrics.flush_metrics() + + output = capture_metrics_output(capsys) + + # THEN the serialized metric should not contain this dimension + assert my_dimension not in output + + def test_get_and_set_namespace_and_service_properties(namespace, service, metrics, capsys): # GIVEN Metrics instance is initialized without namespace and service my_metrics = Metrics() From 8744555bae2a5fd8640f75b5569e2174b0772bc1 Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Tue, 12 Nov 2024 10:12:30 +0000 Subject: [PATCH 2/3] Strip empty chars --- .../metrics/provider/cloudwatch_emf/cloudwatch.py | 2 +- docs/core/metrics.md | 4 +++- .../required_dependencies/test_metrics_cloudwatch_emf.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py index f8f0334a94a..487d29f272d 100644 --- a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py +++ b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py @@ -273,7 +273,7 @@ def add_dimension(self, name: str, value: str) -> None: f"Maximum number of dimensions exceeded ({MAX_DIMENSIONS}): Unable to add dimension {name}.", ) - if not name or not value: + if not name.strip() or not value.strip(): warnings.warn( f"The dimension {name} doesn't meet the requirements and won't be added. " "Ensure the dimension name and value are non empty strings", diff --git a/docs/core/metrics.md b/docs/core/metrics.md index 5f2fec3d134..6fdcf1fa043 100644 --- a/docs/core/metrics.md +++ b/docs/core/metrics.md @@ -25,7 +25,7 @@ If you're new to Amazon CloudWatch, there are five terminologies you must be awa * **Resolution**. It's a value representing the storage resolution for the corresponding metric. Metrics can be either Standard or High resolution. Read more [here](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/publishingMetrics.html#high-resolution-metrics){target="_blank"}.
- + Terminology
Metric terminology, visually explained
@@ -131,6 +131,8 @@ If you'd like to remove them at some point, you can use `clear_default_dimension --8<-- "examples/metrics/src/set_default_dimensions_log_metrics.py" ``` +**Note:** Dimensions with empty values will not be included. + ### Changing default timestamp When creating metrics, we use the current timestamp. If you want to change the timestamp of all the metrics you create, utilize the `set_timestamp` function. You can specify a datetime object or an integer representing an epoch timestamp in milliseconds. diff --git a/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py b/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py index b4cccc0bc92..5633d573a54 100644 --- a/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py +++ b/tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py @@ -1053,7 +1053,7 @@ def test_add_dimensions_with_empty_value(namespace, capsys, metric): # WHEN we try to add a dimension with empty value with pytest.warns(UserWarning, match=f"The dimension {my_dimension} doesn't meet the requirements *"): - my_metrics.add_dimension(name="my_empty_dimension", value="") + my_metrics.add_dimension(name="my_empty_dimension", value=" ") my_metrics.add_metric(**metric) my_metrics.flush_metrics() From 9dc8bf810d91aff7e0664988525781312273f9b7 Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Tue, 12 Nov 2024 10:31:10 +0000 Subject: [PATCH 3/3] Strip empty chars --- .../metrics/provider/cloudwatch_emf/cloudwatch.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py index 487d29f272d..4fc77b9467c 100644 --- a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py +++ b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py @@ -273,6 +273,8 @@ def add_dimension(self, name: str, value: str) -> None: f"Maximum number of dimensions exceeded ({MAX_DIMENSIONS}): Unable to add dimension {name}.", ) + value = value if isinstance(value, str) else str(value) + if not name.strip() or not value.strip(): warnings.warn( f"The dimension {name} doesn't meet the requirements and won't be added. " @@ -283,7 +285,7 @@ def add_dimension(self, name: str, value: str) -> None: # 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 - self.dimension_set[name] = value if isinstance(value, str) else str(value) + self.dimension_set[name] = value def add_metadata(self, key: str, value: Any) -> None: """Adds high cardinal metadata for metrics object