chore(tracer): cdk examples + e2e tests #347

dreamorosi · 2021-12-23T10:24:36Z

Description of your changes

Extracted cdk resources to create NodejsFunction & invoke it in its own cdk Construct to avoid repetition
Added cdk examples for Tracer
Updated examples & docs to use correct case (camelCase) & fixed captureLambdaHandler references
Added e2e tests in cdk

How to verify this change

Pull & checkout the branch, then cdk deploy the stack & review the resources & traces created.

Related issues, RFCs

#332

PR status

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

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.

packages/tracing/tests/e2e/tracer.test.ts

flochaz · 2021-12-27T09:46:57Z

For examples, wondering if we shouldn't have more realistic tests, reading at them first made me thought they were e2e tests. I think we should have a more concrete use case using most of the feature of tracer to show the potential (and maybe 3 variations of them : decorator/manual/middleware). But not that much a per feature example ? just wondering ...

dreamorosi · 2021-12-27T18:11:08Z

For examples, wondering if we shouldn't have more realistic tests, reading at them first made me thought they were e2e tests. I think we should have a more concrete use case using most of the feature of tracer to show the potential (and maybe 3 variations of them : decorator/manual/middleware). But not that much a per feature example ? just wondering ...

Happy to discuss this offline but please take a look at the e2e tests I've implemented and let me know if that's more similar to what you mean.

flochaz · 2021-12-27T20:38:33Z

For examples, wondering if we shouldn't have more realistic tests, reading at them first made me thought they were e2e tests. I think we should have a more concrete use case using most of the feature of tracer to show the potential (and maybe 3 variations of them : decorator/manual/middleware). But not that much a per feature example ? just wondering ...

Happy to discuss this offline but please take a look at the e2e tests I've implemented and let me know if that's more similar to what you mean.

e2e are great ! was just wondering if examples were not too redundant . but fine for now. will open a discussion

saragerion · 2021-12-28T11:09:43Z

docs/core/tracer.md

+Tracer has three global settings that can be used to configure its behavior:
+
+Setting | Description | Environment variable | Constructor parameter
+------------------------------------------------- | ------------------------------------------------- | ------------------------------------------------- | -------------------------------------------------
+**Trace Enabled** | Explicitly enables/disables tracing | `POWERTOOLS_TRACE_ENABLED` | `enabled`
+**Capture Responses** | Captures Lambda or method return as metadata |  `POWERTOOLS_TRACER_CAPTURE_RESPONSE` | N/A
+**Capture Errors** | Captures Lambda or method exception as metadata |  `POWERTOOLS_TRACER_CAPTURE_ERROR` | N/A
+


So this bit has been changed in main, getting started section + settings. How will it look like once you merge?

I will resolve the conflict & accept the version on main since I made this change several days ago before your PR was merged.

saragerion · 2021-12-28T11:13:08Z

examples/cdk/lib/example-function.Tracer.CaptureErrorDisabled.ts

+import { captureLambdaHandler, Tracer } from '@aws-lambda-powertools/tracer';
+
+// Set environment variable to disable capture response
+process.env.POWERTOOLS_TRACER_ERROR_RESPONSE = 'false';


Minor: what about provisioning env variables in CDK? I think this is what developers would do in prod
https://docs.aws.amazon.com/cdk/api/v1/docs/@aws-cdk_aws-lambda.Function.html#environment

BTW happy to address this as part of the prod release milestone, in case you also agree

I initially declared them this way because of how I am creating the functions in the test setup (see here) but you are right, Customers would do it as you suggest so in the latest commit (7caeb28) I have included this in the test setup & also removed some redundant files.

saragerion · 2021-12-28T11:14:55Z

examples/cdk/lib/example-function.Tracer.CaptureResponseDisabled.ts

+import { captureLambdaHandler, Tracer } from '@aws-lambda-powertools/tracer';
+
+// Set environment variable to disable capture response
+process.env.POWERTOOLS_TRACER_CAPTURE_RESPONSE = 'false';


Same as here: https://github.com/awslabs/aws-lambda-powertools-typescript/pull/347/files#r775854496

Addressed in 7caeb28, see this comment.

saragerion · 2021-12-28T11:24:21Z

examples/cdk/package.json

+    "@aws-lambda-powertools/logger": "^0.2.0-beta.6",
+    "@aws-lambda-powertools/metrics": "^0.2.0-beta.6",
+    "@aws-lambda-powertools/tracer": "^0.2.0-beta.6",
+    "@aws-sdk/client-sts": "^3.43.0",


Based on the semantic versioning annotation used in this package.json, every time we have a minor or major breaking change like this:
X.Z.X or Z.X.X
We will have to manually update these dependencies numbers. Can we make them more loose?

Relaxed to get all beta versions (1320cec), my reasoning is that currently we might be bumping up version quite often and might want to include / have reflected these changes in the examples; once we exit beta we'll lock examples only to major releases or similar..

saragerion

Left some minor comments, but looks good already!

dreamorosi added 4 commits December 22, 2021 18:16

Simplified base CDK examples

e0e6e9e

Added Tracer examples

d7381b3

Merge branch 'main' into chore/tracer_examples

7ffa71e

Added back LambdaInterface in Tracer.test

98b702f

dreamorosi added documentation Improvements or additions to documentation tracer This item relates to the Tracer Utility labels Dec 23, 2021

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

dreamorosi self-assigned this Dec 23, 2021

dreamorosi added 8 commits December 23, 2021 11:28

Updated package.json

5e442b0

Updates to examples & docs

630b56d

Updated docs example

7d633cf

Updated examples + added unit test groups

dadf44e

Added base e2e tests

45f7b60

Added back folder to jest config

ac8e578

Added disabled tracer e2e test

682d62c

Added patch AWS SDK clients to manual e2e test

9a4fc51

flochaz suggested changes Dec 27, 2021

View reviewed changes

packages/tracing/tests/e2e/tracer.test.ts Outdated Show resolved Hide resolved

packages/tracing/tests/e2e/tracer.test.ts Outdated Show resolved Hide resolved

dreamorosi added 2 commits December 27, 2021 14:31

Reduced test timeouts

196db23

Added captureAWS feature to e2e

640a431

dreamorosi changed the title ~~chore(tracer): cdk examples~~ chore(tracer): cdk examples + e2e tests Dec 27, 2021

Added additional e2e test cases

5349347

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

dreamorosi marked this pull request as ready for review December 27, 2021 18:12

flochaz previously approved these changes Dec 27, 2021

View reviewed changes

saragerion reviewed Dec 28, 2021

View reviewed changes

saragerion previously approved these changes Dec 28, 2021

View reviewed changes

Removed redundant e2e test files

7caeb28

dreamorosi dismissed stale reviews from saragerion and flochaz via 7caeb28 December 28, 2021 13:03

Relaxed dependency version on cdk examples

1320cec

dreamorosi requested a review from saragerion December 28, 2021 13:34

saragerion previously approved these changes Dec 28, 2021

View reviewed changes

ijemmy previously approved these changes Dec 28, 2021

View reviewed changes

Resolving merge conflicts

92e4ee3

dreamorosi dismissed stale reviews from ijemmy and saragerion via 92e4ee3 December 28, 2021 15:01

Resolving merge conflicts

93c19e3

dreamorosi merged commit b69cfdd into main Dec 28, 2021

dreamorosi deleted the chore/tracer_examples branch December 28, 2021 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(tracer): cdk examples + e2e tests #347

chore(tracer): cdk examples + e2e tests #347

dreamorosi commented Dec 23, 2021 •

edited

Loading

flochaz commented Dec 27, 2021

dreamorosi commented Dec 27, 2021 •

edited

Loading

flochaz commented Dec 27, 2021

saragerion Dec 28, 2021

dreamorosi Dec 28, 2021

saragerion Dec 28, 2021

saragerion Dec 28, 2021

dreamorosi Dec 28, 2021

saragerion Dec 28, 2021

dreamorosi Dec 28, 2021

saragerion Dec 28, 2021 •

edited

Loading

dreamorosi Dec 28, 2021

saragerion left a comment

chore(tracer): cdk examples + e2e tests #347

chore(tracer): cdk examples + e2e tests #347

Conversation

dreamorosi commented Dec 23, 2021 • edited Loading

Description of your changes

How to verify this change

Related issues, RFCs

PR status

Checklist

Breaking change checklist

flochaz commented Dec 27, 2021

dreamorosi commented Dec 27, 2021 • edited Loading

flochaz commented Dec 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saragerion Dec 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saragerion left a comment

Choose a reason for hiding this comment

dreamorosi commented Dec 23, 2021 •

edited

Loading

dreamorosi commented Dec 27, 2021 •

edited

Loading

saragerion Dec 28, 2021 •

edited

Loading