fix: Made write_entries raise ValueError on ParseErrors (#958)

* fix: Log Logger errors internally rather than crashing

* Documentation.

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Made write_entries raise ValueError on ParseErrors

* Finished fixing up merge conflicts

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* linting pt.2

* docstring change

* Improved docstring message

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

google/cloud/logging_v2/_gapic.py

@@ -30,6 +30,7 @@
 
 from google.protobuf.json_format import MessageToDict
 from google.protobuf.json_format import ParseDict
+from google.protobuf.json_format import ParseError
 
 from google.cloud.logging_v2._helpers import entry_from_resource
 from google.cloud.logging_v2.sink import Sink
@@ -151,7 +152,10 @@ def write_entries(
                 Useful for checking whether the logging API endpoints are working
                 properly before sending valuable data.
         """
-        log_entry_pbs = [_log_entry_mapping_to_pb(entry) for entry in entries]
+        try:
+            log_entry_pbs = [_log_entry_mapping_to_pb(entry) for entry in entries]
+        except ParseError as e:
+            raise ValueError(f"Invalid log entry: {str(e)}") from e
 
         request = WriteLogEntriesRequest(
             log_name=logger_name,

google/cloud/logging_v2/logger.py

@@ -162,6 +162,7 @@ def _do_log(self, client, _entry_class, payload=None, **kw):
 
         api_repr = entry.to_api_repr()
         entries = [api_repr]
+
         if google.cloud.logging_v2._instrumentation_emitted is False:
             entries = _add_instrumentation(entries, **kw)
             google.cloud.logging_v2._instrumentation_emitted = True
@@ -200,18 +201,38 @@ def log_text(self, text, *, client=None, **kw):
         self._do_log(client, TextEntry, text, **kw)
 
     def log_struct(self, info, *, client=None, **kw):
-        """Log a dictionary message
+        """Logs a dictionary message.
 
         See
         https://cloud.google.com/logging/docs/reference/v2/rest/v2/entries/write
 
+        The message must be able to be serializable to a Protobuf Struct.
+        It must be a dictionary of strings to one of the following:
+
+            - :class:`str`
+            - :class:`int`
+            - :class:`float`
+            - :class:`bool`
+            - :class:`list[str|float|int|bool|list|dict|None]`
+            - :class:`dict[str, str|float|int|bool|list|dict|None]`
+
+        For more details on Protobuf structs, see https://protobuf.dev/reference/protobuf/google.protobuf/#value.
+        If the provided dictionary cannot be serialized into a Protobuf struct,
+        it will not be logged, and a :class:`ValueError` will be raised.
+
         Args:
-            info (dict): the log entry information
+            info (dict[str, str|float|int|bool|list|dict|None]):
+                the log entry information.
             client (Optional[~logging_v2.client.Client]):
                 The client to use.  If not passed, falls back to the
                 ``client`` stored on the current sink.
             kw (Optional[dict]): additional keyword arguments for the entry.
                 See :class:`~logging_v2.entries.LogEntry`.
+
+        Raises:
+            ValueError:
+                if the dictionary message provided cannot be serialized into a Protobuf
+                struct.
         """
         for field in _STRUCT_EXTRACTABLE_FIELDS:
             # attempt to copy relevant fields from the payload into the LogEntry body
@@ -405,8 +426,22 @@ def log_text(self, text, **kw):
     def log_struct(self, info, **kw):
         """Add a struct entry to be logged during :meth:`commit`.
 
+        The message must be able to be serializable to a Protobuf Struct.
+        It must be a dictionary of strings to one of the following:
+
+            - :class:`str`
+            - :class:`int`
+            - :class:`float`
+            - :class:`bool`
+            - :class:`list[str|float|int|bool|list|dict|None]`
+            - :class:`dict[str, str|float|int|bool|list|dict|None]`
+
+        For more details on Protobuf structs, see https://protobuf.dev/reference/protobuf/google.protobuf/#value.
+        If the provided dictionary cannot be serialized into a Protobuf struct,
+        it will not be logged, and a :class:`ValueError` will be raised during :meth:`commit`.
+
         Args:
-            info (dict): The struct entry,
+            info (dict[str, str|float|int|bool|list|dict|None]): The struct entry,
             kw (Optional[dict]): Additional keyword arguments for the entry.
                 See :class:`~logging_v2.entries.LogEntry`.
         """
@@ -451,6 +486,10 @@ def commit(self, *, client=None, partial_success=True):
                 Whether a batch's valid entries should be written even
                 if some other entry failed due to a permanent error such
                 as INVALID_ARGUMENT or PERMISSION_DENIED.
+
+        Raises:
+            ValueError:
+                if one of the messages in the batch cannot be successfully parsed.
         """
         if client is None:
             client = self.client

tests/unit/test__gapic.py

@@ -17,6 +17,8 @@
 import google.auth.credentials
 import mock
 
+from datetime import datetime
+
 import google.cloud.logging
 from google.cloud import logging_v2
 from google.cloud.logging_v2 import _gapic
@@ -173,6 +175,21 @@ def test_write_entries_single(self):
         assert request.entries[0].resource.type == entry["resource"]["type"]
         assert request.entries[0].text_payload == "text"
 
+    def test_write_entries_parse_error(self):
+        client = self.make_logging_api()
+        with self.assertRaises(ValueError):
+            with mock.patch.object(
+                type(client._gapic_api.transport.write_log_entries), "__call__"
+            ) as call:
+                entry = {
+                    "logName": self.LOG_PATH,
+                    "resource": {"type": "global"},
+                    "jsonPayload": {"time": datetime.now()},
+                }
+                client.write_entries([entry])
+
+        call.assert_not_called()
+
     def test_logger_delete(self):
         client = self.make_logging_api()