fix: updated CDK examples to remove old references & improve comments #439
Conversation
dreamorosi
added this to Pull Requests - Work in progress
in Pull Requests
via automation
dreamorosi
moved this from Pull Requests - Work in progress
to Pull Requests - Review needed
in Pull Requests
| try { | ||
| throw new Error('test'); | ||
| // Add the response as metadata | ||
| res = { foo: 'bar' }; | ||
| } catch (err) { |
There was a problem hiding this comment.
Same as above. Why try/catch without throw?
There was a problem hiding this comment.
See this comment.
There was a problem hiding this comment.
This one has the throw at L57.
examples/cdk/lib/example-function.Tracer.CaptureErrorDisabled.ts
Outdated
Show resolved
Hide resolved
| const sts = tracer.captureAWSClient(new STS()); | ||
|
|
||
| // Here we are showing an example with manual instrumentation but you can do the same also with the captureLambdaHandler Middy Middleware and Class decorator |
There was a problem hiding this comment.
(minor) I am not aware of this. O.o
AFAI see, this is not mentioned in the documentation. Can we add it to the list in this section . Another PR is fine.
There was a problem hiding this comment.
The actual example, in this file, is from L4 to L11 since this example wants to demonstrate how to instrument an AWS SDK v2 client.
What I wanted to say with this comment is that even though this example uses manual instrumentation, you don't need to use this method specifically over others (Middy middleware, decorator). Essentially, as long as you instrument the AWS SDK client with tracer.capture* it'll be traced & appear as custom subsegment on X-Ray no matter how you write your function.
That's why in the docs the AWS SDK patching example only has these lines.
I included a function in the example just because otherwise the lines above alone wouldn't run as a Lambda function and the lines from L15 onwards are similar to manual method in the section you linked.
…bled.ts Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>
Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>
…rtools-typescript into fix/cdk_examples
|
As CDK V2 is GA, would it make sense to just start using it from the get go @dreamorosi ? |
Would love to, but at the moment we are relying on some classes that are not exported in v2, see this issue I've opened on the CDK repo. |
Description of your changes
This PR sets up #425 and aims at:
How to verify this change
After #416 has been merged,checkout the branch, go toexamples/cdk& runcdk deploy.Related issues, RFCs
#435
#425
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.