Skip to content

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

Conversation

leandrodamascena
Copy link
Contributor

@leandrodamascena leandrodamascena commented Feb 8, 2023

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.

StorageResolution— An OPTIONAL integer value representing the storage resolution for the corresponding metric. Setting this to 1 specifies this metric as a high-resolution metric, so that CloudWatch stores the metric with sub-minute resolution down to one second. Setting this to 60 specifies this metric as standard-resolution, which CloudWatch stores at 1-minute resolution. Values SHOULD be valid CloudWatch supported resolutions, 1 or 60. If a value is not provided, then a default value of 60 is assumed.

User experience

Before

from aws_lambda_powertools import Metrics
from aws_lambda_powertools.metrics import MetricUnit
from aws_lambda_powertools.utilities.typing import LambdaContext

metrics = Metrics()


@metrics.log_metrics  # ensures metrics are flushed upon request completion/failure
def lambda_handler(event: dict, context: LambdaContext):
    metrics.add_metric(name="SuccessfulUpgrade", unit=MetricUnit.Count, value=1)

After

from aws_lambda_powertools import Metrics
from aws_lambda_powertools.metrics import MetricUnit, MetricResolution
from aws_lambda_powertools.utilities.typing import LambdaContext

metrics = Metrics()


@metrics.log_metrics  # ensures metrics are flushed upon request completion/failure
def lambda_handler(event: dict, context: LambdaContext):
    # Publish a metric with standard resolution i.e. StorageResolution = 60
    metrics.add_metric(name="SuccessfulBooking", unit=MetricUnit.Count, value=1, resolution=MetricResolution.Standard)

    # Publish a metric with high resolution i.e. StorageResolution = 1
    metrics.add_metric(name="FailedBooking", unit=MetricUnit.Count, value=1, resolution=MetricResolution.High)

    # The last parameter (storage resolution) is optional
    metrics.add_metric(name="SuccessfulUpgrade", unit=MetricUnit.Count, value=1)

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.

@leandrodamascena leandrodamascena requested a review from a team as a code owner February 8, 2023 23:15
@leandrodamascena leandrodamascena requested review from heitorlessa and removed request for a team February 8, 2023 23:15
@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation metrics labels Feb 8, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 8, 2023
@boring-cyborg boring-cyborg bot added the tests label Feb 8, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2023

Codecov Report

Base: 97.52% // Head: 97.45% // Decreases project coverage by -0.07% ⚠️

Coverage data is based on head (fd2b4d9) compared to base (92a526d).
Patch coverage: 100.00% of modified lines in pull request are covered.

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     
Impacted Files Coverage Δ
aws_lambda_powertools/metrics/metrics.py 100.00% <ø> (ø)
aws_lambda_powertools/metrics/__init__.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/base.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/exceptions.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/types.py 100.00% <100.00%> (ø)
aws_lambda_powertools/utilities/batch/base.py 94.67% <0.00%> (-3.12%) ⬇️
...ools/utilities/idempotency/persistence/dynamodb.py 98.76% <0.00%> (-0.05%) ⬇️
aws_lambda_powertools/utilities/batch/__init__.py 100.00% <0.00%> (ø)

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@heitorlessa heitorlessa linked an issue Feb 9, 2023 that may be closed by this pull request
2 tasks
@heitorlessa heitorlessa self-assigned this Feb 9, 2023
"""

if isinstance(resolution, int):
if resolution in self._metric_resolution_options:
Copy link
Contributor

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.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed

@heitorlessa heitorlessa changed the title feat(metrics) - Add high resolution metrics feat(metrics) - add support for high resolution metrics Feb 10, 2023
@heitorlessa
Copy link
Contributor

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

image

@leandrodamascena leandrodamascena merged commit a23f0bf into aws-powertools:develop Feb 10, 2023
@leandrodamascena leandrodamascena deleted the feat/metrics-high-resolution branch February 10, 2023 15:09
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 metrics size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: support high resolution metrics
3 participants