Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(metrics): add flush_metrics() method to allow manual flushing of metrics #2171

Merged
merged 6 commits into from
Apr 28, 2023

Conversation

rubenfonseca
Copy link
Contributor

@rubenfonseca rubenfonseca commented Apr 27, 2023

Issue number: #2109

Summary

Changes

Please provide a summary of what's being changed

This PR adds a new flush_metrics to the metrics utility to manually flush the metrics.

carbon (1)

User experience

Please share what the user experience looks like before and after this change

Before this PR it was hard for users of the metrics utility to flush the metrics when not running on the Lambda runtime. This simple refactor enables the users
quick access to manually flushing the metrics.

image

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

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

Acknowledgment

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation metrics tests labels Apr 27, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 27, 2023
@rubenfonseca rubenfonseca changed the title feat(logger): add flush_metrics feat(metrics): add flush_metrics Apr 27, 2023
@github-actions github-actions bot added the feature New feature or functionality label Apr 27, 2023
@rubenfonseca rubenfonseca linked an issue Apr 27, 2023 that may be closed by this pull request
2 tasks
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example file was not being used in the documentation, so I just changed the content and started using it.

@rubenfonseca rubenfonseca marked this pull request as ready for review April 27, 2023 13:27
@rubenfonseca rubenfonseca requested a review from a team as a code owner April 27, 2023 13:27
@rubenfonseca rubenfonseca requested review from leandrodamascena and removed request for a team April 27, 2023 13:27
@codecov-commenter
Copy link

codecov-commenter commented Apr 27, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (4663745) 97.46% compared to head (d10d371) 97.46%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2171   +/-   ##
========================================
  Coverage    97.46%   97.46%           
========================================
  Files          147      147           
  Lines         6872     6875    +3     
  Branches       505      505           
========================================
+ Hits          6698     6701    +3     
  Misses         137      137           
  Partials        37       37           
Impacted Files Coverage Δ
aws_lambda_powertools/metrics/base.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another excellent job Ruben!!

I'm wondering if we should do the same as the typescript and move the "Flushing metrics manually" topic to the "Flushing Metrics" menu. What do you think about?

Reference:
https://awslabs.github.io/aws-lambda-powertools-typescript/latest/core/metrics/#manually

image

@rubenfonseca
Copy link
Contributor Author

Peer-review feedback:

  • remove single metric example
  • expand the example with try/catch to handle uncaught exceptions when manually flushing metrics

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dummy logic to try/finally makes more sense

examples/metrics/src/flush_metrics.py Show resolved Hide resolved
examples/metrics/src/flush_metrics.py Outdated Show resolved Hide resolved
rubenfonseca and others added 3 commits April 28, 2023 15:20
Co-authored-by: Heitor Lessa <lessa@amazon.co.uk>
Signed-off-by: Ruben Fonseca <fonseka@gmail.com>
Co-authored-by: Heitor Lessa <lessa@amazon.co.uk>
Signed-off-by: Ruben Fonseca <fonseka@gmail.com>
@leandrodamascena leandrodamascena changed the title feat(metrics): add flush_metrics feat(metrics): add flush_metrics to flush metrics manually Apr 28, 2023
@leandrodamascena leandrodamascena changed the title feat(metrics): add flush_metrics to flush metrics manually feat(metrics): add flush_metrics method to flush metrics manually Apr 28, 2023
@leandrodamascena leandrodamascena changed the title feat(metrics): add flush_metrics method to flush metrics manually feat(metrics): add flush_metrics() method to allow manual flushing of metrics Apr 28, 2023
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved :)
image

@leandrodamascena leandrodamascena merged commit d4607f3 into develop Apr 28, 2023
@leandrodamascena leandrodamascena deleted the rf/flush_metrics branch April 28, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature New feature or functionality metrics size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Add A method to manually flush metrics
4 participants