fix(metrics): stop spurious overwrite warnings from set_default_dimensions

[vishwakt]

Issue number: closes #8402

Summary

Changes

There are two causes, and the second makes this worse than reported: even with default dimensions set once at module level, the recommended pattern, the warning fires for every default dimension on every flush.

1. Metrics.set_default_dimensions called self.provider.set_default_dimensions(**dimensions) and then re-added every dimension through add_dimension. The second pass always found the keys already registered, so the warning fired even on the very first call. The redundant loop is removed and the method now delegates to the provider. The docstring, which sat after the first statement and was not treated as a docstring by Python, moved to the top of the method.

2. The provider re-registers default dimensions internally: clear_metrics re-adds them after every flush, and warm invocations repeat set_default_dimensions. The previous condition in AmazonCloudWatchEMFProvider.add_dimension warned for any name already present in dimension_set or default_dimensions, so these internal paths warned too. It now warns only when a dimension is overwritten with a different value, which matches the warning message ("The previous value will be overwritten") and the intent of feat(metrics): warn when overwriting dimension #5653, whose test covers overwriting with a different value.

3. The provider constructor replaced a falsy default_dimensions argument with a new dict, so the initially empty dict Metrics passes in was silently swapped out and updates made through the provider never reached the dict Metrics owns. It now keeps the given dict unless None is passed. Credit to @ericbn, who found this in fix(metrics): warnings when there are default_dimensions #8404; his fix is applied here verbatim with commit co-authorship, as requested in review.

User experience

Before: one warning per dimension on the first set_default_dimensions call, plus one per default dimension on every flush and on every warm re-registration.

After: no warnings when nothing is overwritten. Overwriting a dimension with a different value still warns once, including when done via set_default_dimensions.

Seven tests added: no warning on first call, no warning on unchanged re-registration, exactly one warning when a default value changes, no warnings across log_metrics invocations, no warning when re-adding a dimension with the same value, and two regression tests confirming a provided empty default_dimensions dict remains shared between Metrics and the provider. The existing test_add_dimension_overwrite_warning passes unchanged.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[ericbn]

This change is still allowing the add_dimension method in the provider to be called twice. I see there's another redundant call: self.set_default_dimensions(**self.default_dimensions) in the clear_metrics method in the provider.

The change to hide the warning when the new value is the same as the old one is nice, but it's hiding the redundant call that's still happening. Not sure if I'd change the condition when the warning is emitted there.

Also, I think it might be worth investigating this whole code better. I don't get why Metrics keeps its own data and I don't understand the explanation in the comment in

powertools-lambda-python/aws_lambda_powertools/metrics/metrics.py

Lines 77 to 81 in 8db13c7

	# NOTE: We use class attrs to share metrics data across instances
	# this allows customers to initialize Metrics() throughout their code base (and middlewares)
	# and not get caught by accident with metrics data loss, or data deduplication
	# e.g., m1 and m2 add metric ProductCreated, however m1 has 'version' dimension but m2 doesn't
	# Result: ProductCreated is created twice as we now have 2 different EMF blobs

There's even a side effect here: since Metrics passes references of all its data structures to the provider, they're both (the Metrics and the provider) sharing the same instances of data. In this case mutating them from Metrics and from the provider is also redundant. And this does not happen if a Metrics is initialized with a provider instance given in the constructor by the user. So there's a lot of confusing complexity here, I'd say most of it possibly unintentional.

(Disclaimer: this was all reviewed and written without the use of AI)

[vishwakt]

Good catch on clear_metrics, but that call isn't actually redundant. serialize_metric_set only reads dimension_set, never default_dimensions, so the re-registration in clear_metrics is what brings the defaults back after every flush. Drop it and default dimensions would only show up in the first invocation's output (test_log_persist_default_dimensions covers this).

That's also why I changed the warning condition instead of removing the call: the re-registration is doing real work, it just shouldn't be reported to the user as an overwrite. And the old condition was inaccurate on its own anyway, since it warned "the previous value will be overwritten" even when the value hadn't changed. Real overwrites still warn.

The cleaner design would be to merge default_dimensions at serialize time and never store them in dimension_set, but that touches MAX_DIMENSIONS accounting and add_dimension's override semantics, so I kept it out of this PR.

Agreed the Metrics vs provider state sharing is confusing. The class attributes are intentional (shared state across Metrics() instances, see the NOTE at metrics.py:77), but the default provider aliasing those dicts while a custom provider diverges does feel accidental. Probably worth its own issue rather than growing this PR.

[ericbn]

Here's my take on this: #8404

[leandrodamascena]

Thanks for putting this together and for keeping the fix focused. Also, thank you @ericbn for spotting an important detail in #8404.

I think we should bring this default_dimensions initialization change into this PR before merging it.

Once Metrics.set_default_dimensions delegates only to the provider, the provider must preserve the dictionary passed by Metrics. Today we have:

self.default_dimensions = default_dimensions or {}

When Metrics passes its initially empty shared dictionary, it is falsy, so the provider replaces it with a new dictionary. Calling provider.set_default_dimensions(...) then updates only that new dictionary, while the dictionary owned by Metrics remains empty.

Eric fixed it in #8404 with:

self.default_dimensions = default_dimensions if default_dimensions is not None else {}

Could you please bring this small fix into the PR and add a regression test confirming that a provided empty dictionary remains shared after setting default dimensions?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.64%. Comparing base (8bf68fa) to head (2a70f9a).
⚠️ Report is 10 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #8403      +/-   ##
===========================================
- Coverage    96.64%   96.64%   -0.01%     
===========================================
  Files          296      296              
  Lines        14765    14762       -3     
  Branches      1245     1244       -1     
===========================================
- Hits         14269    14266       -3     
  Misses         361      361              
  Partials       135      135              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[vishwakt]

Done @leandrodamascena.

I brought in Eric's fix as-is, with commit co-authorship so the credit is on the record.

The provider now keeps the passed dict unless None is given. Added two regression tests, one at the provider level confirming a provided empty dict stays shared and receives updates through set_default_dimensions, and one confirming Metrics and the default provider hold the same dict afterwards.

@ericbn thanks for catching that one. The falsy empty dict swap explains the order-dependent sharing I stumbled over earlier in this thread, and you're right that it needed to land together with this change now that Metrics delegates to the provider.