feat: support AuditLog and RequestLog protos#274
Conversation
tests/system/test_system.py
Outdated
| logger.log_proto(audit_struct) | ||
|
|
||
| # retrieve log | ||
| filter_ = self.TYPE_FILTER.format(type_url) + f" AND {_time_filter}" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
2 cents: I recall in Python (and Node), this kind of interpolation is preferred
| } | ||
| self.assertEqual(entry.to_api_repr(), expected) | ||
|
|
||
| def test_to_api_repr_struct(self): |
There was a problem hiding this comment.
please, no abbreviations in test or other method names
There was a problem hiding this comment.
"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)
If you run
logger.list_logs()and encounter an unknown proto log, it will crash the libraryThis PR adds support for the two officially supported proto types:
Fixes #16