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

fix: improve API compatibility for next release #292

Merged
merged 12 commits into from May 12, 2021

Conversation

daniel-sanche
Copy link
Contributor

  • As aprt of the next release, I removed some fields in the http_request data (referer, remoteIp, requestSize), because they don't work consistently across all GCP environments yet. I realized this would be considered a breaking change for AppEngine users who may already be relying on these fields. Instead of removing the fields, non-appengine handlers now filter them out. We can re-consider the future of these fields for v3.0.0
  • StructuredLogHandler formatting breaks if users log text with quotes in it. Now, the handler will add "" before each quote found in the message string
  • The Flask request detection broke in the unit tests, so I also fixed that

@daniel-sanche daniel-sanche self-assigned this May 11, 2021
@daniel-sanche daniel-sanche requested review from a team as code owners May 11, 2021 21:18
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label May 11, 2021
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 11, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 11, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 11, 2021
@@ -45,6 +45,10 @@ class CloudLoggingFilter(logging.Filter):
overwritten using the `extras` argument when writing logs.
"""

# The subset of http_request fields have been tested to work consistently across GCP environments
Copy link
Contributor

Choose a reason for hiding this comment

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

Great commenting throughout this PR.

import json

handler = self._make_one()
message = '"test"'
Copy link
Contributor

Choose a reason for hiding this comment

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

Are other special chars escaped as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't seen any that need it. Let me know if you find others, I imagine it will be the same across languages

@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 12, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 12, 2021
@daniel-sanche daniel-sanche merged commit dfe916b into v2_update_2 May 12, 2021
@daniel-sanche daniel-sanche deleted the extra-http-fields branch May 12, 2021 17:57
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants