From 6684ddd63562a5fef003784dfe796f0c817f60ba Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 3 Apr 2017 16:37:49 -0700 Subject: [PATCH] Gracefully continue if LogEntry.proto_payload type URL is not in registry. Fixes #2674. --- logging/google/cloud/logging/_gax.py | 55 +++++++++++++++++++++++-- logging/google/cloud/logging/_http.py | 4 +- logging/google/cloud/logging/entries.py | 18 ++++++-- logging/google/cloud/logging/logger.py | 38 ++++++++++------- 4 files changed, 92 insertions(+), 23 deletions(-) diff --git a/logging/google/cloud/logging/_gax.py b/logging/google/cloud/logging/_gax.py index e2f048fbd54f..d1e6196bbebb 100644 --- a/logging/google/cloud/logging/_gax.py +++ b/logging/google/cloud/logging/_gax.py @@ -243,6 +243,8 @@ def sink_get(self, project, sink_name): if exc_to_code(exc.cause) == StatusCode.NOT_FOUND: raise NotFound(path) raise + # NOTE: LogSink message type does not have an ``Any`` field + # so `MessageToDict`` can safely be used. return MessageToDict(sink_pb) def sink_update(self, project, sink_name, filter_, destination): @@ -270,11 +272,13 @@ def sink_update(self, project, sink_name, filter_, destination): path = 'projects/%s/sinks/%s' % (project, sink_name) sink_pb = LogSink(name=path, filter=filter_, destination=destination) try: - self._gax_api.update_sink(path, sink_pb, options=options) + sink_pb = self._gax_api.update_sink(path, sink_pb, options=options) except GaxError as exc: if exc_to_code(exc.cause) == StatusCode.NOT_FOUND: raise NotFound(path) raise + # NOTE: LogSink message type does not have an ``Any`` field + # so `MessageToDict`` can safely be used. return MessageToDict(sink_pb) def sink_delete(self, project, sink_name): @@ -391,6 +395,8 @@ def metric_get(self, project, metric_name): if exc_to_code(exc.cause) == StatusCode.NOT_FOUND: raise NotFound(path) raise + # NOTE: LogMetric message type does not have an ``Any`` field + # so `MessageToDict`` can safely be used. return MessageToDict(metric_pb) def metric_update(self, project, metric_name, filter_, description): @@ -418,11 +424,14 @@ def metric_update(self, project, metric_name, filter_, description): metric_pb = LogMetric(name=path, filter=filter_, description=description) try: - self._gax_api.update_log_metric(path, metric_pb, options=options) + metric_pb = self._gax_api.update_log_metric( + path, metric_pb, options=options) except GaxError as exc: if exc_to_code(exc.cause) == StatusCode.NOT_FOUND: raise NotFound(path) raise + # NOTE: LogMetric message type does not have an ``Any`` field + # so `MessageToDict`` can safely be used. return MessageToDict(metric_pb) def metric_delete(self, project, metric_name): @@ -444,6 +453,35 @@ def metric_delete(self, project, metric_name): raise +def _parse_log_entry(entry_pb): + """Special helper to parse ``LogEntry`` protobuf into a dictionary. + + The ``proto_payload`` field in ``LogEntry`` is of type ``Any``. This + can be problematic if the type URL in the payload isn't in the + ``google.protobuf`` registry. To help with parsing unregistered types, + this function will remove ``proto_payload`` before parsing. + + :type entry_pb: :class:`.log_entry_pb2.LogEntry` + :param entry_pb: Log entry protobuf. + + :rtype: dict + :returns: The parsed log entry. The ``protoPayload`` key may contain + the raw ``Any`` protobuf from ``entry_pb.proto_payload`` if + it could not be parsed. + """ + try: + return MessageToDict(entry_pb) + except TypeError: + if entry_pb.HasField('proto_payload'): + proto_payload = entry_pb.proto_payload + entry_pb.ClearField('proto_payload') + entry_mapping = MessageToDict(entry_pb) + entry_mapping['protoPayload'] = proto_payload + return entry_mapping + else: + raise + + def _log_entry_mapping_to_pb(mapping): """Helper for :meth:`write_entries`, et aliae @@ -451,6 +489,13 @@ def _log_entry_mapping_to_pb(mapping): the keys expected in the JSON API. """ entry_pb = LogEntry() + # NOTE: We assume ``mapping`` was created in ``Batch.commit`` + # or ``Logger._make_entry_resource``. In either case, if + # the ``protoPayload`` key is present, we assume that the + # type URL is registered with ``google.protobuf`` and will + # not cause any issues in the JSON->protobuf conversion + # of the corresponding ``proto_payload`` in the log entry + # (it is an ``Any`` field). ParseDict(mapping, entry_pb) return entry_pb @@ -482,7 +527,7 @@ def _item_to_entry(iterator, entry_pb, loggers): :rtype: :class:`~google.cloud.logging.entries._BaseEntry` :returns: The next log entry in the page. """ - resource = MessageToDict(entry_pb) + resource = _parse_log_entry(entry_pb) return entry_from_resource(resource, iterator.client, loggers) @@ -499,6 +544,8 @@ def _item_to_sink(iterator, log_sink_pb): :rtype: :class:`~google.cloud.logging.sink.Sink` :returns: The next sink in the page. """ + # NOTE: LogSink message type does not have an ``Any`` field + # so `MessageToDict`` can safely be used. resource = MessageToDict(log_sink_pb) return Sink.from_api_repr(resource, iterator.client) @@ -516,6 +563,8 @@ def _item_to_metric(iterator, log_metric_pb): :rtype: :class:`~google.cloud.logging.metric.Metric` :returns: The next metric in the page. """ + # NOTE: LogMetric message type does not have an ``Any`` field + # so `MessageToDict`` can safely be used. resource = MessageToDict(log_metric_pb) return Metric.from_api_repr(resource, iterator.client) diff --git a/logging/google/cloud/logging/_http.py b/logging/google/cloud/logging/_http.py index d9e7e4dacacd..825533e34e9c 100644 --- a/logging/google/cloud/logging/_http.py +++ b/logging/google/cloud/logging/_http.py @@ -293,7 +293,7 @@ def sink_update(self, project, sink_name, filter_, destination): 'filter': filter_, 'destination': destination, } - self.api_request(method='PUT', path=target, data=data) + return self.api_request(method='PUT', path=target, data=data) def sink_delete(self, project, sink_name): """API call: delete a sink resource. @@ -428,7 +428,7 @@ def metric_update(self, project, metric_name, filter_, description): 'filter': filter_, 'description': description, } - self.api_request(method='PUT', path=target, data=data) + return self.api_request(method='PUT', path=target, data=data) def metric_delete(self, project, metric_name): """API call: delete a metric resource. diff --git a/logging/google/cloud/logging/entries.py b/logging/google/cloud/logging/entries.py index 1ae5d34ec8b9..96f473d9feb0 100644 --- a/logging/google/cloud/logging/entries.py +++ b/logging/google/cloud/logging/entries.py @@ -17,6 +17,7 @@ import json import re +from google.protobuf import any_pb2 from google.protobuf.json_format import Parse from google.cloud._helpers import _name_from_project_path @@ -51,7 +52,9 @@ class _BaseEntry(object): :type payload: text or dict :param payload: The payload passed as ``textPayload``, ``jsonPayload``, - or ``protoPayload``. + or ``protoPayload``. This also may be passed as a raw + :class:`.any_pb2.Any` if the ``protoPayload`` could + not be deserialized. :type logger: :class:`google.cloud.logging.logger.Logger` :param logger: the logger used to write the entry. @@ -74,7 +77,13 @@ class _BaseEntry(object): """ def __init__(self, payload, logger, insert_id=None, timestamp=None, labels=None, severity=None, http_request=None): - self.payload = payload + if isinstance(payload, any_pb2.Any): + self.payload = None + self.payload_pb = payload + else: + self.payload = payload + self.payload_pb = None + self.logger = logger self.insert_id = insert_id self.timestamp = timestamp @@ -99,7 +108,7 @@ def from_api_repr(cls, resource, client, loggers=None): (Optional) A mapping of logger fullnames -> loggers. If not passed, the entry will have a newly-created logger. - :rtype: :class:`google.cloud.logging.entries.TextEntry` + :rtype: :class:`google.cloud.logging.entries._BaseEntry` :returns: Text entry parsed from ``resource``. """ if loggers is None: @@ -155,4 +164,7 @@ def parse_message(self, message): :type message: Protobuf message :param message: the message to be logged """ + # NOTE: This assumes that ``payload`` is already a deserialized + # ``Any`` field and ``message`` has come from an imported + # ``pb2`` module with the relevant protobuf message type. Parse(json.dumps(self.payload), message) diff --git a/logging/google/cloud/logging/logger.py b/logging/google/cloud/logging/logger.py index 459647bbea67..e6ca82abc885 100644 --- a/logging/google/cloud/logging/logger.py +++ b/logging/google/cloud/logging/logger.py @@ -16,7 +16,7 @@ import json -from google.protobuf.json_format import MessageToJson +from google.protobuf.json_format import MessageToDict from google.cloud._helpers import _datetime_to_rfc3339 @@ -106,24 +106,24 @@ def _make_entry_resource(self, text=None, info=None, message=None, :type info: dict :param info: (Optional) struct payload - :type message: Protobuf message or :class:`NoneType` - :param message: protobuf payload + :type message: :class:`~google.protobuf.message.Message` + :param message: (Optional) The protobuf payload to log. :type labels: dict :param labels: (Optional) labels passed in to calling method. :type insert_id: str - :param insert_id: (optional) unique ID for log entry. + :param insert_id: (Optional) unique ID for log entry. :type severity: str - :param severity: (optional) severity of event being logged. + :param severity: (Optional) severity of event being logged. :type http_request: dict - :param http_request: (optional) info about HTTP request associated with + :param http_request: (Optional) info about HTTP request associated with the entry :type timestamp: :class:`datetime.datetime` - :param timestamp: (optional) timestamp of event being logged. + :param timestamp: (Optional) timestamp of event being logged. :rtype: dict :returns: The JSON resource created. @@ -140,9 +140,13 @@ def _make_entry_resource(self, text=None, info=None, message=None, resource['jsonPayload'] = info if message is not None: - as_json_str = MessageToJson(message) - as_json = json.loads(as_json_str) - resource['protoPayload'] = as_json + # NOTE: If ``message`` contains an ``Any`` field with an + # unknown type, this will fail with a ``TypeError``. + # However, since ``message`` will be provided by a user, + # the assumption is that any types needed for the + # protobuf->JSON conversion will be known from already + # imported ``pb2`` modules. + resource['protoPayload'] = MessageToDict(message) if labels is None: labels = self.labels @@ -245,8 +249,8 @@ def log_proto(self, message, client=None, labels=None, insert_id=None, See: https://cloud.google.com/logging/docs/reference/v2/rest/v2/entries/list - :type message: Protobuf message - :param message: the message to be logged + :type message: :class:`~google.protobuf.message.Message` + :param message: The protobuf message to be logged. :type client: :class:`~google.cloud.logging.client.Client` or ``NoneType`` @@ -462,9 +466,13 @@ def commit(self, client=None): elif entry_type == 'struct': info = {'jsonPayload': entry} elif entry_type == 'proto': - as_json_str = MessageToJson(entry) - as_json = json.loads(as_json_str) - info = {'protoPayload': as_json} + # NOTE: If ``entry`` contains an ``Any`` field with an + # unknown type, this will fail with a ``TypeError``. + # However, since ``entry`` was provided by a user in + # ``Batch.log_proto``, the assumption is that any types + # needed for the protobuf->JSON conversion will be known + # from already imported ``pb2`` modules. + info = {'protoPayload': MessageToDict(entry)} else: raise ValueError('Unknown entry type: %s' % (entry_type,)) if labels is not None: