fix(metrics): stop spurious overwrite warnings from set_default_dimensions (#8403)

* fix(metrics): stop spurious overwrite warnings from set_default_dimensions

Metrics.set_default_dimensions called provider.set_default_dimensions
and then re-added every dimension through add_dimension, so the second
pass always found the keys already registered and warned even on the
first call. Remove the redundant loop and delegate to the provider.

The provider also re-registers default dimensions internally, in
clear_metrics after every flush and on repeated set_default_dimensions
calls, which triggered the same warning on every warm invocation. Warn
only when a dimension is overwritten with a different value, matching
the warning message and the intent of #5653.

Closes #8402

* fix(metrics): preserve shared default_dimensions dict in provider

The provider replaced a falsy default_dimensions argument with a new
dict, so the initially empty dict that Metrics shares was silently
swapped out and updates made through the provider never reached the
dict Metrics owns. Keep the given dict unless None is passed.

Fix taken from #8404, requested in review.

Co-authored-by: Eric Nielsen <4120606+ericbn@users.noreply.github.com>

---------

Co-authored-by: Eric Nielsen <4120606+ericbn@users.noreply.github.com>
Co-authored-by: Leandro Damascena <lcdama@amazon.pt>

Author: 3 people

aws_lambda_powertools/metrics/metrics.py

@@ -174,7 +174,6 @@ def log_metrics(
         )
 
     def set_default_dimensions(self, **dimensions) -> None:
-        self.provider.set_default_dimensions(**dimensions)
         """Persist dimensions across Lambda invocations
 
         Parameters
@@ -195,9 +194,7 @@ def set_default_dimensions(self, **dimensions) -> None:
             def lambda_handler():
                 return True
         """
-        for name, value in dimensions.items():
-            self.add_dimension(name, value)
-
+        self.provider.set_default_dimensions(**dimensions)
         self.default_dimensions.update(**dimensions)
 
     def clear_default_dimensions(self) -> None:

aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py

@@ -87,7 +87,7 @@ def __init__(
     ):
         self.metric_set = metric_set if metric_set is not None else {}
         self.dimension_set = dimension_set if dimension_set is not None else {}
-        self.default_dimensions = default_dimensions or {}
+        self.default_dimensions = default_dimensions if default_dimensions is not None else {}
         self.namespace = resolve_env_var_choice(choice=namespace, env=os.getenv(constants.METRICS_NAMESPACE_ENV))
         self.service = resolve_env_var_choice(choice=service, env=os.getenv(constants.SERVICE_NAME_ENV))
         self.function_name = function_name
@@ -317,7 +317,7 @@ def add_dimension(self, name: str, value: str) -> None:
             )
             return
 
-        if name in self.dimension_set or name in self.default_dimensions:
+        if name in self.dimension_set and self.dimension_set[name] != value:
             warnings.warn(
                 f"Dimension '{name}' has already been added. The previous value will be overwritten.",
                 category=PowertoolsUserWarning,

tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py

@@ -1133,6 +1133,105 @@ def test_clear_default_dimensions(namespace):
     assert not my_metrics.default_dimensions
 
 
+def test_set_default_dimensions_no_warning_on_first_call(namespace):
+    # GIVEN a Metrics instance with no dimensions set
+    my_metrics = Metrics(namespace=namespace)
+
+    # WHEN we persist default dimensions for the first time
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("default")
+        my_metrics.set_default_dimensions(environment="test", log_group="/lambda/test")
+
+    # THEN no overwrite warning should be emitted
+    assert not [warning for warning in w if "has already been added" in str(warning.message)]
+
+
+def test_set_default_dimensions_no_warning_when_unchanged(namespace):
+    # GIVEN a Metrics instance with default dimensions persisted
+    my_metrics = Metrics(namespace=namespace)
+    my_metrics.set_default_dimensions(environment="test", log_group="/lambda/test")
+
+    # WHEN we persist the same default dimensions again e.g., on a warm invocation
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("default")
+        my_metrics.set_default_dimensions(environment="test", log_group="/lambda/test")
+
+    # THEN no overwrite warning should be emitted
+    assert not [warning for warning in w if "has already been added" in str(warning.message)]
+
+
+def test_set_default_dimensions_warns_when_value_changes(namespace):
+    # GIVEN a Metrics instance with a default dimension persisted
+    my_metrics = Metrics(namespace=namespace)
+    my_metrics.set_default_dimensions(environment="test")
+
+    # WHEN we persist the same default dimension with a different value
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("default")
+        my_metrics.set_default_dimensions(environment="prod")
+
+    # THEN a single overwrite warning should be emitted
+    assert len([warning for warning in w if "has already been added" in str(warning.message)]) == 1
+
+
+def test_log_metrics_with_default_dimensions_no_warning_across_invocations(namespace, metric, capsys):
+    # GIVEN a Metrics instance with default dimensions persisted
+    my_metrics = Metrics(namespace=namespace)
+    my_metrics.set_default_dimensions(environment="test", log_group="/lambda/test")
+
+    @my_metrics.log_metrics
+    def lambda_handler(evt, ctx):
+        my_metrics.add_metric(**metric)
+
+    # WHEN metrics are flushed across multiple invocations
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("default")
+        lambda_handler({}, {})
+        lambda_handler({}, {})
+
+    # THEN no overwrite warning should be emitted
+    assert not [warning for warning in w if "has already been added" in str(warning.message)]
+
+
+def test_provider_keeps_provided_default_dimensions_dict(namespace):
+    # GIVEN a provider constructed with an empty default dimensions dict e.g., the one Metrics shares
+    shared_default_dimensions: dict = {}
+    my_provider = AmazonCloudWatchEMFProvider(namespace=namespace, default_dimensions=shared_default_dimensions)
+
+    # WHEN default dimensions are set through the provider
+    my_provider.set_default_dimensions(environment="test")
+
+    # THEN the provided dict remains in use and receives the update
+    assert my_provider.default_dimensions is shared_default_dimensions
+    assert shared_default_dimensions == {"environment": "test"}
+
+
+def test_metrics_shares_default_dimensions_with_provider(namespace):
+    # GIVEN a Metrics instance with the default provider
+    my_metrics = Metrics(namespace=namespace)
+
+    # WHEN default dimensions are set
+    my_metrics.set_default_dimensions(environment="test")
+
+    # THEN Metrics and the provider hold the same dict, both with the update
+    assert my_metrics.default_dimensions is my_metrics.provider.default_dimensions
+    assert my_metrics.default_dimensions == {"environment": "test"}
+
+
+def test_add_dimension_no_warning_when_value_unchanged(namespace):
+    # GIVEN a Metrics instance with a dimension added
+    my_metrics = Metrics(namespace=namespace)
+    my_metrics.add_dimension("environment", "test")
+
+    # WHEN the same dimension is added again with the same value
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("default")
+        my_metrics.add_dimension("environment", "test")
+
+    # THEN no overwrite warning should be emitted
+    assert not [warning for warning in w if "has already been added" in str(warning.message)]
+
+
 def test_add_dimensions_with_empty_value(namespace, capsys, metric):
     # GIVEN Metrics is initialized
     my_metrics = Metrics(namespace=namespace)