feat(logger): pretty-print JSON when POWERTOOLS_DEV is set

[pmarko1711]

Issue number: #1527

Summary

Changes

• Logger's default serialize function json.dumps will use indentation by four spaces when in AWS SAM Local. Detection happens using AWS_SAM_LOCAL env var (if =="true").

User experience

There should be no impact unless in AWS SAM Local.

In AWS SAM Local, logs are output to console. To make the serialized json messages more readable and consequently debugging with AWS SAM Local easier, logger's default formatter will in this case automatically use indentation by four spaces. That means that messages normally serialized as

{"level": "INFO", "location": "lambda_handler:39", "message": "Some log message...", "timestamp": "2022-09-26 09:03:38,349+0000", "service": "issue-1527"}

will be in AWS SAM Local by default indented by four spaces:

{
    "level": "INFO",
    "location": "lambda_handler:39",
    "message": "Some log message...",
    "timestamp": "2022-09-26 09:02:05,262+0000",
    "service": "issue-1527"
}
This is to make debugging easier

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
  • (only using pytest)
• Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

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.

---

View rendered docs/core/logger.md

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our AWS Lambda Powertools Discord: Invite link

[heitorlessa]

Thank you so much for kicking the tires here. Made one suggestion, and I have one important note: let's use POWERTOOLS_DEV instead of AWS_SAM_LOCAL, I'll create a RFC today.

During maintainers sync, the TypeScript team also had customers for the same request except that they want to use node <app.ts>, or Amplify-CLI, which makes this scenario muddy.

We thought of a new environment variable POWERTOOLS_DEV to detect intent on overriding decisions like this and others in the future (e.g., Event Handler has a debug mode which we can deprecate in favour of this in v2). This helps provide a consistent experience across Powertools languages, document behaviours this would enable upfront in our docs, and avoid a proliferation of multiple env vars.

This also futureproof a potential move to AWS Powertools where we could support[1] both Lambda, Fargate, etc.

If your're busy, I'm happy to contribute these changes directly to your fork.

[1] Despite our tenets and name, we've been told by customers who use Powertools in Glue jobs, Fargate, and even IoT; classic Hyrum's law.

[heitorlessa]

Suggestion: Use a constant to ease maintaining this later, e.g.

Suggested change

	4 if os.getenv("AWS_SAM_LOCAL", "").lower() == "true" else None
	PRETTY_INDENT if os.getenv("AWS_SAM_LOCAL", "").lower() == "true" else COMPACT_INDENT

[pmarko1711]

OK, I will change this later today. Is there a good place to put the constants in, pls?

[heitorlessa]

OK, I will change this later today. Is there a good place to put the constants in, pls?

Yes! shared/constants.py is where we keep them.

[heitorlessa]

We're making progress in the RFC but this might be of interest if others agree this is a clearer intent:

aws-powertools/powertools-lambda#86 (comment)

[pmarko1711]

OK, I changed everything to POWERTOOLS_DEV and also renamed the test functions and this PR, @heitorlessa .

Since this env var will have multiple effects, shouldn't it be explained in a more prominent place in the documentation? For now I only put an info note in the page about logger.

Maybe it's not for this PR though.

[heitorlessa]

thanks a lot @pmarko1711 and yes, I'll create a separate PR documenting the effects.

I'll look into the PR as soon as I complete another task today, hopefully by EOD the latest. With your permission, I might push changes directly to your fork to speed up merging this change (in case I have delays with my primary task) - I'll share feedback nevertheless.

[pmarko1711]

ok, yes, no problem @heitorlessa . If you need me to set something (permissions wise) on my fork, let me know.

[heitorlessa]

Looking at it now ;) Appreciate the patience very much

[heitorlessa]

Nice work!! I love the wording - straight to the point.

Made a few suggestions to improve test accuracy, and use our strtobool to support cases where customers may use boolean strings like 0 or 1

[pmarko1711]

I've committed all your suggestions, @heitorlessa , with a small change - I counted os.linesep instead of \n in the functional tests

[heitorlessa]

Looking into now ;) Thank you so much for these changes!

[heitorlessa]

Looks great @pmarko1711 - thank you so much for working throughout the review process. Merging now.

Next steps: I'll (1) update the RFC with the latest info on POWERTOOLS_DEV and POWERTOOLS_DEBUG (separate feat altogether), (2) create a PR to update the documentation on these flags, and (3) release this feature by the end of the week.

[boring-cyborg]

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

[pmarko1711]

thanks for all the support, @heitorlessa