feat(logger): introduce log key reordering functionality

[arnabrahman]

Summary

By default, we can not change the log key positions. This PR gives the user the ability to change log key positions. We can change log order positions for any keys added in runtime

Changes

• Mostly followed the recommendations from the comment from the actual issue.
• A new option logRecordOrder is added for reordering.
• Change formatAttributes function for the new option.
• Write unit tests for this.
• Necessary documentation has been added with examples

Issue number: #1568

---

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[dreamorosi]

Hi @arnabrahman, thank you for opening this PR!

I'll be reviewing it this morning and will get back to you, but overall I think we're going in the right direction.

[dreamorosi]

i believe we should type the option for standard structured keys

I was under the same impression, but looking at the docs again I think we're mistaken, since the docs mention this:

You can change the order of standard Logger keys or any keys that will be appended later at runtime

Additionally, in the example they show usage of a key (request_id) that is not part of the standard keys and that is added at runtime.

With this in mind I think we should follow the same pattern, if possible.

I haven't tested this in the code, but at first glance if I remember correctly, I think we should be able to include all keys to the new logic by working with the additionalLogAttributes (of type LogAttributes) object.

Doing this will mean we probably remove this line, which I'm not sure if it'll have any impact on the log output.

powertools-lambda-typescript/packages/logger/src/formatter/PowertoolsLogFormatter.ts

Line 39 in d76eb1d

powertoolsLogItem.addAttributes(additionalLogAttributes);

If this is a viable option, we should also modify the types of the new logRecordOrder accordingly (maybe an union between the keys of UnformattedAttributes and LogAttributes?).

---

Let me know if I cleared your questions 😄

[arnabrahman]

This summer felt like an eternity but good to be back.

I was looking at your suggestions @dreamorosi, on a high level it should be possible to include all the additional keys although I have to look a bit more to validate this. But first I have to merge the main branch but I cannot. I am getting pre-commit error for markdownlint-cli2 in packages/tracer/README.md. I ran setup-local & tried again but still got the same result. Is there anything that needs to be done?

[dreamorosi]

Hi @arnabrahman, glad to hear from you again! I hope you had a great summer and some well deserved rest 😃

The errors in the screenshot are strange because the changelog file with issue should be ignored from the markdown lint (here).

I will look into it as I have a rough idea of why this is happening, but for now I think it's safe to run the commit with the --no-verify flag, so you can skip the local checks for this one time. Either way they'll run again here in the PR. Sorry for the confusion!

[arnabrahman]

@dreamorosi So i have progressed well on this. But I have another question. Right now the logRecordOrder passes down to child logger. What if we want to override logRecordOrder to child instance?

Currently while creating child, logFormatter is passed to child instance. https://github.com/aws-powertools/powertools-lambda-typescript/blob/main/packages/logger/src/Logger.ts#L308

And logFormatter is not reinitialized if it's already set https://github.com/aws-powertools/powertools-lambda-typescript/blob/main/packages/logger/src/Logger.ts#L1085

In this case, if we provide logRecordOrder to child, it will not be overridden. Correct me if I am wrong

[dreamorosi]

Hi @arnabrahman, thank you for the PR - I've started reviewing it and should be able to finish by tomorrow morning at the latest.

[dreamorosi]

Hi @arnabrahman, thank you so much for the patience while I reviewed this and also for the great work!

I think we are getting very closer, I've left a few comments but the main implementation is done I think.

Let me know if you have any questions or disagree with the comments.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[dreamorosi]

Thank you @arnabrahman for another great addition and contribution.

I really appreciate you coming back to this and working on the various iterations until we got here.

Thank you again!

PS: I'll reply to your comment on the other issue later today 😉