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 as code owners Apr 27, 2021
@product-auto-label product-auto-label bot added the api: logging label Apr 27, 2021
@google-cla google-cla bot added the cla: yes label Apr 27, 2021
@daniel-sanche daniel-sanche added the kokoro:run label Apr 27, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run label Apr 27, 2021
@daniel-sanche daniel-sanche added kokoro:force-run kokoro:run labels May 21, 2021
@yoshi-kokoro yoshi-kokoro removed kokoro:run kokoro:force-run 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

@daniel-sanche daniel-sanche Jun 3, 2021

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

@0xSage 0xSage Jun 4, 2021

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

@daniel-sanche daniel-sanche Jun 3, 2021

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)

0xSage
0xSage approved these changes Jun 4, 2021
@daniel-sanche daniel-sanche added the kokoro:force-run label Jun 7, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 7, 2021
@daniel-sanche daniel-sanche added the kokoro:force-run label Jun 8, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 8, 2021
@daniel-sanche daniel-sanche added the kokoro:force-run label Jun 9, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 9, 2021
@daniel-sanche daniel-sanche merged commit 5d91be9 into master Jun 9, 2021
14 of 17 checks passed
@daniel-sanche daniel-sanche deleted the support-protos branch Jun 9, 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.

4 participants