Skip to content
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

Feature request: Add pure functional middleware (willing to contribute) #1833

Open
1 of 2 tasks
dbartholomae opened this issue Dec 30, 2023 · 5 comments
Open
1 of 2 tasks
Labels
discussing The issue needs to be discussed, elaborated, or refined feature-request This item refers to a feature request for an existing or new utility need-customer-feedback Requires more customers feedback before making or revisiting a decision

Comments

@dbartholomae
Copy link

Use case

Especially for small lambdas, adding middy as a dependency might not be worth it. Also, middy doesn't provide good type checking for middlewares. But manual instrumentation is still a lot of work.
Why not allow for a simple way to auto-instrument that doesn't require another library or decorators? This would Keep it lean, ease the adoption of best practices, and be progressive.

Solution/User Experience

import { Tracer, lambdaTracer } from '@aws-lambda-powertools/tracer';
// No additional library needed:
// import middy from '@middy/core';

const tracer = new Tracer({ serviceName: 'serverlessAirline' });

const lambdaHandler = async (
  _event: unknown,
  _context: unknown
): Promise<void> => {
  tracer.putAnnotation('successfulBooking', true);
};

// Use the middleware by passing the Tracer instance as a parameter, pass the handler to the middleware
// Still only one line of code.
export const handler = lambdaTracer(tracer)(lambdaHandler);

In case, multiple middlewares are needed, they can be composed like any other JavaScript function, e.g.

export const handler = lambdaTracer(tracer)(lambdaLogging(logger)(lambdaHandler));

or, with a compose utility,

export const handler = compose(
  lambdaTracer(tracer),
  lambdaLogging(logger),
)(lambdaHandler);

See https://github.com/dbartholomae/lambda-middleware for more similar middlewares.

I'm happy to provide PRs (or a joint PR if that is preferred) to extract the shared code from the middy-specific middleware and add the functional middlewares.

Alternative solutions

This just adds another, simpler way to do the things that are already possible. The alternative would be to force all users to either rely on middy as dependency, use decorators, or do their own, manual setup.

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@dbartholomae dbartholomae added triage This item has not been triaged by a maintainer, please wait feature-request This item refers to a feature request for an existing or new utility labels Dec 30, 2023
Copy link

boring-cyborg bot commented Dec 30, 2023

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

@am29d
Copy link
Contributor

am29d commented Jan 3, 2024

Hey @dbartholomae, thank you for open this feature request. This is an interesting idea to discuss and I see you have put a lot of work already into your lambda-middleware project.

I might not have enough experience when working with middy or other middlewares, so I need a bit more information on two points.

Especially for small lambdas, adding middy as a dependency might not be worth it.

Can you expand a bit on this observation what are the pros and cons one need to be aware of? What is the biggest pain point in using middy with powertools?

Also, middy doesn't provide good type checking for middlewares. But manual instrumentation is still a lot of work.

This is not clear to me, what would be the use case where our implementation would be better than middy?

Why not allow for a simple way to auto-instrument that doesn't require another library or decorators?

I agree the tenet fits here, but it remains a substantial effort to implement and maintain our own middleware and scope it properly down so it does not end up another version of middy. Do you have any key differentiators in mind that would help us draw the boundary between powertools middleware and middy?

@am29d am29d added need-customer-feedback Requires more customers feedback before making or revisiting a decision discussing The issue needs to be discussed, elaborated, or refined and removed triage This item has not been triaged by a maintainer, please wait labels Jan 3, 2024
@dbartholomae
Copy link
Author

Thanks for the quick reply!

Hey @dbartholomae, thank you for open this feature request. This is an interesting idea to discuss and I see you have put a lot of work already into your lambda-middleware project.

Thanks a lot! And just to be sure that there is no confusion: These are just examples for simple functional middleware, adding a simple functional middleware to AWS PowerTools would in no way require to know about or interact with that repo in any way.

Especially for small lambdas, adding middy as a dependency might not be worth it.

Can you expand a bit on this observation what are the pros and cons one need to be aware of? What is the biggest pain point in using middy with powertools?

Basically that it is another additional dependency. I might trust AWS PowerTools as it is maintained by AWS - but I might not want to add another dependency that could be vulnerable to any kinds of security problems. Also, it is additional, unneeded size, which can affect cold starts.
The main point for me is: There is no need to use a dedicated middleware engine in a lambda at all in many use cases, so why force users of PowerTools to install one?

Also, middy doesn't provide good type checking for middlewares. But manual instrumentation is still a lot of work.

This is not clear to me, what would be the use case where our implementation would be better than middy?

Let's say that you use a validation middleware - be it a potential future validation middleware that is part of AWS PowerTools, or just an additional middleware on top of e.g. the PowerTools tracing. With functional middleware, this could look like this:

import { z, validatonMiddleware } from '@aws-lambda-powertools/validation';
import { Tracer, lambdaTracer } from '@aws-lambda-powertools/tracer';

const tracer = new Tracer({ serviceName: 'serverlessAirline' });

const messageEventSchema = z.object({
  // Notice the accidential typo in message
  messag: z.string(),
});

function logMessage(event: { message: string }) {
  console.log(event.message);
}

// The following will error in TypeScript during compile time due to the type in the schema
export const handler = validationMiddleware(messageEventSchema)(lambdaTracer(tracer)(logMessage))

Note that the error happens even though the lambdaTracer middleware is placed between the validationMiddleware and the logMessage handler. This kind of type safety is not possible with middy, and, coincidentally, was the starting point for the lambda-middleware project.

Why not allow for a simple way to auto-instrument that doesn't require another library or decorators?

I agree the tenet fits here, but it remains a substantial effort to implement and maintain our own middleware and scope it properly down so it does not end up another version of middy. Do you have any key differentiators in mind that would help us draw the boundary between powertools middleware and middy?

To be honest, I think that you might overestimate what the middleware engine does. Let's take the tracer middleware as an example, as this seems to be the most complicated middleware in the repo so far.

The functional middleware that I would add would be basically:

export const lambdaTracer = (target: Tracer, options?: CaptureLambdaHandlerOptions) =>
  async <E, C, R> (handler: (event: E, context: C) => Promise<R>) => (event: E, context: C): Promise<R> => {
    await captureLambdaHandlerBefore(target, options);
    let response: R;
    try {
      response = await handler(event, context);
    } catch (err) {
      await captureLambdaHandlerError(target, options, error);
      throw err;
    }
    await captureLambdaHandlerAfter(target, options, response);
    return response;
}

where Tracer, CaptureLambdaHandlerOptions, captureLambdaHandlerBefore, captureLambdaHandlerAfter, and captureLambdaHandlerError are functions that would be reused inside the middy middleware (though they would need to be rewritten slightly to have a cleaner interface that is independent of the middleware layer used). So there shouldn't be much additional work needed to keep things up-to-date that isn't needed already to support middy.

@am29d
Copy link
Contributor

am29d commented Jan 15, 2024

Hey @dbartholomae ,sorry for the delayed reply.

Can you expand a bit on this observation what are the pros and cons one need to be aware of? What is the biggest pain point in using middy with powertools?

Basically that it is another additional dependency. I might trust AWS PowerTools as it is maintained by AWS - but I might not want to add another dependency that could be vulnerable to any kinds of security problems. Also, it is additional, unneeded size, which can affect cold starts. The main point for me is: There is no need to use a dedicated middleware engine in a lambda at all in many use cases, so why force users of PowerTools to install one?

The size and additional dependency management is a fair point, and that is why we carefully evaluate our dependencies and tools. To be clear, we do not force users to install middy, just as we don't force users to use decorators. We try to meet developers where they are and therefore provide multiple options, and many of them already use middy.

For developers who are not using middy this proposal could be a lightweight start to add middleware, but we would need to provide clear separation between both implementations, what to use and when.

This is not clear to me, what would be the use case where our implementation would be better than middy?

Let's say that you use a validation middleware - be it a potential future validation middleware that is part of AWS PowerTools, or just an additional middleware on top of e.g. the PowerTools tracing. With functional middleware, this could look like this:

import { z, validatonMiddleware } from '@aws-lambda-powertools/validation';
import { Tracer, lambdaTracer } from '@aws-lambda-powertools/tracer';

const tracer = new Tracer({ serviceName: 'serverlessAirline' });

const messageEventSchema = z.object({
  // Notice the accidential typo in message
  messag: z.string(),
});

function logMessage(event: { message: string }) {
  console.log(event.message);
}

// The following will error in TypeScript during compile time due to the type in the schema
export const handler = validationMiddleware(messageEventSchema)(lambdaTracer(tracer)(logMessage))

Note that the error happens even though the lambdaTracer middleware is placed between the validationMiddleware and the logMessage handler. This kind of type safety is not possible with middy, and, coincidentally, was the starting point for the lambda-middleware project.

This is indeed an interesting case to chain middlewares with type checking. I tried to replicate it with middy, and I have the same results, using z.infer.

const TestSchema = z.object({
  namse: z.string(),
  age: z.number().min(18).max(99),
});

type schema = z.infer<typeof TestSchema>;

const customMiddleware = (foo: schema): MiddlewareObj => {
  const before = (_request: MiddyLikeRequest): void => {
    console.log(foo.name);
  };

  return {
    before,
  };
};

const middyfiedHandler = middy(handler) // can be any handler, doesn't matter here
  .use(parser({ schema: TestSchema, envelope: sqsEnvelope }))
  .use(customMiddleware(TestSchema));

We get an error:

TS2551: Property name does not exist on type { namse: string; age: number; }. Did you mean namse?

To be honest, I think that you might overestimate what the middleware engine does.

Oh absolutely, I do not have extensive knowledge on the implementation details of a middleware and therefore ...

[...] So there shouldn't be much additional work needed to keep things up-to-date that isn't needed already to support middy.

... I am cautious of this assumption, especially the implications on the current implementation and the effort for future support for new features (decorator, middy, +middleware).

I would suggest to keep this PR open and collect more feedback and support from the customers, if we see "👍" on the rise we can revisit it, and work together on a contribution. We would be happy to have your opinion and insights to make a solid implementation.

@dbartholomae
Copy link
Author

I agree that maintaining something always takes more time than expected, and that it doesn't make sense to spend even a minute of maintenance effort if I'm literally the only one interested in this, so let's wait to see if there is further interest before going any further :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussing The issue needs to be discussed, elaborated, or refined feature-request This item refers to a feature request for an existing or new utility need-customer-feedback Requires more customers feedback before making or revisiting a decision
Projects
Development

No branches or pull requests

2 participants