test: fix list logs test flakiness #301

simonz130 · 2020-10-27T23:43:09Z

Added monitored resource and improved filtering logic for listing logs. Previously it was flaky due to # of calls to list logs that could (despite filter) be executed too many times, exceeding quota.

Fixes #288
Fixes #271
Fixes #270
Fixes #268
Fixes #264 ☕️

Merge branch 'api-logs-codeowners' of github.com:googleapis/java-logging

yoshi-code-bot

Some suggestions could not be made:

google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
- lines 35-40
google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java
- lines 440-442
- lines 461-465
- lines 681-682
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java
- lines 38-39
- lines 56-58
- lines 75-76
google-cloud-logging/src/main/java/com/google/cloud/logging/Metric.java
- lines 29-30
google-cloud-logging/src/main/java/com/google/cloud/logging/MetricInfo.java
- lines 27-28
google-cloud-logging/src/main/java/com/google/cloud/logging/SinkInfo.java
- lines 458-459
- lines 617-618
google-cloud-logging/src/main/java/com/google/cloud/logging/Synchronicity.java
- lines 20-22
google-cloud-logging/src/main/java/com/google/cloud/logging/spi/v2/LoggingRpc.java
- lines 99-101

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingLevel.java

google-cloud-logging/src/test/java/com/google/cloud/logging/BaseSystemTest.java

google-cloud-logging/src/test/java/com/google/cloud/logging/it/ITLoggingTest.java

yoshi-code-bot

Some suggestions could not be made:

google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
- lines 35-40
google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java
- lines 440-442
- lines 461-465
- lines 681-682
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java
- lines 38-39
- lines 56-58
- lines 75-76
google-cloud-logging/src/main/java/com/google/cloud/logging/Metric.java
- lines 29-30
google-cloud-logging/src/main/java/com/google/cloud/logging/MetricInfo.java
- lines 27-28
google-cloud-logging/src/main/java/com/google/cloud/logging/SinkInfo.java
- lines 458-459
- lines 617-618
google-cloud-logging/src/main/java/com/google/cloud/logging/Synchronicity.java
- lines 20-22
google-cloud-logging/src/main/java/com/google/cloud/logging/spi/v2/LoggingRpc.java
- lines 99-101

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingLevel.java

google-cloud-logging/src/test/java/com/google/cloud/logging/BaseSystemTest.java

google-cloud-logging/src/test/java/com/google/cloud/logging/it/ITLoggingTest.java

chingor13

please run mvn com.coveo:fmt-maven-plugin:format. I'm not sure what's going on with the bot suggestions.

gcf-merge-on-green · 2020-10-28T02:44:02Z

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

gcf-merge-on-green · 2020-10-28T02:44:02Z

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

gcf-merge-on-green · 2020-10-28T05:44:02Z

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

gcf-merge-on-green · 2020-10-28T05:44:02Z

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

samples/snippets/src/main/java/com/example/logging/ListLogs.java

freelerobot

Are there Devsite docs that contain these samples with mentions of "stackdriver"? If so, you might want to request changes there as well if you haven't done so already.

daniel-sanche · 2020-10-28T17:27:36Z

google-cloud-logging/src/test/java/com/google/cloud/logging/BaseSystemTest.java

+    String monitoredResourceFilter =
+        appendResourceTypeFilter(filter.toString(), monitoredResources);
+
+    Logging.EntryListOption[] options = {Logging.EntryListOption.filter(monitoredResourceFilter)};


Do you think we should change Logging.EntryListOption.filter to use a 1 day timestamp filter by default? That's what I did in Python. It seems like a bad developer experience to hang on this function using the default options

That's what I did here too - even a bit more restrictive filter of 1 hour back instead of 24 hours back. If you look at line 106 - I call createTimestampFilter(1) to restrict logs range, then on line 110 I add resourceType filter to further restrict the filter on top of time range.

It looks like you changed the test, but should we change the function itself too? (unless I missed that)

simonz130 · 2020-10-28T17:40:57Z

Are there Devsite docs that contain these samples with mentions of "stackdriver"? If so, you might want to request changes there as well if you haven't done so already.

There is a bot that will open a bunch of bugs to verify that all docs pages that include the sample are "OK" with it. I'll verify them as we go.

codecov · 2020-10-28T19:39:01Z

Codecov Report

Merging #301 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #301   +/-   ##
=========================================
  Coverage     75.34%   75.34%           
  Complexity      593      593           
=========================================
  Files            42       42           
  Lines          3715     3715           
  Branches        259      259           
=========================================
  Hits           2799     2799           
  Misses          762      762           
  Partials        154      154

Impacted Files	Coverage Δ	Complexity Δ
...c/main/java/com/google/cloud/logging/LogEntry.java	`90.45% <ø> (ø)`	`51.00 <0.00> (ø)`
...rc/main/java/com/google/cloud/logging/Logging.java	`100.00% <ø> (ø)`	`0.00 <0.00> (ø)`
.../java/com/google/cloud/logging/LoggingHandler.java	`73.63% <ø> (ø)`	`22.00 <0.00> (ø)`
...in/java/com/google/cloud/logging/LoggingLevel.java	`100.00% <ø> (ø)`	`3.00 <0.00> (ø)`
...src/main/java/com/google/cloud/logging/Metric.java	`91.89% <ø> (ø)`	`15.00 <0.00> (ø)`
...main/java/com/google/cloud/logging/MetricInfo.java	`90.00% <ø> (ø)`	`16.00 <0.00> (ø)`
...g/src/main/java/com/google/cloud/logging/Sink.java	`89.74% <ø> (ø)`	`14.00 <0.00> (ø)`
...c/main/java/com/google/cloud/logging/SinkInfo.java	`80.69% <ø> (ø)`	`19.00 <0.00> (ø)`
...n/java/com/google/cloud/logging/Synchronicity.java	`100.00% <ø> (ø)`	`1.00 <0.00> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 453cc05...7ce4515. Read the comment docs.

Simon Zeltser added 5 commits September 23, 2020 18:32

Debrand Cloud Logging library from Stackdriver

08cc05c

Add cdpe ops team to codeowners

b16ac73

chore: add cdpe-ops team to codeowners

2a42272

Merge branch 'api-logs-codeowners' of github.com:googleapis/java-logging

Merge branch 'master' of github.com:simonz130/java-logging

ff00caa

fix: add monitored resource filter to flaky tests

f6270f7

simonz130 requested a review from chingor13 October 27, 2020 23:43

simonz130 requested a review from a team as a code owner October 27, 2020 23:43

simonz130 requested a review from a team October 27, 2020 23:43

simonz130 requested a review from a team as a code owner October 27, 2020 23:43

simonz130 self-assigned this Oct 27, 2020

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 27, 2020

Merge branch 'master' into fixListLogsFlackiness

9437ea9

yoshi-code-bot reviewed Oct 27, 2020

View reviewed changes

chingor13 approved these changes Oct 28, 2020

View reviewed changes

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 28, 2020

chore: run formatter

950f51c

chingor13 added the automerge Merge the pull request once unit tests and other checks pass. label Oct 28, 2020

chingor13 changed the title ~~fix: list logs test flackiness~~ test: fix list logs test flackiness Oct 28, 2020

freelerobot reviewed Oct 28, 2020

View reviewed changes

samples/snippets/src/main/java/com/example/logging/ListLogs.java Show resolved Hide resolved

freelerobot approved these changes Oct 28, 2020

View reviewed changes

freelerobot changed the title ~~test: fix list logs test flackiness~~ test: fix list logs test flakiness Oct 28, 2020

daniel-sanche reviewed Oct 28, 2020

View reviewed changes

fix: updated listlogs test for samples

e9e97ba

simonz130 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2020

simonz130 added 5 commits October 28, 2020 11:51

fix: add calendar import

34ade9d

fix: rfc_3339

e0103ba

fix typo

dc4e192

fix style

a4827b0

fix: lint, imports

7ce4515

simonz130 merged commit 024b73d into googleapis:master Oct 28, 2020

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: fix list logs test flakiness #301

test: fix list logs test flakiness #301

simonz130 commented Oct 27, 2020 •

edited by chingor13

Loading

yoshi-code-bot left a comment

yoshi-code-bot left a comment

chingor13 left a comment

gcf-merge-on-green bot commented Oct 28, 2020

gcf-merge-on-green bot commented Oct 28, 2020

gcf-merge-on-green bot commented Oct 28, 2020

gcf-merge-on-green bot commented Oct 28, 2020

freelerobot left a comment

daniel-sanche Oct 28, 2020

simonz130 Oct 28, 2020

daniel-sanche Oct 28, 2020 •

edited

Loading

simonz130 commented Oct 28, 2020

codecov bot commented Oct 28, 2020

test: fix list logs test flakiness #301

test: fix list logs test flakiness #301

Conversation

simonz130 commented Oct 27, 2020 • edited by chingor13 Loading

yoshi-code-bot left a comment

Choose a reason for hiding this comment

yoshi-code-bot left a comment

Choose a reason for hiding this comment

chingor13 left a comment

Choose a reason for hiding this comment

gcf-merge-on-green bot commented Oct 28, 2020

gcf-merge-on-green bot commented Oct 28, 2020

gcf-merge-on-green bot commented Oct 28, 2020

gcf-merge-on-green bot commented Oct 28, 2020

freelerobot left a comment

Choose a reason for hiding this comment

daniel-sanche Oct 28, 2020

Choose a reason for hiding this comment

simonz130 Oct 28, 2020

Choose a reason for hiding this comment

daniel-sanche Oct 28, 2020 • edited Loading

Choose a reason for hiding this comment

simonz130 commented Oct 28, 2020

codecov bot commented Oct 28, 2020

Codecov Report

simonz130 commented Oct 27, 2020 •

edited by chingor13

Loading

daniel-sanche Oct 28, 2020 •

edited

Loading