Skip to content

Commit 82feaae

Browse files
ehsannasehsannas
authored
fix: Necessary test improvements for CI environments. (#1673)
* fix: Necessary test improvements for CI environments. * Address feedback.
1 parent 8981c00 commit 82feaae

File tree

9 files changed

+226
-85
lines changed

9 files changed

+226
-85
lines changed

‎google-cloud-firestore/pom.xml

-6
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,6 @@
248248
<version>1.3.0</version>
249249
<scope>test</scope>
250250
</dependency>
251-
<dependency>
252-
<groupId>com.google.testparameterinjector</groupId>
253-
<artifactId>test-parameter-injector</artifactId>
254-
<version>1.15</version>
255-
<scope>test</scope>
256-
</dependency>
257251
<!-- END Cloud Ops -->
258252
</dependencies>
259253

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java

+21-34
Original file line numberDiff line numberDiff line change
@@ -1648,14 +1648,13 @@ private void internalStream(
16481648
request.setReadTime(readTime.toProto());
16491649
}
16501650

1651-
traceUtil
1652-
.currentSpan()
1653-
.addEvent(
1654-
TraceUtil.SPAN_NAME_RUN_QUERY,
1655-
new ImmutableMap.Builder<String, Object>()
1656-
.put("isTransactional", transactionId != null)
1657-
.put("isRetryRequestWithCursor", isRetryRequestWithCursor)
1658-
.build());
1651+
TraceUtil.Span currentSpan = traceUtil.currentSpan();
1652+
currentSpan.addEvent(
1653+
TraceUtil.SPAN_NAME_RUN_QUERY,
1654+
new ImmutableMap.Builder<String, Object>()
1655+
.put("isTransactional", transactionId != null)
1656+
.put("isRetryRequestWithCursor", isRetryRequestWithCursor)
1657+
.build());
16591658

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

@@ -1676,18 +1675,13 @@ public void onStart(StreamController streamController) {}
16761675
public void onResponse(RunQueryResponse response) {
16771676
if (!firstResponse) {
16781677
firstResponse = true;
1679-
traceUtil.currentSpan().addEvent(TraceUtil.SPAN_NAME_RUN_QUERY + ": First Response");
1678+
currentSpan.addEvent(TraceUtil.SPAN_NAME_RUN_QUERY + ": First Response");
16801679
}
16811680
if (response.hasDocument()) {
16821681
numDocuments++;
16831682
if (numDocuments % NUM_RESPONSES_PER_TRACE_EVENT == 0) {
1684-
traceUtil
1685-
.currentSpan()
1686-
.addEvent(
1687-
TraceUtil.SPAN_NAME_RUN_QUERY
1688-
+ ": Received "
1689-
+ numDocuments
1690-
+ " documents");
1683+
currentSpan.addEvent(
1684+
TraceUtil.SPAN_NAME_RUN_QUERY + ": Received " + numDocuments + " documents");
16911685
}
16921686
Document document = response.getDocument();
16931687
QueryDocumentSnapshot documentSnapshot =
@@ -1702,9 +1696,8 @@ public void onResponse(RunQueryResponse response) {
17021696
}
17031697

17041698
if (response.getDone()) {
1705-
traceUtil
1706-
.currentSpan()
1707-
.addEvent(TraceUtil.SPAN_NAME_RUN_QUERY + ": Received RunQueryResponse.Done");
1699+
currentSpan.addEvent(
1700+
TraceUtil.SPAN_NAME_RUN_QUERY + ": Received RunQueryResponse.Done");
17081701
onComplete();
17091702
}
17101703
}
@@ -1713,11 +1706,9 @@ public void onResponse(RunQueryResponse response) {
17131706
public void onError(Throwable throwable) {
17141707
QueryDocumentSnapshot cursor = lastReceivedDocument.get();
17151708
if (shouldRetry(cursor, throwable)) {
1716-
traceUtil
1717-
.currentSpan()
1718-
.addEvent(
1719-
TraceUtil.SPAN_NAME_RUN_QUERY + ": Retryable Error",
1720-
Collections.singletonMap("error.message", throwable.getMessage()));
1709+
currentSpan.addEvent(
1710+
TraceUtil.SPAN_NAME_RUN_QUERY + ": Retryable Error",
1711+
Collections.singletonMap("error.message", throwable.getMessage()));
17211712

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

17311722
} else {
1732-
traceUtil
1733-
.currentSpan()
1734-
.addEvent(
1735-
TraceUtil.SPAN_NAME_RUN_QUERY + ": Error",
1736-
Collections.singletonMap("error.message", throwable.getMessage()));
1723+
currentSpan.addEvent(
1724+
TraceUtil.SPAN_NAME_RUN_QUERY + ": Error",
1725+
Collections.singletonMap("error.message", throwable.getMessage()));
17371726
documentObserver.onError(throwable);
17381727
}
17391728
}
@@ -1742,11 +1731,9 @@ public void onError(Throwable throwable) {
17421731
public void onComplete() {
17431732
if (hasCompleted) return;
17441733
hasCompleted = true;
1745-
traceUtil
1746-
.currentSpan()
1747-
.addEvent(
1748-
TraceUtil.SPAN_NAME_RUN_QUERY + ": Completed",
1749-
Collections.singletonMap("numDocuments", numDocuments));
1734+
currentSpan.addEvent(
1735+
TraceUtil.SPAN_NAME_RUN_QUERY + ": Completed",
1736+
Collections.singletonMap("numDocuments", numDocuments));
17501737
documentObserver.onCompleted(readTime);
17511738
}
17521739

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTest.java

+2-11
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@
6262
import com.google.common.base.Preconditions;
6363
import com.google.devtools.cloudtrace.v1.Trace;
6464
import com.google.devtools.cloudtrace.v1.TraceSpan;
65-
import com.google.testing.junit.testparameterinjector.TestParameter;
66-
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
6765
import io.opentelemetry.api.GlobalOpenTelemetry;
6866
import io.opentelemetry.api.trace.Span;
6967
import io.opentelemetry.api.trace.SpanContext;
@@ -96,7 +94,6 @@
9694
import org.junit.Before;
9795
import org.junit.BeforeClass;
9896
import org.junit.Test;
99-
import org.junit.runner.RunWith;
10097

10198
// This End-to-End test verifies Client-side Tracing Functionality instrumented using the
10299
// OpenTelemetry API.
@@ -119,12 +116,8 @@
119116
// 5. Traces are read-back using TraceServiceClient and verified against expected Call Stacks.
120117
// TODO In the future it would be great to have a single test-driver for this test and
121118
// ITTracingTest.
122-
@RunWith(TestParameterInjector.class)
123-
public class ITE2ETracingTest extends ITBaseTest {
124-
125-
protected boolean isUsingGlobalOpenTelemetrySDK() {
126-
return useGlobalOpenTelemetrySDK;
127-
}
119+
public abstract class ITE2ETracingTest extends ITBaseTest {
120+
protected abstract boolean isUsingGlobalOpenTelemetrySDK();
128121

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

274267
private static Firestore firestore;
275268

276-
@TestParameter boolean useGlobalOpenTelemetrySDK;
277-
278269
@BeforeClass
279270
public static void setup() throws IOException {
280271
projectId = FirestoreOptions.getDefaultProjectId();

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestGlobalOtel.java

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright 2024 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.firestore.it;
17+
18+
import org.junit.runner.RunWith;
19+
import org.junit.runners.JUnit4;
20+
21+
@RunWith(JUnit4.class)
22+
public class ITE2ETracingTestGlobalOtel extends ITE2ETracingTest {
23+
@Override
24+
protected boolean isUsingGlobalOpenTelemetrySDK() {
25+
return true;
26+
}
27+
}

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestNonGlobalOtel.java

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright 2024 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.firestore.it;
17+
18+
import org.junit.runner.RunWith;
19+
import org.junit.runners.JUnit4;
20+
21+
@RunWith(JUnit4.class)
22+
public class ITE2ETracingTestNonGlobalOtel extends ITE2ETracingTest {
23+
@Override
24+
protected boolean isUsingGlobalOpenTelemetrySDK() {
25+
return false;
26+
}
27+
}

0 commit comments

Comments
 (0)