aws-powertools / powertools-lambda-typescript

Powertools is a developer toolkit to implement Serverless best practices and increase developer velocity.
https://docs.powertools.aws.dev/lambda/typescript/latest/
MIT No Attribution
1.57k stars 138 forks source link

Feature request: widen types for Logger arguments #1777

Open dreamorosi opened 12 months ago

dreamorosi commented 12 months ago

Use case

The current implementation of our Logger is fairly stringent when it comes to types and objects allowed in the logs when compared with the Python implementation.

For instance, in Python it’s possible to log a wide number of objects:

from dataclasses import dataclass
from aws_lambda_powertools import Logger

logger = Logger()

@dataclass
class Stuff:
    def __init__(self, name):
        self.name = name

def main():
    logger.info("Hello world!")
    logger.info(3)
    logger.info(True)
    logger.info(Stuff("foo"))
    try:
        raise Exception("Something went wrong")
    except Exception as e:
        logger.info(e)
    logger.info({"foo": "bar"})
    logger.info(None)
    logger.info([1, 2, 3])

if __name__ == "__main__":
    main()

which produce the following logs:

{"level":"INFO","location":"main:14","message":"Hello world!","timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:15","message":3,"timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:16","message":true,"timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:17","message":"Stuff()","timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:21","message":"Something went wrong","timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:22","message":{"foo":"bar"},"timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:23","timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}
{"level":"INFO","location":"main:24","message":[1,2,3],"timestamp":"2023-10-31 10:42:27,011+0100","service":"service_undefined"}

On TypeScript instead, the current typing allows the first argument to only be a string, and the second a limited amount of types (mainly strings, objects, and errors).

A condensed version of the current types, and its limitation can be found in this TS Playground.

We should aim at expanding the type of arguments of the logger to more closely align with the Python counterpart.

Solution/User Experience

You can find the proposed types in this other TS playground, but essentially after the changes, it should be possible to log all the following:

logger.info('hi');
logger.info('hi', { bob: '' });
logger.info('hi', { bob: 1 });
logger.info('hi', { bob: true });
logger.info('hi', { bob: undefined });
logger.info('hi', { bob: BigInt(1212) });
logger.info('hi', { error: new Error('ol'), bob: 1 });
logger.info('hi', { stuff: new Error('ol') });
logger.info('error', new Error('bummer'));
logger.info(3);
logger.info(undefined);
logger.info(true);
logger.info(null);
logger.info([1, 2, 3]);
logger.info([1, 2, 3, 'a']);
logger.info({ foo: 'bar', message: 'string' });
logger.info({ foo: 'bar', message: 2 });
logger.info({ foo: 'bar' });
logger.info(new Error('hi'));
// @ts-expect-error - second argument must be an object, otherwise we don't know where to put it on the top level
logger.info('hello', 2);
// @ts-expect-error - we limit types to non serializable ones (maybe a bad idea)
logger.info('hi', { bob: new Map([[1, 2]]) });
// @ts-expect-error - you cannot override the `message` key when already specifying the first argument  
logger.info('hi', { message: 'bar' });

Alternative solutions

No response

Acknowledgment

Future readers

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

dreamorosi commented 9 months ago

We would like to hear more feedback from customers and understand how you use the logger as well as which limitations you have encountered before moving forward.

OffensiveBias-08-145 commented 5 months ago

I have not encountered a situation where a message is not provided in a log statement.

IMO, logs should tell you something and have enough context for one to understand. Having logs that simply return null or true gives the reader no context.

For cases such as the one below, I can see the usage intent. However, this then leads down a path of determining what is json serializable and where it specifically should go in each log item.

This would mean a prescribed prop would need to be used to determine where such data is to be placed in the log. This would then conflict with the current understanding of the library and any usage of a custom formatter.

// @ts-expect-error - second argument must be an object, otherwise we don't know where to put it on the top level
logger.info('hello', 2);
dreamorosi commented 4 months ago

IMO, logs should tell you something and have enough context for one to understand. Having logs that simply return null or true gives the reader no context.

That's a fair statement and the above is just a proposal for now.

I have not encountered a situation where a message is not provided in a log statement.

In production environments this is definitely the case, but especially with debug logs sometimes I have found myself just wanting to log a value and not necessarily wanting to provide a message.