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.

Sign up for GitHub

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

Jump to bottom

feat: set partial_success to default to true for batched logs #649

Merged
merged 7 commits into from
Oct 17, 2022
Merged

feat: set partial_success to default to true for batched logs #649

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
00dd76c
set partial_success to default to true for batched logs
daniel-sanche Oct 11, 2022
dfdc92a
changed default partial success value for http
daniel-sanche Oct 11, 2022
1355a63
Merge branch 'main' into partial_success
daniel-sanche Oct 17, 2022
035ea7f
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Oct 17, 2022
2748b0d
fixed http tests
daniel-sanche Oct 17, 2022
38268f1
ran blacken
daniel-sanche Oct 17, 2022
252ec5b
Merge branch 'partial_success' of github.com:googleapis/python-loggin…
daniel-sanche Oct 17, 2022
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion google/cloud/logging_v2/_gapic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def write_entries(
logger_name=None,
resource=None,
labels=None,
partial_success=False,
losalex marked this conversation as resolved.
Show resolved Hide resolved
partial_success=True,
dry_run=False,
):
"""Log an entry resource via a POST request
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/logging_v2/_http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def write_entries(
logger_name=None,
resource=None,
labels=None,
partial_success=False,
partial_success=True,
dry_run=False,
):
"""Log an entry resource via a POST request
Expand Down
16 changes: 10 additions & 6 deletions google/cloud/logging_v2/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ def _do_log(self, client, _entry_class, payload=None, **kw):
kw["log_name"] = kw.pop("log_name", self.full_name)
kw["labels"] = kw.pop("labels", self.labels)
kw["resource"] = kw.pop("resource", self.default_resource)
partial_success = False

severity = kw.get("severity", None)
if isinstance(severity, str) and not severity.isupper():
Expand All @@ -159,11 +158,10 @@ 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:
partial_success = True
entries = _add_instrumentation(entries, **kw)
google.cloud.logging_v2._instrumentation_emitted = True

client.logging_api.write_entries(entries, partial_success=partial_success)
# partial_success is true to avoid dropping instrumentation logs
client.logging_api.write_entries(entries, partial_success=True)

def log_empty(self, *, client=None, **kw):
"""Log an empty message
Expand Down Expand Up @@ -437,13 +435,17 @@ def log(self, message=None, **kw):
entry_type = TextEntry
self.entries.append(entry_type(payload=message, **kw))

def commit(self, *, client=None):
def commit(self, *, client=None, partial_success=True):
"""Send saved log entries as a single API call.

Args:
client (Optional[~logging_v2.client.Client]):
The client to use. If not passed, falls back to the
``client`` stored on the current batch.
partial_success (Optional[bool]):
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.
"""
if client is None:
client = self.client
Expand All @@ -458,5 +460,7 @@ def commit(self, *, client=None):

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

client.logging_api.write_entries(entries, **kwargs)
client.logging_api.write_entries(
entries, partial_success=partial_success, **kwargs
)
del self.entries[:]
2 changes: 1 addition & 1 deletion tests/unit/test__gapic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def test_write_entries_single(self):
# Check the request
call.assert_called_once()
request = call.call_args.args[0]
assert request.partial_success is False
assert request.partial_success is True
assert len(request.entries) == 1
assert request.entries[0].log_name == entry["logName"]
assert request.entries[0].resource.type == entry["resource"]["type"]
Expand Down
104 changes: 76 additions & 28 deletions tests/unit/test_logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ def test_log_empty_defaults_w_default_labels(self):

logger.log_empty()

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_empty_w_explicit(self):
import datetime
Expand Down Expand Up @@ -177,7 +179,9 @@ def test_log_empty_w_explicit(self):
trace_sampled=True,
)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_text_defaults(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -199,7 +203,9 @@ def test_log_text_defaults(self):

logger.log_text(TEXT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_text_w_unicode_and_default_labels(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -223,7 +229,9 @@ def test_log_text_w_unicode_and_default_labels(self):

logger.log_text(TEXT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_text_explicit(self):
import datetime
Expand Down Expand Up @@ -280,7 +288,9 @@ def test_log_text_explicit(self):
trace_sampled=True,
)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_struct_defaults(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -302,7 +312,9 @@ def test_log_struct_defaults(self):

logger.log_struct(STRUCT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_nested_struct(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -324,7 +336,9 @@ def test_log_nested_struct(self):

logger.log(STRUCT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_struct_w_default_labels(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -348,7 +362,9 @@ def test_log_struct_w_default_labels(self):

logger.log_struct(STRUCT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_struct_w_explicit(self):
import datetime
Expand Down Expand Up @@ -405,7 +421,9 @@ def test_log_struct_w_explicit(self):
trace_sampled=True,
)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_struct_inference(self):
"""
Expand Down Expand Up @@ -439,7 +457,9 @@ def test_log_struct_inference(self):

logger.log_struct(STRUCT, resource=RESOURCE)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_w_dict_resource(self):
"""
Expand Down Expand Up @@ -467,7 +487,9 @@ def test_log_w_dict_resource(self):
}
]
logger.log(MESSAGE, resource=resource)
self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_lowercase_severity(self):
"""
Expand Down Expand Up @@ -505,7 +527,7 @@ def test_log_lowercase_severity(self):
logger.log(MESSAGE, severity=lower_severity)

self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None)
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_proto_defaults(self):
Expand All @@ -530,7 +552,9 @@ def test_log_proto_defaults(self):

logger.log_proto(message)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_proto_w_default_labels(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -556,7 +580,9 @@ def test_log_proto_w_default_labels(self):

logger.log_proto(message)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_proto_w_explicit(self):
import json
Expand Down Expand Up @@ -617,7 +643,9 @@ def test_log_proto_w_explicit(self):
trace_sampled=True,
)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_inference_empty(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -638,7 +666,9 @@ def test_log_inference_empty(self):

logger.log()

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_inference_text(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -659,7 +689,9 @@ def test_log_inference_text(self):

logger.log(TEXT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_inference_struct(self):
from google.cloud.logging_v2.handlers._monitored_resources import (
Expand All @@ -680,7 +712,9 @@ def test_log_inference_struct(self):

logger.log(STRUCT)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_log_inference_proto(self):
import json
Expand All @@ -704,7 +738,9 @@ def test_log_inference_proto(self):

logger.log(message)

self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

def test_delete_w_bound_client(self):
client = _Client(project=self.PROJECT)
Expand Down Expand Up @@ -1033,12 +1069,16 @@ def test_first_log_emits_instrumentation(self):
api = client.logging_api = _DummyLoggingAPI()
logger = self._make_one(self.LOGGER_NAME, client=client, labels=DEFAULT_LABELS)
logger.log_empty()
self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)

ENTRIES = ENTRIES[-1:]
api = client.logging_api = _DummyLoggingAPI()
logger.log_empty()
self.assertEqual(api._write_entries_called_with, (ENTRIES, None, None, None))
self.assertEqual(
api._write_entries_called_with, (ENTRIES, None, None, None, True)
)


class TestBatch(unittest.TestCase):
Expand Down Expand Up @@ -1436,7 +1476,8 @@ def test_commit_w_unknown_entry_type(self):

self.assertEqual(list(batch.entries), [])
self.assertEqual(
api._write_entries_called_with, ([ENTRY], logger.full_name, None, None)
api._write_entries_called_with,
([ENTRY], logger.full_name, None, None, True),
)

def test_commit_w_resource_specified(self):
Expand All @@ -1461,7 +1502,7 @@ def test_commit_w_resource_specified(self):
batch.commit()
self.assertEqual(
api._write_entries_called_with,
(ENTRIES, logger.full_name, RESOURCE._to_dict(), None),
(ENTRIES, logger.full_name, RESOURCE._to_dict(), None, True),
)

def test_commit_w_bound_client(self):
Expand Down Expand Up @@ -1550,7 +1591,8 @@ def test_commit_w_bound_client(self):

self.assertEqual(list(batch.entries), [])
self.assertEqual(
api._write_entries_called_with, (ENTRIES, logger.full_name, None, None)
api._write_entries_called_with,
(ENTRIES, logger.full_name, None, None, True),
)

def test_commit_w_alternate_client(self):
Expand Down Expand Up @@ -1597,12 +1639,12 @@ def test_commit_w_alternate_client(self):
batch.log_text(TEXT, labels=LABELS)
batch.log_struct(STRUCT, severity=SEVERITY)
batch.log_proto(message, http_request=REQUEST)
batch.commit(client=client2)
batch.commit(client=client2, partial_success=False)

self.assertEqual(list(batch.entries), [])
self.assertEqual(
api._write_entries_called_with,
(ENTRIES, logger.full_name, None, DEFAULT_LABELS),
(ENTRIES, logger.full_name, None, DEFAULT_LABELS, False),
)

def test_context_mgr_success(self):
Expand Down Expand Up @@ -1653,7 +1695,7 @@ def test_context_mgr_success(self):
self.assertEqual(list(batch.entries), [])
self.assertEqual(
api._write_entries_called_with,
(ENTRIES, logger.full_name, None, DEFAULT_LABELS),
(ENTRIES, logger.full_name, None, DEFAULT_LABELS, True),
)

def test_context_mgr_failure(self):
Expand Down Expand Up @@ -1719,7 +1761,13 @@ def write_entries(
labels=None,
partial_success=False,
):
self._write_entries_called_with = (entries, logger_name, resource, labels)
self._write_entries_called_with = (
entries,
logger_name,
resource,
labels,
partial_success,
)

def logger_delete(self, logger_name):
self._logger_delete_called_with = logger_name
Expand Down