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: Add support for custom JSON encoders #657

Conversation

thiromi
Copy link
Contributor

@thiromi thiromi commented Oct 25, 2022

Add the ability to provide custom JSONEncoder classes to StructuredLogHandler, so the log caller doesn't have to convert to primitive types every time a log record is created

Fixes #656 🦕

@thiromi thiromi requested review from a team as code owners October 25, 2022 11:10
@google-cla
Copy link

google-cla bot commented Oct 25, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: logging Issues related to the googleapis/python-logging API. labels Oct 25, 2022
@thiromi thiromi force-pushed the add-json-encoder-to-structured-log-handler branch from 84dd13f to 302491d Compare October 25, 2022 11:31
@thiromi thiromi force-pushed the add-json-encoder-to-structured-log-handler branch from 302491d to f9fc491 Compare October 26, 2022 06:45
@parthea parthea changed the title Add support to custom JSON encoders feat: Add support to custom JSON encoders Oct 26, 2022
@parthea parthea added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 26, 2022
@parthea parthea changed the title feat: Add support to custom JSON encoders feat: Add support for custom JSON encoders Oct 26, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 26, 2022
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2022
@daniel-sanche
Copy link
Contributor

Thanks for putting this together. My only hesitation is that the CloudLoggingHandler also doesn't support logging unserializable dictionary types, so it might make more sense to implement this in a more universal place. It would be ideal if the handlers can be as similar as possible.

That said, I'm not seeing a clear solution for CloudLoggingHandler, since it uses a different method of parsing JSON, and never serializes into strings like the StructuredLogHandler. It's not as simple as passing in a single argument like this.

I opened a feature request to look into this in the future, but I don't think it needs to block merging this

@daniel-sanche daniel-sanche merged commit 77e621c into googleapis:main Oct 26, 2022
@thiromi thiromi deleted the add-json-encoder-to-structured-log-handler branch October 27, 2022 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/python-logging API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support to custom JSONEncoder
5 participants