Feature Request: support logging Set & Map types

[hfahlbusch]

Expected Behaviour

If I enter a Set as a value of the LogItemExtraInput it should print it's elements like the console.log() and not return {}.
I was thinking there were no data in the set and searched a while for the error.

Current Behaviour

Set with elements will be printed as {}

Code snippet

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

const log = new Logger({
logLevel: 'DEBUG'
});
const a = new Set();
a.add(1);
a.add(2);

log.debug('My set not logged properly', { data: a});

//{"level":"DEBUG","message":"My set not logged properly","service":"service_undefined","timestamp":"2023-08-12T14:27:00.205Z","data":{}}

console.log('My set logged', a);
// My set logged Set(2) { 1, 2 }

Steps to Reproduce

1. Create new node project
2. Install powertools logger in node modules
3. Copy code snippet from above
4. Run and see missing Set data

Possible Solution

I digged a little into the code.
It seems the extra input is stringified with JSON.stringify which cannot handle Sets in the expected way, because the values are not properties of the Set.

Could be added to the getReplacer() in Logger.js that also handles bigints.
Maybe convert Set to an array [...mySet] or create string representation manually.

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

18.x

Packaging format used

npm

Execution logs

Example in the logs:

{
    "logLevel": "DEBUG",
    "message": "Modified products and documents",
    "awsRegion": "eu-central-1",
    "service": "cache-invalidation",
    "timestamp": "2023-08-12T13:43:30.197Z",
    "correlationIds": {
        "xRayTraceId": "xxxxxxxxxxxxxxxxxxxxxxxx"
    },
    "durationInMs": 1291,
    "changedAfter": "2023-08-09T22:59:56.820Z",
    "numberOfProducts": 4,
    "numberOfDocuments": 5,
    "productIds": [
        [
            "PROD1",
            "sv-se"
        ],
        [
            "PROD2",
            "no-no"
        ],
        [
            "PROD3",
            "da-dk"
        ],
        [
            "PROD4",
            "no-no"
        ]
    ],
    "documentIds": {}
}

[boring-cyborg]

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

[dreamorosi]

Hi @hfahlbusch thank you for taking the time to open this issue.

Before emitting a log the Logger utility calls JSON.stringify() on the log object.

Like you mentioned Set, as well as Map and others complex types, is not natively JSON-serializable (docs) so the behavior is not a bug but a missing feature.

I think it could be a good addition to the library so I'm going to change the issue type and mark it as feature request.

If there's interest we could definitely add support for those types after agreeing on a format to use.

[hfahlbusch]

Yeah, you are right!
I had a different mental model in my head, I was assuming it behaves more like the console.log not knowing that it's basically a JSON.stringify(). So that's why I framed it as a bug.

I guess with the advent of Sets and Maps it would be a good addition. In my opinion it brings some more comfort.
But for me even the confusion was causing more trouble, than "just transforming the Set to an array" to see it's values.

I would suggest to convert Set to Array and Map to Object, so it stays valid JSON or are you thinking about "arbitrary" strings to make them distinguishable?
(Maybe easy processing of logs as input for another tool could be also a factor here.)

[dreamorosi]

hi @hfahlbusch apologies for the delayed response.

The suggestion to convert Set to Array and Map to Object seems sensible, I wonder however if there's any edge case that we should consider or avoid, especially with Map -> Object. From the top of my head one of the main differences between Map and plain objects is that the former can have many more types as key rather than just string like objects.

I think we can approximate and cast most of them to strings, i.e. a Map that has numbers as key is relatively safe to cast to an object that has string representations of these numbers. However for more complex types like Symbols we might not be able to do this.

I'm putting the issue on the backlog and including it in the scope for the next upcoming major version. I've also edited the type to explicitly include the Map type.

I'd like to also open the feature for other contributors. If anyone is interested in picking this up, please leave a comment below and let us know if you have any question. If everything is clear, please still leave a comment to let us know you're starting to work on this so we avoid duplicating effort, and if possible also share your comments on the implementation that you'd like to carry out.

[hfahlbusch]

Hi @dreamorosi,

maybe I could implement this as a first contribution.

Do you have any additional comments to the planned serialization?
What do you think?

[dreamorosi]

Hi @hfahlbusch thank you for offering your contribution, we'd love to have your contribution!

As for additional comments, the only thing to call out would be to start your branch from feat/v2 rather than from the main branch, since we'll merge the contribution into the former rather than on the v1 branch.

Other than that I would maybe recommend researching what would be the most appropriate way of coercing Set and Map types to JSON serializable values. As I mentioned before I think converting Set to an array should work for average cases but Map types might be trickier due to the fact that they can have many different types other than string as keys.

I'll assign the issue to you and mark it as work in progress, we can continue the conversation on this last point either in the issue or in the PR if you prefer.

If you have any questions on how to setup the dev environment or any other aspect of the project, please let me know - I'll be happy to support.

[dreamorosi]

Hi @hfahlbusch, just wanted to check in and see if there's anything that I can help with or if you have any questions 😃

[dreamorosi]

Hello,

We have decided to close this feature in favor of a more powerful implementation that will allow customers to provide their own JSON replacer function. You can find more details in the dedicated issue: #1776

[github-actions]

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues 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.