Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(logger): accept arbitrary keyword=value for ephemeral metadata #1658

Merged

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Oct 26, 2022

Issue number: #1167

This supersedes PR #1505 as (1) Noah is busy with his day-to-day work priorities, and (2) the actual solution requires a full rewrite of the original PR

Summary

This PR backports the necessary code implemented in CPython 3.8, monkeypatch and adds additional guards to only use the backported code in older Python versions to not impact performance and future improvements.

Changes

Please provide a summary of what's being changed

This change allows customers to pass arbitrary and ephemeral keyword=value arguments in log statements.

It addresses one of the oldest issues and makes it a better UX than extra argument. Both can coexist. This is a backport feature from Python 3.8.

Why did it take us so long?

The reason that this took us so long was that we inject the location key in log, so customers know where a logging statement came from in their code.

{
    "level": "WARNING",
    "location": "warning:465",
    "message": "Testing",
    "timestamp": "2022-10-26 16:22:27,543+0200",
    "service": "payment",
    "request_id": "0c7ff166-3be1-4795-8f0c-b48ca5ce5a95"
}

When using a class like Logger, we need to override all log statement methods. When doing so, it moves the stack frame location, where the location key now points to where the statement was called inside Logger, instead of the customer log statement location.

def warning(msg, *args, **kwargs):...
def info(msg, *args, **kwargs):
    return self._logger.info(msg, *args, **kwargs)  # <--- location would always show up here
....

With this change, the location continues to be precise as we move the stack frame location.

{
    "level": "WARNING",
    "location": "<module>:7",
    "message": "Testing",
    "timestamp": "2022-10-26 15:54:34,945+0200",
    "service": "payment",
    "request_id": "c344d8ca-2697-42b1-ad67-2bed036c550d",
}

User experience

Please share what the user experience looks like before and after this change

BEFORE

from aws_lambda_powertools import Logger

from uuid import uuid4

logger = Logger(service="payment")

logger.warning("Testing", extra={"request_id": f"{uuid4()}"})

AFTER

from aws_lambda_powertools import Logger

from uuid import uuid4

logger = Logger(service="payment")

logger.warning("Testing", request_id=f"{uuid4()}")

Checklist

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

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.

@heitorlessa heitorlessa requested a review from a team as a code owner October 26, 2022 14:00
@heitorlessa heitorlessa requested review from leandrodamascena and removed request for a team October 26, 2022 14:00
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 26, 2022
@github-actions github-actions bot added the feature New feature or functionality label Oct 26, 2022
@heitorlessa
Copy link
Contributor Author

Waiting for CI to complete to ensure it works in a fresh Python env, then will refresh Docs.

@heitorlessa
Copy link
Contributor Author

Need to add co-authorship for Noah before merging (@OGoodness). I'll work on docs now as we've finished peer review due to the complexity of this change.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Oct 26, 2022
@heitorlessa
Copy link
Contributor Author

Docs now in!

image

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @heitorlessa! Thanks for sharing the ideas you had for changing the code this way and make it simple.

image

@leandrodamascena leandrodamascena merged commit 87de70c into aws-powertools:develop Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature New feature or functionality size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logger to support arbitrary key=value style arguments in log statements
2 participants