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): captureColdStartMetric and throwOnEmptyMetrics when set to false was interpreted as true #1090

Merged
merged 1 commit into from
Sep 29, 2022

Conversation

flochaz
Copy link
Contributor

@flochaz flochaz commented Sep 16, 2022

Description of your changes

Metrics' Middy implementation was checking for undefined to evaluate throwOnEmptyMetrics and captureColdStartMetrics.

How to verify this change

  1. Get only the unit tests from this PR, see them fail
  2. Get the update on middy.ts
  3. rerun the unit tests and see them passed

Related issues, RFCs

Issue number: #1088

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding changes to the examples
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

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

@github-actions
Copy link
Contributor

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

@github-actions github-actions bot added do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further bug Something isn't working labels Sep 16, 2022
@dreamorosi dreamorosi added metrics This item relates to the Metrics Utility and removed do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels Sep 17, 2022
@dreamorosi dreamorosi changed the title fix: captureColdStartMetric and throwOnEmptyMetrics when set to false was interpreted as true fix(metrics): captureColdStartMetric and throwOnEmptyMetrics when set to false was interpreted as true Sep 19, 2022
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@saragerion saragerion merged commit 127aad4 into main Sep 29, 2022
@saragerion saragerion deleted the bug-1088 branch September 29, 2022 09:50
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 This item relates to the Metrics Utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: throws on empty metrics even if told not to in middy middleware
3 participants