feat: improve error handling for log_metrics decorator

[to-mc]

Issue #, if available:

Description of changes:

Change default behavior for log_metrics decorator: no longer throws exception if no metrics are emitted during the wrapped function's execution. Added parameter to revert this behavior if exception is desired in this case.

• Meet tenets criteria
• Update tests
• Update docs
• PR title follows conventional commit semantics

Breaking change checklist

Ignore if it's not a breaking change

RFC issue #:

• Migration process documented
• Implement warnings (if it can live side by side)

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

[codecov-commenter]

Codecov Report

Merging #71 into develop will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop       #71   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        16    +1     
  Lines          508       516    +8     
  Branches        48        49    +1     
=========================================
+ Hits           508       516    +8     

Impacted Files	Coverage Δ
aws_lambda_powertools/metrics/metrics.py	100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/base.py	100.00% <0.00%> (ø)
aws_lambda_powertools/metrics/metric.py	100.00% <0.00%> (ø)
aws_lambda_powertools/metrics/__init__.py	100.00% <0.00%> (ø)
aws_lambda_powertools/package_logger.py	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 967d86e...a78dc10. Read the comment docs.

[heitorlessa]

I'm trying to think of an actual customer scenario where this would be common to have no validation by default - Could you share one or two use cases where you wouldn't have a metric if you want to use Metrics?

I thought about scenarios where metric value would be 0, but even then it's a data point rather than missing data point.

[heitorlessa]

That’s fair enough - I thought you were also adding metrics on unhappy path (my expectation). I’m happy with the change - On my phone now, only change I’d request is to use “raise_on...” rather than “raise_for...”

…

On Mon, 8 Jun 2020 at 17:37, Tom McCarthy ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In aws_lambda_powertools/metrics/metrics.py <#71 (comment)> : > @@ -122,10 +131,13 @@ def decorate(event, context): if capture_cold_start_metric: self.__add_cold_start_metric(context=context) finally: - metrics = self.serialize_metric_set() - self.clear_metrics() - logger.debug("Publishing metrics", {"metrics": metrics}) - print(json.dumps(metrics)) + if not raise_for_empty_metrics and not self.metric_set: I added it to the decorator rather than base because I think the behavior should be different for the decorator. If I'm explicitly making a call to serialize metrics when their are none, throwing an exception is fine and I can handle it myself. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#71 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBAEJCOB6HNJILPIPETRVUHVFANCNFSM4NYPVZBQ> .

[to-mc]

That’s fair enough - I thought you were also adding metrics on unhappy path (my expectation). I’m happy with the change - On my phone now, only change I’d request is to use “raise_on...” rather than “raise_for...”

Done!