powertools-lambda-typescript icon indicating copy to clipboard operation
powertools-lambda-typescript copied to clipboard

Feature request: Support Logging Incoming Event Without Use of Middy or Class

Open bestickley opened this issue 1 year ago • 4 comments

Use case

I want the incoming log event to be automatically logged for me when POWERTOOLS_LOGGER_LOG_EVENT is set to true and I'm not using middy or a handler class. Middy and handler class usage is demonstrated in docs here.

Solution/User Experience

Conditionally log incoming event based on POWERTOOLS_LOGGER_LOG_EVENT within Logger.addContext. I know this may seem strange to put "log incoming event logic" into the addContext method, but that's how it's implemented with middy (injectLambdaContext) and class decorator (logger.injectLambdaContext).

With this solution, the below function would log incoming event. Currently it does not.

import { Logger } from "@aws-lambda-powertools/logger";
process.env.POWERTOOLS_LOGGER_LOG_EVENT = "true";
const logger = new Logger();
export function handler(event, context) {
  logger.addContext(context);
  // do work
}

Alternative solutions

No response

Acknowledgment

Future readers

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

bestickley avatar Jan 12 '24 14:01 bestickley

Hi @bestickley , thanks for the feature request. This is indeed a missing feature for customer who are not using middy or decorator. I checked the implementation and from the first impression it looks like we could move event logging into the addContext method. injectLambdaContext calls addContext before logging the event.

I think the tricky part is to keep the call clean as you did your example logger.addContext(contex). In this case, we have no chance to grab the event object without explicitly passing it to the function. So it is most likely to have the signature logger.addContext(context, event) which looks weird because the parameters are reversed.

The feature request is valid and will bring value, we just need to agree on the implementation details.

am29d avatar Jan 15 '24 06:01 am29d

@am29d, I agree logger.addContext(context, event) is a little weird but I'm not opposed. You could make event optional as second argument. Alternatively you could overload the function to allow for logger.addContext({ context, event }). I don't have a strong preference.

bestickley avatar Jan 15 '24 11:01 bestickley

It's true that we have overloaded the meaning of the injectLambdaContext decorator and Middy middleware making it do multiple things, this was done for two reasons: 1/ injecting the context in each log entry is the main feature, and 2/ consistency with other Powertools versions (mainly Python).

The overloading came also from a place of necessity. Both the Middy middleware and decorator wrap the handler and have access to its arguments (aka event and context), so adding other features made sense versus adding a different decorator or middleware for each minor feature.

In this case the logger.addContext() method both in name and implementation is exclusively dedicated to inject the context so in my opinion adding a second unrelated argument is not the way to go.

Starting from the assumption that you can already log the event by just calling logger.info('event', event); anywhere in your function scope, the real value of this feature (and potentially the root cause of the request) revolves around the ability to control whether the event is logged or not using the POWERTOOLS_LOGGER_LOG_EVENT environment variable without having to modify your code.

If this is true I would be more inclined to go in a different direction:

  • short-term: add a dedicated logger.logEvent(event); method which emits the log based on the value of POWERTOOLS_LOGGER_LOG_EVENT
  • medium-term: come up with a new high-level equivalent for the decorator and Middy middleware that doesn't require you to use decorators (which are experimental and require TS as well as to opt in to a specific way of structuring your code) nor Middy middleware (which requires you to add and trust an external dependency) - this is a discussion that has come up other times with the most recent being in #1833

dreamorosi avatar Jan 15 '24 17:01 dreamorosi

I agree on having a separate method logEvent. I have seen many customers trying various ways to log an event, including JSON.stringify() inside logger.info.

am29d avatar Jan 16 '24 16:01 am29d

Hi both, while doing some maintenance on the Logger I noticed that we already have a mostly undocumented public method called logEventIfEnabled() that does exactly what we are proposing in one of the comments above.

The method would be used like this:

import { Logger } from '@aws-lambda-powertools/logger';

const logger = new Logger();

export const handler = async (event: unknown) => {
  logger.logEventIfEnabled(event); // (1)
  // ... your handler code
};

It takes in account the value of POWERTOOLS_LOG_EVENT and besides that, behaves the same as the usage with Middy.js middleware or class method decorator.

For now I have opened a PR to add documentation and examples for this method and will be closing the issue once the PR is merged, since the first low hanging fruit has been addressed.

In regards to a better and less verbose DX for users who don't want to use Middy.js nor class method decorators, we can continue the discussion in #1833.

dreamorosi avatar Aug 14 '24 10:08 dreamorosi

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Aug 16 '24 10:08 github-actions[bot]