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
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions google/cloud/logging_v2/entries.py
Expand Up @@ -27,6 +27,9 @@
from google.cloud._helpers import _rfc3339_nanos_to_datetime
from google.cloud._helpers import _datetime_to_rfc3339

# import officially supported proto definitions
import google.cloud.audit.audit_log_pb2 # noqa: F401
import google.cloud.appengine_logging # noqa: F401

_GLOBAL_RESOURCE = Resource(type="global", labels={})

Expand Down Expand Up @@ -316,13 +319,18 @@ def payload_pb(self):

@property
def payload_json(self):
if not isinstance(self.payload, Any):
if isinstance(self.payload, collections.abc.Mapping):
return self.payload

def to_api_repr(self):
"""API repr (JSON format) for entry."""
info = super(ProtobufEntry, self).to_api_repr()
info["protoPayload"] = MessageToDict(self.payload)
proto_payload = None
if self.payload_json:
proto_payload = dict(self.payload_json)
elif self.payload_pb:
proto_payload = MessageToDict(self.payload_pb)
info["protoPayload"] = proto_payload
return info

def parse_message(self, message):
Expand Down
2 changes: 2 additions & 0 deletions setup.py
Expand Up @@ -44,6 +44,8 @@
release_status = "Development Status :: 5 - Production/Stable"
dependencies = [
"google-api-core[grpc] >= 1.22.2, < 2.0.0dev",
"google-cloud-appengine-logging >= 0.1.0, < 1.0.0dev",
"google-cloud-audit-log >= 0.1.0, < 1.0.0dev",
"google-cloud-core >= 1.4.1, < 2.0dev",
"proto-plus >= 1.11.0",
]
Expand Down
134 changes: 116 additions & 18 deletions tests/system/test_system.py
Expand Up @@ -16,6 +16,7 @@
from datetime import timedelta
from datetime import timezone
import logging
import numbers
import os
import pytest
import unittest
Expand All @@ -36,6 +37,8 @@
from google.cloud.logging_v2 import client
from google.cloud.logging_v2.resource import Resource

from google.protobuf.struct_pb2 import Struct, Value, ListValue, NullValue

from test_utils.retry import RetryErrors
from test_utils.retry import RetryResult
from test_utils.system import unique_resource_id
Expand Down Expand Up @@ -142,32 +145,127 @@ def tearDown(self):
def _logger_name(prefix):
return prefix + unique_resource_id("-")

def test_list_entry_with_unregistered(self):
from google.protobuf import any_pb2
@staticmethod
def _to_value(data):
if data is None:
return Value(null_value=NullValue.NULL_VALUE)
elif isinstance(data, numbers.Number):
return Value(number_value=data)
elif isinstance(data, str):
return Value(string_value=data)
elif isinstance(data, bool):
return Value(bool_value=data)
elif isinstance(data, (list, tuple, set)):
return Value(
list_value=ListValue(values=(TestLogging._to_value(e) for e in data))
)
elif isinstance(data, dict):
return Value(struct_value=TestLogging._dict_to_struct(data))
else:
raise TypeError("Unknown data type: %r" % type(data))

@staticmethod
def _dict_to_struct(data):
return Struct(fields={k: TestLogging._to_value(v) for k, v in data.items()})

def test_list_entry_with_auditlog(self):
daniel-sanche marked this conversation as resolved.
Show resolved Hide resolved
"""
Test emitting and listing logs containing a google.cloud.audit.AuditLog proto message
"""
from google.protobuf import descriptor_pool
from google.cloud.logging_v2 import entries

pool = descriptor_pool.Default()
type_name = "google.cloud.audit.AuditLog"
# Make sure the descriptor is not known in the registry.
with self.assertRaises(KeyError):
pool.FindMessageTypeByName(type_name)
type_url = "type.googleapis.com/" + type_name
# Make sure the descriptor is known in the registry.
# Raises KeyError if unknown
pool.FindMessageTypeByName(type_name)

# create log
audit_dict = {
"@type": type_url,
"methodName": "test",
"requestMetadata": {"callerIp": "::1", "callerSuppliedUserAgent": "test"},
"resourceName": "test",
"serviceName": "test",
"status": {"code": 0},
}
audit_struct = self._dict_to_struct(audit_dict)

logger = Config.CLIENT.logger("audit-proto")
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

entry_iter = iter(logger.list_entries(page_size=1, filter_=filter_))

retry = RetryErrors((TooManyRequests, StopIteration), max_tries=10)
protobuf_entry = retry(lambda: next(entry_iter))()

self.assertIsInstance(protobuf_entry, entries.ProtobufEntry)
self.assertIsNone(protobuf_entry.payload_pb)
self.assertIsInstance(protobuf_entry.payload_json, dict)
self.assertEqual(protobuf_entry.payload_json["@type"], type_url)
self.assertEqual(
protobuf_entry.payload_json["methodName"], audit_dict["methodName"]
)
self.assertEqual(
protobuf_entry.to_api_repr()["protoPayload"]["@type"], type_url
)
self.assertEqual(
protobuf_entry.to_api_repr()["protoPayload"]["methodName"],
audit_dict["methodName"],
)

def test_list_entry_with_requestlog(self):
"""
Test emitting and listing logs containing a google.appengine.logging.v1.RequestLog proto message
"""
from google.protobuf import descriptor_pool
from google.cloud.logging_v2 import entries

pool = descriptor_pool.Default()
type_name = "google.appengine.logging.v1.RequestLog"
type_url = "type.googleapis.com/" + type_name
# Make sure the descriptor is known in the registry.
# Raises KeyError if unknown
pool.FindMessageTypeByName(type_name)

# create log
req_dict = {
"@type": type_url,
"ip": "0.0.0.0",
"appId": "test",
"versionId": "test",
"requestId": "12345",
"startTime": "2021-06-02T23:15:41.225062Z",
"endTime": "2021-06-02T23:16:41.225062Z",
"latency": "500.0s",
"method": "GET",
"status": 500,
"resource": "test",
"httpVersion": "HTTP/1.1",
}
req_struct = self._dict_to_struct(req_dict)

logger = Config.CLIENT.logger("req-proto")
logger.log_proto(req_struct)

# retrieve log
filter_ = self.TYPE_FILTER.format(type_url) + f" AND {_time_filter}"
entry_iter = iter(Config.CLIENT.list_entries(page_size=1, filter_=filter_))
entry_iter = iter(logger.list_entries(page_size=1, filter_=filter_))

retry = RetryErrors(TooManyRequests)
retry = RetryErrors((TooManyRequests, StopIteration), max_tries=10)
protobuf_entry = retry(lambda: next(entry_iter))()

self.assertIsInstance(protobuf_entry, entries.ProtobufEntry)
if Config.CLIENT._use_grpc:
self.assertIsNone(protobuf_entry.payload_json)
self.assertIsInstance(protobuf_entry.payload_pb, any_pb2.Any)
self.assertEqual(protobuf_entry.payload_pb.type_url, type_url)
else:
self.assertIsNone(protobuf_entry.payload_pb)
self.assertEqual(protobuf_entry.payload_json["@type"], type_url)
self.assertIsNone(protobuf_entry.payload_pb)
self.assertIsInstance(protobuf_entry.payload_json, dict)
self.assertEqual(protobuf_entry.payload_json["@type"], type_url)
self.assertEqual(
protobuf_entry.to_api_repr()["protoPayload"]["@type"], type_url
)

def test_log_text(self):
TEXT_PAYLOAD = "System test: test_log_text"
Expand Down Expand Up @@ -288,7 +386,7 @@ def test_log_handler_async(self):

cloud_logger = logging.getLogger(handler.name)
cloud_logger.addHandler(handler)
cloud_logger.warn(LOG_MESSAGE)
cloud_logger.warning(LOG_MESSAGE)
handler.flush()
entries = _list_entries(logger)
expected_payload = {"message": LOG_MESSAGE, "python_logger": handler.name}
Expand All @@ -310,7 +408,7 @@ def test_log_handler_sync(self):
LOGGER_NAME = "mylogger"
cloud_logger = logging.getLogger(LOGGER_NAME)
cloud_logger.addHandler(handler)
cloud_logger.warn(LOG_MESSAGE)
cloud_logger.warning(LOG_MESSAGE)

entries = _list_entries(logger)
expected_payload = {"message": LOG_MESSAGE, "python_logger": LOGGER_NAME}
Expand Down Expand Up @@ -340,7 +438,7 @@ def test_handlers_w_extras(self):
"resource": Resource(type="cloudiot_device", labels={}),
"labels": {"test-label": "manual"},
}
cloud_logger.warn(LOG_MESSAGE, extra=extra)
cloud_logger.warning(LOG_MESSAGE, extra=extra)

entries = _list_entries(logger)
self.assertEqual(len(entries), 1)
Expand All @@ -361,7 +459,7 @@ def test_log_root_handler(self):
self.to_delete.append(logger)

google.cloud.logging.handlers.handlers.setup_logging(handler)
logging.warn(LOG_MESSAGE)
logging.warning(LOG_MESSAGE)

entries = _list_entries(logger)
expected_payload = {"message": LOG_MESSAGE, "python_logger": "root"}
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/test_entries.py
Expand Up @@ -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)

from google.protobuf.struct_pb2 import Struct, Value
from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE

LOG_NAME = "struct.log"
message = Struct(fields={"foo": Value(bool_value=True)})
entry = self._make_one(log_name=LOG_NAME, payload=message)
expected = {
"logName": LOG_NAME,
"jsonPayload": message,
"resource": _GLOBAL_RESOURCE._to_dict(),
}
self.assertEqual(entry.to_api_repr(), expected)

def test_to_api_repr_explicit(self):
import datetime
from google.cloud.logging import Resource
Expand Down