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: support AuditLog and RequestLog protos #274

Merged
merged 19 commits into from Jun 9, 2021
Merged

Conversation

daniel-sanche
Copy link
Contributor

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

If you run logger.list_logs() and encounter an unknown proto log, it will crash the library

This PR adds support for the two officially supported proto types:

  • "type.googleapis.com/google.cloud.audit.AuditLog"
  • "type.googleapis.com/google.appengine.logging.v1.RequestLog"

Fixes #16

@daniel-sanche daniel-sanche requested review from a team as code owners April 27, 2021 00:31
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label Apr 27, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2021
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 27, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 27, 2021
@daniel-sanche daniel-sanche added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels May 21, 2021
@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 May 21, 2021
@daniel-sanche daniel-sanche changed the title [DRAFT] Support AuditLog protos Support AuditLog and RequestLog protos Jun 3, 2021
@daniel-sanche daniel-sanche changed the title Support AuditLog and RequestLog protos feat: support AuditLog and RequestLog protos Jun 3, 2021
tests/system/test_system.py Show resolved Hide resolved
logger.log_proto(audit_struct)

# retrieve log
filter_ = self.TYPE_FILTER.format(type_url) + f" AND {_time_filter}"

Choose a reason for hiding this comment

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

this is a side note for logging language syntax and not to this specific line:
I find it ugly when we concatenate strings to form a query. Perhaps, we should think about creation of an abstraction that will make forming these filters more intuitive when you read the code.
For example:

Instead of

xxx + " AND " + time_filter

it could be

x.And(new BinaryOperator(time1, operands.Equal, time2)

This could be a separate task and not part of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an interesting idea. I'm not sure if that specific example would be seen as idiomatic for Python, but there's probably something interesting we could do with this

Copy link
Contributor

Choose a reason for hiding this comment

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

2 cents: I recall in Python (and Node), this kind of interpolation is preferred

@@ -503,6 +503,20 @@ def test_to_api_repr_defaults(self):
}
self.assertEqual(entry.to_api_repr(), expected)

def test_to_api_repr_struct(self):

Choose a reason for hiding this comment

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

please, no abbreviations in test or other method names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"to_api_repr" is an existing function that we can't change here without breaking the API. We could add this to the list for v3.0.0 changes (but it may not be worth the user confusion)

@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 7, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 7, 2021
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 8, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 8, 2021
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 9, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 9, 2021
@daniel-sanche daniel-sanche merged commit 5d91be9 into master Jun 9, 2021
@daniel-sanche daniel-sanche deleted the support-protos branch June 9, 2021 19:36
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.

Logging: Add support for deserializing protobuf payloads.
4 participants