chore(tracer): quality of life improvements#337
Conversation
|
I revisited the implementation of the Tracer middleware because while testing it on an actual Lambda function I was running in an issue similar to the one described here. The integration and API haven't changed but the implementation now correctly handles the segments, the key was in calling |
ijemmy
left a comment
There was a problem hiding this comment.
LGTM overall. I have only a few nits for you to double check.
|
|
||
| const open = (): void => { | ||
| lambdaSegment = target.getSegment(); | ||
| const handlerSegment = lambdaSegment.addNewSubsegment(`## ${process.env._HANDLER}`); |
There was a problem hiding this comment.
The python tracer library is opinionated on the syntax of the subsegments' labels (as it is with the format of the structure logs, for example).
I say that for consistency and feature parity we should leave it as it is, but allow customers to control the annotation labels if they want to in the future (but this we can do after the beta).
saragerion
left a comment
There was a problem hiding this comment.
Added a couple of comments (minor). Thanks @dreamorosi! 🎉
saragerion
left a comment
There was a problem hiding this comment.
Looks great, I just left a minor nit (I would find/replace in the codebase for captureLambdaHanlder), but for me it's already good to merge :chefskiss:
Co-authored-by: Sara Gerion <[email protected]>
Co-authored-by: Sara Gerion <[email protected]>
Co-authored-by: Sara Gerion <[email protected]>
Co-authored-by: Sara Gerion <[email protected]>
Description of your changes
This PR includes the following changes, all related to the
Tracermodule:@aws-lambda-powertools-typescript/commonspackage in tests (examples will be updated in separate PR related to Maintenance: delete examples in packages folder #332).README.mdfile of the package to include only intro + link to main docs as discussed in standup.tracer.addResponseAsMetadataas public API, changed middleware to use it, updated tests & docs (Feature request: feature parity for manual instrumentation tracer #334).tracer.addErrorAsMetadataas public API, changed middleware to use it, updated tests & docs (Feature request: feature parity for manual instrumentation tracer #334).tracer.annotateColdStartas public API, changed middleware to use it, updated tests & docs (Feature request: feature parity for manual instrumentation tracer #334).tracer.annotateColdStartso that annotation is always added to main segment even when it's not a cold start, this is a change mentioned here Feature request: tracer feature improvements #275.tracer.addServiceNameAnnotationto allow Customers to add the annotation to the main segment, changed middleware & decorator to use it, updated tests & docs; this is a change mentioned here Feature request: tracer feature improvements #275.How to verify this change
Verify that all tests are passing & coverage hasn't decreased.
Related issues, RFCs
#332
#334
#275
#851 (Python)
#861 (Python)
PR status
Is this ready for review?: YES
Is it a breaking change?: YES
Checklist
Breaking change checklist
This is a breaking change in the sense that generated traces will now all have
ColdStart(sometimestrue, sometimesfalse) andServiceNameannotations.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.