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

refactor(metrics): optimize validation and serialization #307

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Mar 4, 2021

Issue #, if available: #303

Description of changes:

This change allows customers to disable metric validation to gain extra speed if they're confident they provided the bare minimum for metrics to be generated correctly in CloudWatch.

Checklist

  • Meet tenets criteria
  • Update tests
  • Update docs
  • PR title follows conventional commit semantics
  • Introduce validate_metrics parameter in Metrics and Single Metric
  • Assess whether native Python validation over JSON Schema validation gains significant speed while keeping maintenance low
  • Reduce string length within JSON encoder separator
  • Reduce system calls by merging validation logic as part of serialization and addition methods

Breaking change checklist

RFC issue #:

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

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

@heitorlessa heitorlessa marked this pull request as draft March 4, 2021 12:24
@heitorlessa heitorlessa added the feature New feature or functionality label Mar 4, 2021
@heitorlessa heitorlessa changed the title feat: allow metric validation to be disabled refactor(metrics): optimize validation and serialization Mar 4, 2021
@heitorlessa
Copy link
Contributor Author

Posted on original issue


Alright, here are the results with the lowest memory (128m) using the same 99 metrics -- @tgip-work @nmoutschen @michaelbrewer

I'm pretty happy with it, and will not proceed with ujson as the underlying machine Lambda uses doesn't seem to make much use tbh - it could be different in other scenarios perhaps. I'll adjust the test SLA to take into account GitHub CI slower machines but I'd consider it solved... let me know otherwise @tgip-work :)

BEFORE - Current public Powertools

Fastest Lambda execution according to X-Ray: 54ms

image

All virtual users finished
Summary report @ 16:09:49(+0100) 2021-03-04
Scenarios launched: 10
Scenarios completed: 10
Requests completed: 2000
Mean response/sec: 48.23
Response time (msec):
min: 124.1
max: 1212.6
median: 189.9
p95: 244.5
p99: 385.2
Scenario counts:
0: 10 (100%)
Codes:
200: 1999
502: 1


AFTER -- New Powertools with Lazy Loading and this PR optimization

Fastest Lambda execution according to X-Ray: 2.2ms

image

All virtual users finished
Summary report @ 16:13:18(+0100) 2021-03-04
Scenarios launched: 10
Scenarios completed: 10
Requests completed: 2000
Mean response/sec: 100.5
Response time (msec):
min: 63.1
max: 1128.9
median: 81.1
p95: 132.5
p99: 209
Scenario counts:
0: 10 (100%)
Codes:
200: 2000


AFTER - New optimizations + ujson

Fastest Lambda execution according to X-Ray: 2.6ms

image

All virtual users finished
Summary report @ 16:28:12(+0100) 2021-03-04
Scenarios launched: 10
Scenarios completed: 10
Requests completed: 2000
Mean response/sec: 105.71
Response time (msec):
min: 62.3
max: 525.1
median: 81
p95: 134
p99: 211.5
Scenario counts:
0: 10 (100%)
Codes:
200: 2000


Details

Load test with artillery using REST API + Lambda (could be even faster w/ HTTP API): artillery quick --count 10 -n 200 <endpoint>

Source code

from aws_lambda_powertools import Metrics

# https://awslabs.github.io/aws-lambda-powertools-python/#features
metrics = Metrics(namespace="perftest", service="perftest")


@metrics.log_metrics
def lambda_handler(event, context):
    for i in range(99):
        metrics.add_metric(name=f"metric_{i}", value=1, unit="Count")

    return {"statusCode": 200, "body": "hello world"}

@heitorlessa heitorlessa marked this pull request as ready for review March 4, 2021 15:56
@codecov-io
Copy link

Codecov Report

Merging #307 (fcdbeb8) into develop (b4d0baa) will decrease coverage by 0.24%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #307      +/-   ##
===========================================
- Coverage    99.78%   99.54%   -0.25%     
===========================================
  Files           91       90       -1     
  Lines         3315     3291      -24     
  Branches       157      160       +3     
===========================================
- Hits          3308     3276      -32     
- Misses           5       10       +5     
- Partials         2        5       +3     
Impacted Files Coverage Δ
aws_lambda_powertools/metrics/base.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/metric.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/metrics.py 100.00% <100.00%> (ø)
...wertools/utilities/idempotency/persistence/base.py 93.43% <0.00%> (-5.84%) ⬇️
aws_lambda_powertools/logging/logger.py 100.00% <0.00%> (ø)
aws_lambda_powertools/tracing/tracer.py 100.00% <0.00%> (ø)
aws_lambda_powertools/logging/formatter.py 100.00% <0.00%> (ø)
aws_lambda_powertools/utilities/batch/base.py 100.00% <0.00%> (ø)
aws_lambda_powertools/shared/jmespath_functions.py 100.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4d0baa...fcdbeb8. Read the comment docs.

@heitorlessa heitorlessa added enhancement and removed feature New feature or functionality labels Mar 4, 2021
@heitorlessa
Copy link
Contributor Author

thanks @nmoutschen ;) 🎉

@heitorlessa heitorlessa merged commit 271f560 into aws-powertools:develop Mar 4, 2021
@heitorlessa heitorlessa deleted the feat/skip_metrics_validation_and_caching branch March 4, 2021 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants