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: record source locations #254

Merged
merged 29 commits into from Apr 16, 2021
Merged

feat: record source locations #254

merged 29 commits into from Apr 16, 2021

Conversation

daniel-sanche
Copy link
Contributor

@daniel-sanche daniel-sanche commented Apr 7, 2021

environments using the new CloudLoggingFilter will now capture source locations when sending logs to GCP

This feature will currently not work in GAE and GKE environments. Those use custom handlers, which I want to keep those consistent until 3.0.0 is released

@daniel-sanche daniel-sanche requested review from as code owners Apr 7, 2021
@product-auto-label product-auto-label bot added the api: logging label Apr 7, 2021
@google-cla google-cla bot added the cla: yes label Apr 7, 2021
0xSage
0xSage approved these changes Apr 8, 2021
Copy link
Contributor

@0xSage 0xSage left a comment

Is it working for all environments?

user_labels = getattr(record, "labels", {})
# merge labels
total_labels = self.labels if self.labels is not None else {}
total_labels.update(user_labels)
if len(total_labels) == 0:
total_labels = None
# create source location object
if record.lineno and record.funcName and record.pathname:

Choose a reason for hiding this comment

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

Can some of the 3 be empty? For example, code that's not part of any function.

Copy link
Contributor Author

@daniel-sanche daniel-sanche Apr 9, 2021

Choose a reason for hiding this comment

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

No, these are all part of the official record object spec, so the attributes will all exist.

They could be None, but that would be caught by this check

Base automatically changed from structured-log-handler-2 to v2_update_2 Apr 9, 2021
@daniel-sanche daniel-sanche merged commit a5c2f8e into v2_update_2 Apr 16, 2021
15 checks passed
@daniel-sanche daniel-sanche deleted the add-source-locations branch Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants