fix: Retrieving logentries pagewise always results in an exception (#…

…1220) * fix: Retrieving logentries pagewise always results in an exception * Address PR comments * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
googleapis · Dec 7, 2022 · 662a439 · 662a439
1 parent f4c346e
commit 662a439
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 19 deletions.
diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java
@@ -1203,7 +1203,8 @@ ApiFuture<AsyncPage<MonitoredResourceDescriptor>> listMonitoredResourceDescripto
  * {@link EntryListOption#pageToken(String)} to specify the page token from which to start listing
  * entries. Use {@link EntryListOption#sortOrder(SortingField, SortingOrder)} to sort log entries
  * according to your preferred order (default is most-recent last). Use {@link
- * EntryListOption#filter(String)} to filter listed log entries.
+ * EntryListOption#filter(String)} to filter listed log entries. By default a 24 hour filter is
+ * applied.
  *
  * <p>Example of listing log entries for a specific log.
  *
@@ -1231,7 +1232,8 @@ ApiFuture<AsyncPage<MonitoredResourceDescriptor>> listMonitoredResourceDescripto
  * specify the page size. Use {@link EntryListOption#pageToken(String)} to specify the page token
  * from which to start listing entries. Use {@link EntryListOption#sortOrder(SortingField,
  * SortingOrder)} to sort log entries according to your preferred order (default is most-recent
- * last). Use {@link EntryListOption#filter(String)} to filter listed log entries.
+ * last). Use {@link EntryListOption#filter(String)} to filter listed log entries. By default a 24
+ * hour filter is applied.
  *
  * <p>Example of asynchronously listing log entries for a specific log.
  *

diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java
@@ -1014,31 +1014,19 @@ static ListLogEntriesRequest listLogEntriesRequest(
  if (orderBy != null) {
  builder.setOrderBy(orderBy);
  }
- String filter = EntryListOption.OptionType.FILTER.get(options);
- // Make sure timestamp filter is either explicitly specified or we add a default
- // time filter
- // of 24 hours back to be inline with gcloud behavior for the same API
+ String filter = generateFilter(EntryListOption.OptionType.FILTER.get(options));
  if (filter != null) {
- if (!Ascii.toLowerCase(filter).contains("timestamp")) {
- filter =
- String.format(
- "%s AND %s", filter, defaultTimestampFilterCreator.createDefaultTimestampFilter());
- }
  builder.setFilter(filter);
- } else {
- // If filter is not specified, default filter is looking back 24 hours in line
- // with gcloud
- // behavior
- builder.setFilter(defaultTimestampFilterCreator.createDefaultTimestampFilter());
  }
-
  return builder.build();
  }
 
  private static ApiFuture<AsyncPage<LogEntry>> listLogEntriesAsync(
  final LoggingOptions serviceOptions, final Map<Option.OptionType, ?> options) {
+ // Make sure to set a filter option which later can be reused in subsequent calls
+ final Map<Option.OptionType, ?> updatedOptions = updateFilter(options);
  final ListLogEntriesRequest request =
- listLogEntriesRequest(serviceOptions.getProjectId(), options);
+ listLogEntriesRequest(serviceOptions.getProjectId(), updatedOptions);
  ApiFuture<ListLogEntriesResponse> list = serviceOptions.getLoggingRpcV2().list(request);
  return transform(
  list,
@@ -1055,7 +1043,7 @@ public AsyncPage<LogEntry> apply(ListLogEntriesResponse listLogEntriesResponse)
  ? null
  : listLogEntriesResponse.getNextPageToken();
  return new AsyncPageImpl<>(
- new LogEntryPageFetcher(serviceOptions, cursor, options), cursor, entries);
+ new LogEntryPageFetcher(serviceOptions, cursor, updatedOptions), cursor, entries);
  }
  });
  }
@@ -1137,6 +1125,37 @@ public void close() throws Exception {
  return optionMap;
  }
 
+ static Map<Option.OptionType, ?> updateFilter(final Map<Option.OptionType, ?> options) {
+ // We should see if filter provided in otiopns have a timestamp parameter
+ // and if not, it should be added with further update of options map.
+ String existingFilter = EntryListOption.OptionType.FILTER.get(options);
+ String newFilter = generateFilter(existingFilter);
+ if (newFilter.equals(existingFilter)) {
+ return options;
+ }
+ // Update
+ Map<Option.OptionType, Object> optionsCopy = Maps.newHashMap(options);
+ optionsCopy.put(EntryListOption.OptionType.FILTER, newFilter);
+ return optionsCopy;
+ }
+
+ static String generateFilter(String filter) {
+ String newFilter = filter;
+ // Make sure timestamp filter is either explicitly specified or we add a default
+ // time filter of 24 hours back to be inline with gcloud behavior for the same API
+ if (newFilter != null) {
+ if (!Ascii.toLowerCase(filter).contains("timestamp")) {
+ newFilter =
+ String.format(
+ "%s AND %s",
+ newFilter, defaultTimestampFilterCreator.createDefaultTimestampFilter());
+ }
+ } else {
+ newFilter = defaultTimestampFilterCreator.createDefaultTimestampFilter();
+ }
+ return newFilter;
+ }
+
  @VisibleForTesting
  int getNumPendingWrites() {
  return pendingWrites.size();

diff --git a/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingTest.java b/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingTest.java
@@ -18,6 +18,9 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
 
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.logging.Logging.EntryListOption;
@@ -27,6 +30,7 @@
 import com.google.cloud.logging.Logging.WriteOption;
 import com.google.common.collect.ImmutableMap;
 import com.google.logging.v2.ListLogEntriesRequest;
+import java.util.Map;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -100,6 +104,18 @@ public void testEntryListOption() {
  "folders/test-folder");
  }
 
+ @Test
+ public void testFilterUpdate() {
+ Map<Option.OptionType, ?> options = LoggingImpl.optionMap(EntryListOption.filter(FILTER));
+ assertThat((String) EntryListOption.OptionType.FILTER.get(options)).isEqualTo(FILTER);
+ Map<Option.OptionType, ?> updated = LoggingImpl.updateFilter(options);
+ assertTrue(((String) EntryListOption.OptionType.FILTER.get(updated)).contains("timestamp"));
+ assertFalse(options == updated);
+ assertNotEquals(EntryListOption.OptionType.FILTER.get(updated), FILTER);
+ Map<Option.OptionType, ?> anotherUpdated = LoggingImpl.updateFilter(updated);
+ assertTrue(anotherUpdated == updated);
+ }
+
  @Test
  public void testWriteOption() {
  WriteOption writeOption = WriteOption.labels(LABELS);