search

alexcasalboni / aws-lambda-power-tuning

AWS Lambda Power Tuning is an open-source tool that can help you visualize and fine-tune the memory/power configuration of Lambda functions. It runs in your own AWS account - powered by AWS Step Functions - and it supports three optimization strategies: cost, speed, and balanced.
Apache License 2.0
5.27k stars 362 forks source link

Actual Payload Shared in Step Functions Console On Function Error #212

Closed rrhodes closed 10 months ago

rrhodes commented 11 months ago

Problem

I want to use AWS Lambda Power Tuning to invoke Lambda functions with an input payload that contains credentials. I do not want this information in the AWS Step Functions execution history.

Initially I explored the use of a pre-processor to workaround this by allowing the pre-processor to fetch credentials and pass those direct to the Lambda being tuned. This I understand would all execute within a single state of the Step Functions, so credentials would not be shared in the Step Functions console.

However, if the Lambda being tuned throws an error, we log the payload, which in my case would include the credentials.

Proposed Solutions

Two ideas spring to mind, but I'm open to discussing the problem further and open to other suggestions how to solve this:

  1. we stop including the payload in the error message thrown. The payload I think should be the same for each execution of the Step Functions, unless we use a pre-processor which returns different output given the same input. So is logging the payload valuable?

  2. we introduce a new toggle to enable / disable inclusion of the payload in the error thrown, defaulting to showing the payload to maintain consistent behaviour, but this would allow users like me to disable it if desired.

Contributing

I'm happy to contribute to this project with any agreed changes.

alexcasalboni commented 11 months ago

Hi @rrhodes, thanks for reaching out!

Have you considered using the payloadS3 input parameter? That allows you to provide an S3 bucket/key that contains the input payload (instead of providing it directly to Step Functions).

This should solve the more common case (no error).

In case of errors, I'd suggest to just avoid dumping the credentials into your logs (or somehow encrypt it before dumping it).

alexcasalboni commented 11 months ago

Apologies, I didn't fully understand the issue at first :)

Your're correct, we should also add a toggle for the executor function to avoid logging the invocation payload.

Happy to accept a PR 🚀

rrhodes commented 11 months ago

Thanks for the prompt reply @alexcasalboni! I'll take a look into this and try to have a PR up next week for review. 🙂

alexcasalboni commented 11 months ago

@rrhodes would it make more sense as an execution parameter (at runtime) or as a CloudFormation parameter (at deploy time)? I don't have a strong opinion, but it sounds like it would be more flexible as an execution parameter, so you can use the same deployment for both use cases.

rrhodes commented 11 months ago

it sounds like it would be more flexible as an execution parameter, so you can use the same deployment for both use cases.

An execution parameter I agree would make more sense, yep. 👍🏻

alexcasalboni commented 11 months ago

Super 😄

Based on the naming conventions used so far, I'd imagine a parameter such as disablePayloadLogs (boolean) or something similar.

And please note that the same happens here (series invocations) and here (pre/post-processor invocations).

rrhodes commented 11 months ago

Thanks @alexcasalboni! The name sounds good to me, and thanks for the head's up on the other areas to review. 😊

rrhodes commented 10 months ago

Hey @alexcasalboni, that should be PR #213 ready for review implementing the agreed approach. Feedback welcome!