Skip to content

Commit

Permalink
fix: Necessary test improvements for CI environments. (#1673)
Browse files Browse the repository at this point in the history
* fix: Necessary test improvements for CI environments.

* Address feedback.
  • Loading branch information
ehsannas authored May 4, 2024
1 parent 8981c00 commit 82feaae
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 85 deletions.
6 changes: 0 additions & 6 deletions google-cloud-firestore/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,6 @@
<version>1.3.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.testparameterinjector</groupId>
<artifactId>test-parameter-injector</artifactId>
<version>1.15</version>
<scope>test</scope>
</dependency>
<!-- END Cloud Ops -->
</dependencies>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1648,14 +1648,13 @@ private void internalStream(
request.setReadTime(readTime.toProto());
}

traceUtil
.currentSpan()
.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY,
new ImmutableMap.Builder<String, Object>()
.put("isTransactional", transactionId != null)
.put("isRetryRequestWithCursor", isRetryRequestWithCursor)
.build());
TraceUtil.Span currentSpan = traceUtil.currentSpan();
currentSpan.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY,
new ImmutableMap.Builder<String, Object>()
.put("isTransactional", transactionId != null)
.put("isRetryRequestWithCursor", isRetryRequestWithCursor)
.build());

final AtomicReference<QueryDocumentSnapshot> lastReceivedDocument = new AtomicReference<>();

Expand All @@ -1676,18 +1675,13 @@ public void onStart(StreamController streamController) {}
public void onResponse(RunQueryResponse response) {
if (!firstResponse) {
firstResponse = true;
traceUtil.currentSpan().addEvent(TraceUtil.SPAN_NAME_RUN_QUERY + ": First Response");
currentSpan.addEvent(TraceUtil.SPAN_NAME_RUN_QUERY + ": First Response");
}
if (response.hasDocument()) {
numDocuments++;
if (numDocuments % NUM_RESPONSES_PER_TRACE_EVENT == 0) {
traceUtil
.currentSpan()
.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY
+ ": Received "
+ numDocuments
+ " documents");
currentSpan.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Received " + numDocuments + " documents");
}
Document document = response.getDocument();
QueryDocumentSnapshot documentSnapshot =
Expand All @@ -1702,9 +1696,8 @@ public void onResponse(RunQueryResponse response) {
}

if (response.getDone()) {
traceUtil
.currentSpan()
.addEvent(TraceUtil.SPAN_NAME_RUN_QUERY + ": Received RunQueryResponse.Done");
currentSpan.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Received RunQueryResponse.Done");
onComplete();
}
}
Expand All @@ -1713,11 +1706,9 @@ public void onResponse(RunQueryResponse response) {
public void onError(Throwable throwable) {
QueryDocumentSnapshot cursor = lastReceivedDocument.get();
if (shouldRetry(cursor, throwable)) {
traceUtil
.currentSpan()
.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Retryable Error",
Collections.singletonMap("error.message", throwable.getMessage()));
currentSpan.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Retryable Error",
Collections.singletonMap("error.message", throwable.getMessage()));

Query.this
.startAfter(cursor)
Expand All @@ -1729,11 +1720,9 @@ public void onError(Throwable throwable) {
/* isRetryRequestWithCursor= */ true);

} else {
traceUtil
.currentSpan()
.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Error",
Collections.singletonMap("error.message", throwable.getMessage()));
currentSpan.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Error",
Collections.singletonMap("error.message", throwable.getMessage()));
documentObserver.onError(throwable);
}
}
Expand All @@ -1742,11 +1731,9 @@ public void onError(Throwable throwable) {
public void onComplete() {
if (hasCompleted) return;
hasCompleted = true;
traceUtil
.currentSpan()
.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Completed",
Collections.singletonMap("numDocuments", numDocuments));
currentSpan.addEvent(
TraceUtil.SPAN_NAME_RUN_QUERY + ": Completed",
Collections.singletonMap("numDocuments", numDocuments));
documentObserver.onCompleted(readTime);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@
import com.google.common.base.Preconditions;
import com.google.devtools.cloudtrace.v1.Trace;
import com.google.devtools.cloudtrace.v1.TraceSpan;
import com.google.testing.junit.testparameterinjector.TestParameter;
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanContext;
Expand Down Expand Up @@ -96,7 +94,6 @@
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;

// This End-to-End test verifies Client-side Tracing Functionality instrumented using the
// OpenTelemetry API.
Expand All @@ -119,12 +116,8 @@
// 5. Traces are read-back using TraceServiceClient and verified against expected Call Stacks.
// TODO In the future it would be great to have a single test-driver for this test and
// ITTracingTest.
@RunWith(TestParameterInjector.class)
public class ITE2ETracingTest extends ITBaseTest {

protected boolean isUsingGlobalOpenTelemetrySDK() {
return useGlobalOpenTelemetrySDK;
}
public abstract class ITE2ETracingTest extends ITBaseTest {
protected abstract boolean isUsingGlobalOpenTelemetrySDK();

// Helper class to track call-stacks in a trace
protected static class TraceContainer {
Expand Down Expand Up @@ -273,8 +266,6 @@ private boolean dfsContainsCallStack(long spanId, List<String> expectedCallStack

private static Firestore firestore;

@TestParameter boolean useGlobalOpenTelemetrySDK;

@BeforeClass
public static void setup() throws IOException {
projectId = FirestoreOptions.getDefaultProjectId();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright 2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.cloud.firestore.it;

import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class ITE2ETracingTestGlobalOtel extends ITE2ETracingTest {
@Override
protected boolean isUsingGlobalOpenTelemetrySDK() {
return true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright 2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.cloud.firestore.it;

import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class ITE2ETracingTestNonGlobalOtel extends ITE2ETracingTest {
@Override
protected boolean isUsingGlobalOpenTelemetrySDK() {
return false;
}
}
Loading

0 comments on commit 82feaae

Please sign in to comment.