From 8648078ceb8bd15ddd60dd228c14cf44bea65069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ana=20Falc=C3=A3o?= Date: Tue, 26 Nov 2024 14:08:10 -0300 Subject: [PATCH] feat(metrics): warn when overwriting dimension (#5653) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add warning to metrics overwrite dimensions * resolve conflicts cloudwatch file --------- Signed-off-by: Ana Falcão Co-authored-by: Ana Falcao --- .../provider/cloudwatch_emf/cloudwatch.py | 24 +++++++---- tests/unit/metrics/test_functions.py | 41 +++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py index 4fc77b9467c..50ad1871953 100644 --- a/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py +++ b/aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py @@ -22,6 +22,8 @@ from aws_lambda_powertools.metrics.provider.cloudwatch_emf.metric_properties import MetricResolution, MetricUnit from aws_lambda_powertools.shared import constants from aws_lambda_powertools.shared.functions import resolve_env_var_choice +from aws_lambda_powertools.warnings import PowertoolsUserWarning + if TYPE_CHECKING: from aws_lambda_powertools.metrics.provider.cloudwatch_emf.types import CloudWatchEMFOutput @@ -278,14 +280,22 @@ def add_dimension(self, name: str, value: str) -> None: 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", + "Ensure the dimension name and value are non-empty strings", + category=PowertoolsUserWarning, 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 + return + + if name in self.dimension_set or name in self.default_dimensions: + warnings.warn( + f"Dimension '{name}' has already been added. The previous value will be overwritten.", + category=PowertoolsUserWarning, + stacklevel=2, + ) + + self.dimension_set[name] = value + + def add_metadata(self, key: str, value: Any) -> None: """Adds high cardinal metadata for metrics object @@ -294,7 +304,7 @@ def add_metadata(self, key: str, value: Any) -> None: Instead, this will be searchable through logs. If you're looking to add metadata to filter metrics, then - use add_dimensions method. + use add_dimension method. Example ------- diff --git a/tests/unit/metrics/test_functions.py b/tests/unit/metrics/test_functions.py index f3414720bba..142be729ae6 100644 --- a/tests/unit/metrics/test_functions.py +++ b/tests/unit/metrics/test_functions.py @@ -1,4 +1,5 @@ import pytest +import warnings from aws_lambda_powertools.metrics.functions import ( extract_cloudwatch_metric_resolution_value, @@ -9,6 +10,18 @@ MetricUnitError, ) from aws_lambda_powertools.metrics.provider.cloudwatch_emf.metric_properties import MetricResolution, MetricUnit +from aws_lambda_powertools.metrics import Metrics +from aws_lambda_powertools.warnings import PowertoolsUserWarning + +@pytest.fixture +def warning_catcher(monkeypatch): + caught_warnings = [] + + def custom_warn(message, category=None, stacklevel=1, source=None): + caught_warnings.append(PowertoolsUserWarning(message)) + + monkeypatch.setattr(warnings, 'warn', custom_warn) + return caught_warnings def test_extract_invalid_cloudwatch_metric_resolution_value(): @@ -61,3 +74,31 @@ def test_extract_valid_cloudwatch_metric_unit_value(): # THEN value must be extracted assert extracted_unit_value == unit + + +def test_add_dimension_overwrite_warning(warning_catcher): + """ + Adds a dimension and then tries to add another with the same name + but a different value. Verifies if the dimension is updated with + the new value and warning is issued when an existing dimension + is overwritten. + """ + metrics = Metrics(namespace="TestNamespace") + + # GIVEN default dimension + dimension_name = "test-dimension" + value1 = "test-value-1" + value2 = "test-value-2" + + # WHEN adding the same dimension twice with different values + metrics.add_dimension(dimension_name, value1) + metrics.add_dimension(dimension_name, value2) + + # THEN the dimension should be updated with the new value + assert metrics._dimensions[dimension_name] == value2 + + # AND a warning should be issued with the exact message + expected_warning = f"Dimension '{dimension_name}' has already been added. The previous value will be overwritten." + assert any(str(w) == expected_warning for w in warning_catcher) + +