Skip to content

feat(tracer): middy middleware #324

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

Merged
merged 6 commits into from
Dec 17, 2021
Merged

feat(tracer): middy middleware #324

merged 6 commits into from
Dec 17, 2021

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Dec 15, 2021

Description of your changes

  • Implement middy-based middleware with feature parity against captureLambdaHandler decorator
  • Implement middy-based middleware with feature parity against captureMethod decorator
  • Documentation & JSDocs updates
  • Implementation unit tests

How to verify this change

Checkout branch & run npm run tests in src/packages/tracing and/or observe actions results.

Related issues, RFCs

#295
#282

PR status

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

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

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 dreamorosi added enhancement tracer This item relates to the Tracer Utility labels Dec 15, 2021
@dreamorosi dreamorosi added this to the beta-release milestone Dec 15, 2021
@dreamorosi dreamorosi self-assigned this Dec 15, 2021
@dreamorosi
Copy link
Contributor Author

I couldn't find any mention about implementing middy middlewares to generic functions that are not a Lambda Handler.
To some extent middy should work also on regular functions since it's a function closure, but the lack of typing & examples require further exploration that should be its own unit of work.

At the moment I've added a notice to the docs (see below) with a link to open an feature request, if Customers want this feature we'll reconsider in the future:
image

@dreamorosi dreamorosi marked this pull request as ready for review December 15, 2021 19:14
@@ -107,8 +106,7 @@ You can quickly start by importing the `Tracer` class, initialize it outside the
}
```

<!-- TODO: Replace name of middleware once implemented -->
When using thes `captureLambdaHanlder` decorator or the `TBD` middleware, Tracer performs these additional tasks to ease operations:
When using the `captureLambdaHanlder` decorator or middleware, Tracer performs these additional tasks to ease operations:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor fix:

Suggested change
When using the `captureLambdaHanlder` decorator or middleware, Tracer performs these additional tasks to ease operations:
When using the `captureLambdaHandler` decorator or middleware, Tracer performs these additional tasks to ease operations:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

}
```
!!! info
We currently support a middleware for tracing methods, [let us know](https://github.com/awslabs/aws-lambda-powertools-typecsript/issues/new?assignees=&labels=feature-request%2C+triage&template=feature_request.md&title=) if you'd like to see one!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor fix:

Suggested change
We currently support a middleware for tracing methods, [let us know](https://github.com/awslabs/aws-lambda-powertools-typecsript/issues/new?assignees=&labels=feature-request%2C+triage&template=feature_request.md&title=) if you'd like to see one!
We currently support a middleware for tracing methods, [let us know](https://github.com/awslabs/aws-lambda-powertools-typescript/issues/new?assignees=&labels=feature-request%2C+triage&template=feature_request.md&title=) if you'd like to see one!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this!

saragerion
saragerion previously approved these changes Dec 17, 2021
Copy link
Contributor

@saragerion saragerion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some minor comments

@dreamorosi dreamorosi merged commit 2909d6f into main Dec 17, 2021
@dreamorosi dreamorosi deleted the feat/tracer-middleware branch December 17, 2021 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracer This item relates to the Tracer Utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants