Skip to content

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
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.

3 participants