feat(metrics): logMetrics middleware by saragerion · Pull Request #338 · aws-powertools/powertools-lambda-typescript

saragerion · 2021-12-17T21:13:23Z

Description of your changes

Business logic, tests, documentation and examples added for the logMetrics middy middleware.

How to verify this change

Docs:

build the doc with npm run docs-runLocalDocker0
go to http://localhost:8000/core/metrics/

Examples:
See added NPM scripts for examples in the package.json file.

Related issues, RFCs

#25

PR status

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

Checklist

Breaking change checklist

N/A

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

dreamorosi

Great job, really like the rewording & edits on the docs as well.

Left some minor notes/comments.

Co-authored-by: Andrea Amorosi <dreamorosi@gmail.com>

ijemmy

Thanks for cleaning out the inconsistent case style!

ijemmy · 2021-12-22T14:02:14Z

+      const getRandomInt = (): number => Math.floor(Math.random() * 1000000000);
+
+      const awsRequestId = getRandomInt().toString();
+      const context = {


(minor) Can we extract context object out to reuse and avoid duplication?

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

flochaz · 2021-12-27T10:05:00Z

      const metrics = new Metrics({ namespace: 'test' });

-      const handler = async (event: any, context: Context) => {
+      const handler = async (_event: unknown, _context: unknown): Promise<void> => {


Why moving _context to Unknown type ?

I was getting a linting error, and because we should use unknown instead of any, whenever needed: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#any

weird that we are getting linting error when (for once :) ) we know which type to expect :/

I fixed it with a more descriptive type

flochaz · 2021-12-27T10:05:07Z


      const metrics = new Metrics({ namespace: 'test' });
-      const handler = async (event: any, context: Context) => {
+      const handler = async (_event: unknown, _context: unknown): Promise<void> => {


See here: #338 (comment)

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

Co-authored-by: Florian Chazal <florianchazal@gmail.com>

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

…da-powertools-typescript into feat/metrics-middleware

dreamorosi

LGTM, thanks a lot for taking this one!

saragerion added 4 commits December 17, 2021 22:04

feat(metric): add logic, unit tests for the middy middleware

8fbe2dd

chore(metrics): lint changes

79e5aae

docs(metrics): logMetric middleware and examples

9c1644d

test(metrics): add array input for middleware

f430195

saragerion changed the title ~~Feat/metrics middleware~~ feat(metrics): logMetrics middleware Dec 21, 2021

saragerion marked this pull request as ready for review December 21, 2021 18:27

saragerion requested review from dreamorosi, flochaz and ijemmy December 21, 2021 18:27

dreamorosi assigned saragerion Dec 22, 2021

dreamorosi added metrics This item relates to the Metrics Utility enhancement labels Dec 22, 2021

dreamorosi added this to the beta-release milestone Dec 22, 2021

saragerion linked an issue Dec 22, 2021 that may be closed by this pull request

Feature request: Metrics utility #25

Closed

8 tasks

dreamorosi reviewed Dec 22, 2021

View reviewed changes

Update packages/metrics/package.json

69100ce

Co-authored-by: Andrea Amorosi <dreamorosi@gmail.com>

ijemmy suggested changes Dec 22, 2021

View reviewed changes

saragerion and others added 4 commits December 22, 2021 15:05

Update docs/core/metrics.md

d3d13ea

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

Update docs/core/metrics.md

ef6a58e

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

Update docs/core/metrics.md

f52e198

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

Update docs/core/metrics.md

d503075

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

dreamorosi previously approved these changes Dec 23, 2021

View reviewed changes

flochaz reviewed Dec 27, 2021

View reviewed changes

Comment thread packages/metrics/examples/cold-start.ts

flochaz reviewed Dec 27, 2021

View reviewed changes

saragerion mentioned this pull request Dec 27, 2021

Feature request: rename raiseOnEmptyMetrics to throwOnEmptyMetrics #348

Closed

saragerion added 2 commits December 27, 2021 13:47

improv(metrics): improve docs, examples, naming

419c8d2

chore(metrics): merge conflict

f29fc49

saragerion dismissed dreamorosi’s stale review via f29fc49 December 27, 2021 12:51

saragerion and others added 2 commits December 27, 2021 13:51

Update packages/metrics/examples/decorator/default-dimensions.ts

ac08ffe

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

Update packages/metrics/examples/decorator/empty-metrics.ts

a42b050

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

saragerion and others added 7 commits December 27, 2021 14:28

Update docs/core/metrics.md

625b79d

Co-authored-by: Florian Chazal <florianchazal@gmail.com>

Update docs/core/metrics.md

0d5c161

Co-authored-by: Florian Chazal <florianchazal@gmail.com>

Update packages/metrics/examples/decorator/manual-flushing.ts

d75c40f

Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>

improv(metrics): logMetricsAfterOrError middleware, examples update

3d423e5

improv(metrics): logMetricsAfterOrError middleware, examples update

2b23c86

improv(metrics): logMetricsAfterOrError middleware, examples update

9b295ed

Merge branch 'feat/metrics-middleware' of github.com:awslabs/aws-lamb…

8269677

…da-powertools-typescript into feat/metrics-middleware

dreamorosi previously approved these changes Dec 27, 2021

View reviewed changes

flochaz previously approved these changes Dec 27, 2021

View reviewed changes

tests(metrics): fix handlers type

29d1c05

saragerion dismissed stale reviews from flochaz and dreamorosi via 29d1c05 December 27, 2021 15:07

saragerion added 3 commits December 27, 2021 16:11

fix(metrics): examples file path

d23bbc5

revert(metrics): jest groups

7941119

chore(metrics): rename examples script, prepending 'decorator'

3b6b053

saragerion requested review from dreamorosi and flochaz December 27, 2021 15:23

dreamorosi previously approved these changes Dec 27, 2021

View reviewed changes

flochaz previously approved these changes Dec 27, 2021

View reviewed changes

ijemmy previously approved these changes Dec 27, 2021

View reviewed changes

chore(all): merge

fe6bad1

saragerion dismissed stale reviews from ijemmy, flochaz, and dreamorosi via fe6bad1 December 27, 2021 19:44

saragerion added 4 commits December 27, 2021 20:53

fix(metrics): removed empty lint file

0ab77da

ci(all): exclude examples CDK for linting

c909154

chore(all): lint fix for CDK examples

a95f08c

test(metrics): added metrics middleware jest group

28b9df1

saragerion merged commit f8cf705 into main Dec 27, 2021

saragerion deleted the feat/metrics-middleware branch December 27, 2021 20:22

Uh oh!

Conversation

saragerion commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

How to verify this change

Related issues, RFCs

PR status

Checklist

Breaking change checklist

Uh oh!

dreamorosi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ijemmy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreamorosi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

saragerion commented Dec 17, 2021 •

edited

Loading