Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

feat: include context on batch log errors #650

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

Merged
merged 10 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions 43 google/cloud/logging_v2/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""Define API Loggers."""

import collections
import re

from google.cloud.logging_v2._helpers import _add_defaults_to_filter
from google.cloud.logging_v2.entries import LogEntry
Expand All @@ -25,6 +26,9 @@
from google.cloud.logging_v2.handlers._monitored_resources import detect_resource
from google.cloud.logging_v2._instrumentation import _add_instrumentation

from google.api_core.exceptions import InvalidArgument
from google.rpc.error_details_pb2 import DebugInfo

import google.protobuf.message

_GLOBAL_RESOURCE = Resource(type="global", labels={})
Expand Down Expand Up @@ -459,8 +463,39 @@ def commit(self, *, client=None, partial_success=True):
kwargs["labels"] = self.logger.labels

entries = [entry.to_api_repr() for entry in self.entries]

client.logging_api.write_entries(
entries, partial_success=partial_success, **kwargs
)
try:
client.logging_api.write_entries(
entries, partial_success=partial_success, **kwargs
)
except InvalidArgument as e:
# InvalidArgument is often sent when a log is too large
# attempt to attach extra contex on which log caused error
self._append_context_to_error(e)
raise e
del self.entries[:]

def _append_context_to_error(self, err):
"""
Attempts to Modify `write_entries` exception messages to contain
context on which log in the batch caused the error.

Best-effort basis. If another exception occurs while processing the
input exception, the input will be left unmodified

Args:
err (~google.api_core.exceptions.InvalidArgument):
The original exception object
"""
try:
# find debug info proto if in details
debug_info = next(x for x in err.details if isinstance(x, DebugInfo))
losalex marked this conversation as resolved.
Show resolved Hide resolved
# parse out the index of the faulty entry
error_idx = re.search("(?<=key: )[0-9]+", debug_info.detail).group(0)
# find the faulty entry object
found_entry = self.entries[int(error_idx)]
str_entry = str(found_entry.to_api_repr())
# modify error message to contain extra context
err.message = f"{err.message}: {str_entry:.2000}..."
except Exception:
# if parsing fails, abort changes and leave err unmodified
pass
102 changes: 102 additions & 0 deletions 102 tests/unit/test_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
from datetime import datetime
from datetime import timedelta
from datetime import timezone
import sys

import unittest
import pytest

import mock

Expand Down Expand Up @@ -1739,6 +1741,87 @@ def test_context_mgr_failure(self):
self.assertEqual(list(batch.entries), UNSENT)
self.assertIsNone(api._write_entries_called_with)

@pytest.mark.skipif(
sys.version_info < (3, 8),
reason="InvalidArgument init with details requires python3.8+",
)
def test_append_context_to_error(self):
"""
If an InvalidArgument exception contains info on the log that threw it,
we should be able to add it to the exceptiom message. If not, the
exception should be unchanged
"""
from google.api_core.exceptions import InvalidArgument
from google.rpc.error_details_pb2 import DebugInfo
from google.cloud.logging import TextEntry

logger = _Logger()
client = _Client(project=self.PROJECT)
batch = self._make_one(logger, client=client)
test_entries = [TextEntry(payload=str(i)) for i in range(11)]
batch.entries = test_entries
starting_message = "test"
# test that properly formatted exceptions add log details
for idx, entry in enumerate(test_entries):
api_entry = entry.to_api_repr()
err = InvalidArgument(
starting_message, details=["padding", DebugInfo(detail=f"key: {idx}")]
)
batch._append_context_to_error(err)
self.assertEqual(err.message, f"{starting_message}: {str(api_entry)}...")
self.assertIn(str(idx), str(entry))
# test with missing debug info
err = InvalidArgument(starting_message, details=[])
batch._append_context_to_error(err)
self.assertEqual(
err.message, starting_message, "message should have been unchanged"
)
# test with missing key
err = InvalidArgument(
starting_message, details=["padding", DebugInfo(detail="no k e y here")]
)
batch._append_context_to_error(err)
self.assertEqual(
err.message, starting_message, "message should have been unchanged"
)
# test with key out of range
err = InvalidArgument(
starting_message, details=["padding", DebugInfo(detail="key: 100")]
)
batch._append_context_to_error(err)
self.assertEqual(
err.message, starting_message, "message should have been unchanged"
)

@pytest.mark.skipif(
sys.version_info < (3, 8),
reason="InvalidArgument init with details requires python3.8+",
)
def test_batch_error_gets_context(self):
"""
Simulate an InvalidArgument sent as part of a batch commit, to ensure
_append_context_to_error is thrown
"""
from google.api_core.exceptions import InvalidArgument
from google.rpc.error_details_pb2 import DebugInfo
from google.cloud.logging import TextEntry

logger = _Logger()
client = _Client(project=self.PROJECT)
starting_message = "hello"
exception = InvalidArgument(
starting_message, details=[DebugInfo(detail="key: 1")]
)
client.logging_api = _DummyLoggingExceptionAPI(exception)
batch = self._make_one(logger, client=client)
test_entries = [TextEntry(payload=str(i)) for i in range(11)]
batch.entries = test_entries
with self.assertRaises(InvalidArgument) as e:
batch.commit()
expected_log = test_entries[1]
api_entry = expected_log.to_api_repr()
self.assertEqual(e.message, f"{starting_message}: {str(api_entry)}...")


class _Logger(object):

Expand Down Expand Up @@ -1773,6 +1856,25 @@ def logger_delete(self, logger_name):
self._logger_delete_called_with = logger_name


class _DummyLoggingExceptionAPI(object):
def __init__(self, exception):
self.exception = exception

def write_entries(
self,
entries,
*,
logger_name=None,
resource=None,
labels=None,
partial_success=False,
):
raise self.exception

def logger_delete(self, logger_name):
raise self.exception


class _Client(object):
def __init__(self, project, connection=None):
self.project = project
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.