-
Notifications
You must be signed in to change notification settings - Fork 421
feat(metrics) - add support for high resolution metrics #1915
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 support for high resolution metrics #1915
Conversation
Codecov ReportBase: 97.52% // Head: 97.45% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1915 +/- ##
===========================================
- Coverage 97.52% 97.45% -0.07%
===========================================
Files 143 144 +1
Lines 6573 6628 +55
Branches 468 475 +7
===========================================
+ Hits 6410 6459 +49
- Misses 128 132 +4
- Partials 35 37 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
""" | ||
|
||
if isinstance(resolution, int): | ||
if resolution in self._metric_resolution_options: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: try renaming to _metric_resolution_valid_options
or something along these lines so it's quicker to read code for the intent you have at hand (validate whether resolution falls within the valid options.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed
SUPERB addition, thank you @leandrodamascena <3 Added additional tests and discovered the metric resolution error wasn't actionable (Accidentally), improved non-integer/enum values so it raises accordingly, otherwise it could lead to metric data loss. Final branching logic now |
Issue number: #1908
Summary
Changes
Added new parameter to Metrics.add_metric to support high resolution metrics, the latest Amazon Cloudwatch feature - https://aws.amazon.com/en/about-aws/whats-new/2023/02/amazon-cloudwatch-high-resolution-metric-extraction-structured-logs/
If the user does not send the resolution parameter, the default value (60) will be assumed according to AWS documentation.
User experience
Before
After
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
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.