Skip to content

fix(datastore): reduce flakiness of E2E tracing tests #13645

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

Open
lqiu96 wants to merge 4 commits into
base: main
Choose a base branch
from
+39 −17
Open

fix(datastore): reduce flakiness of E2E tracing tests #13645

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e5a18c
fix(datastore): reduce flakiness of E2E tracing tests
lqiu96 Jul 2, 2026
9493964
fix(datastore): use openTelemetrySdk.close() in teardown
lqiu96 Jul 6, 2026
91b9e21
fix(datastore): make tracer and SDK fields non-static in E2E tests
lqiu96 Jul 6, 2026
6d01781
fix(datastore): address PR comments on E2E tracing tests
lqiu96 Jul 6, 2026
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
11 changes: 10 additions & 1 deletion .../google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITE2ETracingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ private boolean dfsContainsCallStack(long spanId, List<String> expectedCallStack

private static final int NUM_SPAN_ID_BYTES = 16;

private static final int GET_TRACE_RETRY_COUNT = 60;
private static final int GET_TRACE_RETRY_COUNT = 120;

@blakeli0 blakeli0 Jul 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that we do Thread.sleep after each unsuccessful check, consider migrate the test to use Awaitility?


private static final int GET_TRACE_RETRY_BACKOFF_MILLIS = 1000;

Expand Down Expand Up @@ -393,6 +393,12 @@ public void after() throws Exception {
tracer = null;
retrievedTrace = null;
customSpanContext = null;
if (openTelemetrySdk != null) {
openTelemetrySdk
.getSdkTracerProvider()
.shutdown()
.join(TRACE_PROVIDER_SHUTDOWN_MILLIS, TimeUnit.MILLISECONDS);
}
Comment thread
lqiu96 marked this conversation as resolved.
Outdated
openTelemetrySdk = null;
}

Expand Down Expand Up @@ -441,6 +447,9 @@ protected void waitForTracesToComplete() throws Exception {
CompletableResultCode completableResultCode =
openTelemetrySdk.getSdkTracerProvider().forceFlush();
completableResultCode.join(TRACE_FORCE_FLUSH_MILLIS, TimeUnit.MILLISECONDS);
if (!completableResultCode.isSuccess()) {
logger.warning("Force flush did not complete successfully");
}
Comment on lines 460 to +465

@gemini-code-assist gemini-code-assist Bot Jul 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If isUsingGlobalOpenTelemetrySDK() is false, openTelemetrySdk will be null, leading to a NullPointerException when calling openTelemetrySdk.getSdkTracerProvider(). Adding a null check and falling back to flushing traceExporter directly ensures robust behavior.

    if (openTelemetrySdk != null) {
      CompletableResultCode completableResultCode =
          openTelemetrySdk.getSdkTracerProvider().forceFlush();
      completableResultCode.join(TRACE_FORCE_FLUSH_MILLIS, TimeUnit.MILLISECONDS);
      if (!completableResultCode.isSuccess()) {
        logger.warning("Force flush did not complete successfully");
      }
    } else if (traceExporter != null) {
      CompletableResultCode completableResultCode = traceExporter.flush();
      completableResultCode.join(TRACE_FORCE_FLUSH_MILLIS, TimeUnit.MILLISECONDS);
      if (!completableResultCode.isSuccess()) {
        logger.warning("Trace exporter flush did not complete successfully");
      }
    }
References
  1. When using lazily initialized resources (such as ExecutorService), ensure that teardown or close methods perform explicit null checks before invoking methods on them to prevent NullPointerException.

}

// Validates `retrievedTrace`. Cloud Trace indexes traces w/ eventual consistency, even when
Expand Down
Loading