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

googleapis · Oct 17, 2022 · e56d3e8 · e56d3e8
1 parent e27da52
commit e56d3e8
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 39 deletions.
diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py
@@ -121,7 +121,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

diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py
@@ -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

diff --git a/google/cloud/logging_v2/logger.py b/google/cloud/logging_v2/logger.py
@@ -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():
@@ -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
@@ -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
@@ -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[:]
diff --git a/tests/unit/test__gapic.py b/tests/unit/test__gapic.py
@@ -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"]

diff --git a/tests/unit/test__http.py b/tests/unit/test__http.py
@@ -306,7 +306,7 @@ def test_write_entries_single(self):
  client = _Client(conn)
  api = self._make_one(client)
 
- api.write_entries([ENTRY])
+ api.write_entries([ENTRY], partial_success=False)
 
  self.assertEqual(conn._called_with["method"], "POST")
  path = f"/{self.WRITE_ENTRIES_PATH}"
@@ -325,7 +325,7 @@ def test_write_entries_multiple(self):
  "resource": RESOURCE,
  "labels": LABELS,
  "entries": [ENTRY1, ENTRY2],
- "partialSuccess": False,
+ "partialSuccess": True,
  "dry_run": False,
  }
  conn = _Connection({})

diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py
@@ -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
@@ -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 (
@@ -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 (
@@ -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
@@ -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 (
@@ -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 (
@@ -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 (
@@ -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
@@ -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):
  """
@@ -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):
  """
@@ -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):
  """
@@ -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):
@@ -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 (
@@ -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
@@ -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 (
@@ -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 (
@@ -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 (
@@ -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
@@ -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)
@@ -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):
@@ -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):
@@ -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):
@@ -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):
@@ -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):
@@ -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):
@@ -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