fix(logger): fix handling of additional log keys

[ghost]

Description of your changes

Logger accepts a second parameter that allows to append additional keys and values to a single log item.
Currently, there are 5 use cases that are supported for this:

// 1) object
logger.info('This is the message value', { extra: 'parameter' });

// 2) object, object, ...
logger.info('This is the message value', { parameterOne: 'foo' }, { parameterTwo: 'bar' });

// 3) Object (that contains the message key)
logger.info( { message: 'This is the message value', extra: 'parameter' });

// 4) error
logger.info('This is the message value', new Error('Something happened!') );

// 5) error with custom key
logger.info('This is the message value', { myCustomErrorKey: new Error('Something happened!') });

However, the current implementation allows passing simple strings, arrays, and any other unknown objects (single or multiple) as a second (third, fourth, ...) parameter, causing unwanted side effects.

This PR addresses this issue by aligning the implementation to other AWS Lambda Powertools flavors like e.g. Python.
So, one more use cases is allowed:

// 6) simple string
logger.info('This is the message value', 'Additional string value');

Additionally:

• passing multiple objects is restricted to only { key: 'value' } (key-value) kind of ones
• in general, passing arrays and other unknown objects is not supported anymore (BREAKING)
• passing multiple strings and Errors is not supported anymore (BREAKING)

How to verify this change

See the unit tests.
Check the updated documentation (section "Appending additional log keys and values to a single log item").
Try to pass unsupported objects, e.g.:

logger.info('foo', 'bar', 'baz');
logger.info('foo', ['bar', 'baz']);

Related issues, RFCs

#565

PR status

Is this ready for review?: YES
Is it a breaking change?: YES

Checklist

• My changes meet the tenets criteria
• I have performed a self-review of my own code
• I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding changes to the examples
• My changes generate no new warnings
• The code coverage hasn't decreased
• I have added tests that prove my change is effective and works
• New and existing unit tests pass locally and in Github Actions
• Any dependent changes have been merged and published in downstream module
• The PR title follows the conventional commit semantics

Breaking change checklist

The TypeScript type system / compiler will produce errors for any unsupported API usage.
Users will have to fix those errors by converting any unsupported objects into strings or supported key-value objects.

• I have documented the migration process
• I have added, implemented necessary warnings (if it can live side by side)

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[dreamorosi]

Hi @AWSDB thanks for the PR, have set aside some time to review it later today.

[dreamorosi]

Looks good to me, thanks a ton for taking up this PR

[ijemmy]

LGTM.

[ijemmy]

@dreamorosi We cannot run E2E on PR from a forked repo. This is another gap in our process :(

I ran gh pr checkout 614 and git push origin so we have this branch in this project.

Here's the run: https://github.com/awslabs/aws-lambda-powertools-typescript/runs/5464360763?check_suite_focus=true

Will merge once it has passed.

[dreamorosi]

@dreamorosi We cannot run E2E on PR from a forked repo. This is another gap in our process :(

I ran gh pr checkout 614 and git push origin so we have this branch in this project.

Here's the run: https://github.com/awslabs/aws-lambda-powertools-typescript/runs/5464360763?check_suite_focus=true

Will merge once it has passed.

Yep, coincidentally we discussed this earlier this morning during the usual meeting. @flochaz is going to look at it in the next days to see if we can make the e2e tests run for this type of contributions.

Thanks for the review btw!

e2e tests are not passing

[dreamorosi]

@AWSDB it seems that the integration tests that @ijemmy just ran are not passing, could you please look at it? Will approve again & merge once they are green

[ijemmy]

Reran e2e result passes : https://github.com/awslabs/aws-lambda-powertools-typescript/runs/5466416155?check_suite_focus=true

@dreamorosi Seem like a bad version in package-lock.json. Rebasing to the latest main solves the issue. Can we have 2nd review?

[ghost]

@dreamorosi As @ijemmy pointed out, there seemed to be an issue with the main branch commit I based my branch initially on. After rebasing the branch onto the latest main state, the missing dependency issue doesn't occur anymore.

[dreamorosi]

Thanks both for following up on this. Unfortunately I have made a release just a couple of hours ago so this change will be included in the next one.

Merged.