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): add dynamic logging key support #1505

Closed
wants to merge 59 commits into from

Conversation

OGoodness
Copy link

@OGoodness OGoodness commented Sep 8, 2022

Issue number: #1167

Summary

Allows extra keywords provided to logger.xxx to be added to log via append_keys

Changes

Logger subclass extends the _log method, allowing extra keys to be passed in without throwing error due to excess parameters. Routes expected parameters directly to super _log after appending excess keys.

User experience

Nothing is removed from the user, and they can now add logging keys with little effort.

Checklist

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

Is this a breaking change? No

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 and others added 30 commits August 25, 2022 18:21
…60 (aws-powertools#1481)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…60 (aws-powertools#306)

Bumps [mypy-boto3-dynamodb](https://github.com/youtype/mypy_boto3_builder) from 1.24.55.post1 to 1.24.60.
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

---
updated-dependencies:
- dependency-name: mypy-boto3-dynamodb
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ools#1483)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
pyproject.toml isn't being pushed back to trunk as part of release.
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 8, 2022

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

@github-actions github-actions bot added the feature New feature or functionality label Sep 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2022

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

@github-actions github-actions bot added do-not-merge need-issue PRs that are missing related issues labels Sep 8, 2022
…ls#1507)

Bumps [aws-cdk-lib](https://github.com/aws/aws-cdk) from 2.40.0 to 2.41.0.
- [Release notes](https://github.com/aws/aws-cdk/releases)
- [Changelog](https://github.com/aws/aws-cdk/blob/main/CHANGELOG.v2.md)
- [Commits](aws/aws-cdk@v2.40.0...v2.41.0)

---
updated-dependencies:
- dependency-name: aws-cdk-lib
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@heitorlessa heitorlessa requested review from leandrodamascena and removed request for mploski September 9, 2022 13:32
@leandrodamascena
Copy link
Contributor

leandrodamascena commented Sep 9, 2022

Hi @OGoodness! Thanks for sending this PR and helping to improve the project!

We suggest not removing the Issue number: from the beginning. We use this to run some automation in the PR to keep governance on track. I fixed the labels for now, so it's not a problem anymore. 😄

Before I starting to review this PR, could you please check the errors in the build action and fix them ? You can run make pr locally and check if everything is working. If you have trouble fixing it locally, ping me on Discord and we can do it together.

Thank you 🚀

@leandrodamascena leandrodamascena removed do-not-merge need-issue PRs that are missing related issues labels Sep 9, 2022
@leandrodamascena leandrodamascena self-assigned this Sep 9, 2022
ran-isenberg and others added 4 commits September 9, 2022 17:48
@OGoodness
Copy link
Author

@leandrodamascena
Yeah, I ran make pr originally and everything worked fine on my machine. Looks like it had to do with a parameter that got added in python 3.8 and doesn't exist in python 3.7.

Decided to remove the optional parameter from the input and not pass it in to _log, as it's situational at best, and I believe the Logger already accounts for its feature.

Tested this build using 3.7, so it should be functional

@OGoodness
Copy link
Author

🤦
Okay, let's try all 3 then. Hopefully this isn't an environment level thing.
(Trying out gitpod for the first time, so don't know how that might play into things)

@OGoodness
Copy link
Author

Ran mypy individually on 3.7, 3.8, 3.9, then ran make pr. (Originally thought make pr was only for first runs)
This should be good to go @leandrodamascena Sorry for the wait

Notes:

  • Added python version check in the _log, this is to account for the difference mentioned earlier.
  • Removed # type: ignore from the super calls. Due to the version check, mypy would always flag one or the other as being an unnecessary ignore
  • changed exc_info to be Any, as this removes the need for the expected type that is a union of 3+ other types, simpler to set it to any, as that will avoid complexity if the type is ever changed.

@leandrodamascena
Copy link
Contributor

Hello @OGoodness! Sorry for the delay in responding, but we were working on some items for v2 and are now I'm ready to work on it.

I see you've fixed most of the bugs in mypy, but I'm still worried about this support in Python 3.7. Could you please write some tests to cover this new code and make sure everything is working as expected?
After that we can work (I can help you) on updating the documentation and merge this PR!

Thank you 🚀

@heitorlessa
Copy link
Contributor

As discussed, closing in favour of #1658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or functionality need-more-information Pending information to continue size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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