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

fix(metrics): support additional arguments in functions wrapped with log_metrics decorator #3120

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

thegeorgeliu
Copy link
Contributor

Issue number: #3119

Summary

Changes

Please provide a summary of what's being changed

Allow a function wrapped by the @metrics.log_metrics decorator to accept arguments besides the event and lambda context. The decorate() function now takes in *args and **kwargs as arguments and then passes them to the lambda_handler() call.

User experience

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

Previously, the user would experience a "wrong number of positional arguments" or "unexpected keyword argument" TypeError when attempting to call a function wrapped by the decorator (e.g. lambda handler) with additional arguments.

After this change, the user is able to successfully pass additional arguments, positional or keyword, to the wrapped function without raising any errors.

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)

No, this is not a breaking change.

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.

@thegeorgeliu thegeorgeliu requested a review from a team September 21, 2023 23:39
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 21, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 21, 2023

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@heitorlessa
Copy link
Contributor

@thegeorgeliu thank you so much for taking the time to report and send a fix (with tests!).

We'll include it in tomorrows release.

cc @leandrodamascena

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.03% ⚠️

Comparison is base (1009729) 96.13% compared to head (63645f1) 96.11%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3120      +/-   ##
===========================================
- Coverage    96.13%   96.11%   -0.03%     
===========================================
  Files          186      186              
  Lines         8131     8131              
  Branches      1525      618     -907     
===========================================
- Hits          7817     7815       -2     
- Misses         252      253       +1     
- Partials        62       63       +1     
Files Changed Coverage Δ
aws_lambda_powertools/metrics/base.py 71.23% <0.00%> (ø)
aws_lambda_powertools/metrics/provider/base.py 95.65% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@heitorlessa heitorlessa merged commit e5cab1e into aws-powertools:develop Sep 22, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 22, 2023

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

@heitorlessa heitorlessa added the bug Something isn't working label Sep 22, 2023
@heitorlessa
Copy link
Contributor

Released in 2.25.1 in PyPi. Building and optimizing Lambda Layers now -- should be all done in ~20m or so. Next Layer version being 45.

rubenfonseca pushed a commit that referenced this pull request Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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.

Bug: @metrics.log_metrics decorator does not accept additional arguments passed to wrapped function
3 participants