diff --git a/google-cloud-firestore/pom.xml b/google-cloud-firestore/pom.xml index 742b2ee6a..becd396d4 100644 --- a/google-cloud-firestore/pom.xml +++ b/google-cloud-firestore/pom.xml @@ -248,12 +248,6 @@ 1.3.0 test - - com.google.testparameterinjector - test-parameter-injector - 1.15 - test - diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java index ceea15f61..2cda535fa 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java @@ -1648,14 +1648,13 @@ private void internalStream( request.setReadTime(readTime.toProto()); } - traceUtil - .currentSpan() - .addEvent( - TraceUtil.SPAN_NAME_RUN_QUERY, - new ImmutableMap.Builder() - .put("isTransactional", transactionId != null) - .put("isRetryRequestWithCursor", isRetryRequestWithCursor) - .build()); + TraceUtil.Span currentSpan = traceUtil.currentSpan(); + currentSpan.addEvent( + TraceUtil.SPAN_NAME_RUN_QUERY, + new ImmutableMap.Builder() + .put("isTransactional", transactionId != null) + .put("isRetryRequestWithCursor", isRetryRequestWithCursor) + .build()); final AtomicReference lastReceivedDocument = new AtomicReference<>(); @@ -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 = @@ -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(); } } @@ -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) @@ -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); } } @@ -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); } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTest.java index f782a18a2..9dbb4c3b8 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTest.java @@ -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; @@ -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. @@ -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 { @@ -273,8 +266,6 @@ private boolean dfsContainsCallStack(long spanId, List expectedCallStack private static Firestore firestore; - @TestParameter boolean useGlobalOpenTelemetrySDK; - @BeforeClass public static void setup() throws IOException { projectId = FirestoreOptions.getDefaultProjectId(); diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestGlobalOtel.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestGlobalOtel.java new file mode 100644 index 000000000..85bfb5437 --- /dev/null +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestGlobalOtel.java @@ -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; + } +} diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestNonGlobalOtel.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestNonGlobalOtel.java new file mode 100644 index 000000000..21272ff2e --- /dev/null +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITE2ETracingTestNonGlobalOtel.java @@ -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; + } +} diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTest.java index 0f5f16eb4..9c565b092 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTest.java @@ -41,6 +41,8 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.OpenTelemetrySdkBuilder; +import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; import io.opentelemetry.sdk.trace.SdkTracerProvider; @@ -61,19 +63,21 @@ import java.util.logging.Logger; import javax.annotation.Nullable; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestName; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -@RunWith(JUnit4.class) -public class ITTracingTest { +public abstract class ITTracingTest { + protected abstract boolean isUsingGlobalOpenTelemetrySDK(); + private static final Logger logger = Logger.getLogger(com.google.cloud.firestore.it.ITTracingTest.class.getName()); + private static final int TRACE_FORCE_FLUSH_MILLIS = 1000; + private static final int TRACE_PROVIDER_SHUTDOWN_MILLIS = 1000; + private static final int IN_MEMORY_SPAN_EXPORTER_DELAY_MILLIS = 50; private static final String SERVICE = "google.firestore.v1.Firestore/"; private static final String BATCH_GET_DOCUMENTS_RPC_NAME = "BatchGetDocuments"; private static final String COMMIT_RPC_NAME = "Commit"; @@ -85,9 +89,11 @@ public class ITTracingTest { private static final String BEGIN_TRANSACTION_RPC_NAME = "BeginTransaction"; private static final String ROLLBACK_RPC_NAME = "Rollback"; + private static OpenTelemetrySdk openTelemetrySdk; + // We use an InMemorySpanExporter for testing which keeps all generated trace spans // in memory so that we can check their correctness. - protected static InMemorySpanExporter inMemorySpanExporter = InMemorySpanExporter.create(); + protected InMemorySpanExporter inMemorySpanExporter; protected Firestore firestore; @@ -97,35 +103,48 @@ public class ITTracingTest { @Rule public TestName testName = new TestName(); - @BeforeClass - public static void beforeAll() { - // Create Firestore with an in-memory span exporter. - GlobalOpenTelemetry.resetForTest(); + @Before + public void before() { + inMemorySpanExporter = InMemorySpanExporter.create(); + Resource resource = Resource.getDefault().merge(Resource.builder().put(SERVICE_NAME, "Sparky").build()); SpanProcessor inMemorySpanProcessor = SimpleSpanProcessor.create(inMemorySpanExporter); - OpenTelemetrySdk.builder() - .setTracerProvider( - SdkTracerProvider.builder() - .setResource(resource) - .addSpanProcessor(inMemorySpanProcessor) - .setSampler(Sampler.alwaysOn()) - .build()) - .buildAndRegisterGlobal(); - } + FirestoreOptions.Builder optionsBuilder = FirestoreOptions.newBuilder(); + FirestoreOpenTelemetryOptions.Builder otelOptionsBuilder = + FirestoreOpenTelemetryOptions.newBuilder(); + OpenTelemetrySdkBuilder openTelemetrySdkBuilder = + OpenTelemetrySdk.builder() + .setTracerProvider( + SdkTracerProvider.builder() + .setResource(resource) + .addSpanProcessor(inMemorySpanProcessor) + .setSampler(Sampler.alwaysOn()) + .build()); + + if (isUsingGlobalOpenTelemetrySDK()) { + GlobalOpenTelemetry.resetForTest(); + openTelemetrySdk = openTelemetrySdkBuilder.buildAndRegisterGlobal(); + optionsBuilder.setOpenTelemetryOptions(otelOptionsBuilder.setTracingEnabled(true).build()); + } else { + openTelemetrySdk = openTelemetrySdkBuilder.build(); + optionsBuilder.setOpenTelemetryOptions( + otelOptionsBuilder.setTracingEnabled(true).setOpenTelemetry(openTelemetrySdk).build()); + } - @Before - public void before() { - FirestoreOptions.Builder optionsBuilder = - FirestoreOptions.newBuilder() - .setOpenTelemetryOptions( - FirestoreOpenTelemetryOptions.newBuilder().setTracingEnabled(true).build()); String namedDb = System.getProperty("FIRESTORE_NAMED_DATABASE"); if (namedDb != null) { - logger.log(Level.INFO, "Integration test using named database " + namedDb); + logger.log( + Level.INFO, + String.format( + "Integration test using named database %s for test %s", + namedDb, testName.getMethodName())); optionsBuilder = optionsBuilder.setDatabaseId(namedDb); } else { - logger.log(Level.INFO, "Integration test using default database."); + logger.log( + Level.INFO, + String.format( + "Integration test using default database for test %s", testName.getMethodName())); } firestore = optionsBuilder.build().getService(); @@ -144,11 +163,30 @@ public void after() throws Exception { inMemorySpanExporter.reset(); } + @AfterClass + public static void teardown() { + CompletableResultCode completableResultCode = + openTelemetrySdk.getSdkTracerProvider().shutdown(); + completableResultCode.join(TRACE_PROVIDER_SHUTDOWN_MILLIS, TimeUnit.MILLISECONDS); + } + void waitForTracesToComplete() throws Exception { // We need to call `firestore.close()` because that will also close the // gRPC channel and hence force the gRPC instrumentation library to flush // its spans. firestore.close(); + + // The same way that querying the Cloud Trace backend may not give us the + // full trace on the first try, querying the in-memory traces may not result + // in the full trace immediately. Note that performing the `flush` is not + // enough. This doesn't pose an issue in practice, but can make tests flaky. + // Therefore, we're adding a delay to make sure we avoid any flakiness. + inMemorySpanExporter.flush().join(IN_MEMORY_SPAN_EXPORTER_DELAY_MILLIS, TimeUnit.MILLISECONDS); + TimeUnit.MILLISECONDS.sleep(IN_MEMORY_SPAN_EXPORTER_DELAY_MILLIS); + + CompletableResultCode completableResultCode = + openTelemetrySdk.getSdkTracerProvider().forceFlush(); + completableResultCode.join(TRACE_FORCE_FLUSH_MILLIS, TimeUnit.MILLISECONDS); } // Prepares all the spans in memory for inspection. @@ -269,7 +307,7 @@ void assertHasExpectedAttributes(SpanData spanData, String... additionalExpected } } - // Returns true if an only if the given span data contains an event with the given name and the + // Returns true if and only if the given span data contains an event with the given name and the // given expected // attributes. boolean hasEvent(SpanData spanData, String eventName, @Nullable Attributes expectedAttributes) { @@ -277,6 +315,12 @@ boolean hasEvent(SpanData spanData, String eventName, @Nullable Attributes expec return false; } + logger.log( + Level.INFO, + String.format( + "Checking if span named '%s' (ID='%s') contains an event named '%s'", + spanData.getName(), spanData.getSpanId(), eventName)); + List events = spanData.getEvents(); for (EventData event : events) { if (event.getName().equals(eventName)) { @@ -293,10 +337,22 @@ boolean hasEvent(SpanData spanData, String eventName, @Nullable Attributes expec } // This is a POJO used for testing APIs that take a POJO. - static class Pojo { + public static class Pojo { public int bar; - Pojo(int bar) { + public Pojo() { + bar = 0; + } + + public Pojo(int bar) { + this.bar = bar; + } + + public int getBar() { + return bar; + } + + public void setBar(int bar) { this.bar = bar; } } @@ -583,10 +639,7 @@ public void queryGet() throws Exception { .put("isTransactional", false) .build())); assertTrue( - hasEvent( - getGrpcSpanByName(RUN_QUERY_RPC_NAME), - "RunQuery: Completed", - Attributes.builder().put("numDocuments", 0).build())); + hasEvent(span, "RunQuery: Completed", Attributes.builder().put("numDocuments", 0).build())); } @Test diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTestGlobalOtel.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTestGlobalOtel.java new file mode 100644 index 000000000..89495ed50 --- /dev/null +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTestGlobalOtel.java @@ -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 ITTracingTestGlobalOtel extends ITTracingTest { + @Override + protected boolean isUsingGlobalOpenTelemetrySDK() { + return true; + } +} diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTestNonGlobalOtel.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTestNonGlobalOtel.java new file mode 100644 index 000000000..490b68b50 --- /dev/null +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITTracingTestNonGlobalOtel.java @@ -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 ITTracingTestNonGlobalOtel extends ITTracingTest { + @Override + protected boolean isUsingGlobalOpenTelemetrySDK() { + return false; + } +} diff --git a/google-cloud-firestore/src/test/resources/META-INF/native-image/reflect-config.json b/google-cloud-firestore/src/test/resources/META-INF/native-image/reflect-config.json index 066cb670f..87e033f5c 100644 --- a/google-cloud-firestore/src/test/resources/META-INF/native-image/reflect-config.json +++ b/google-cloud-firestore/src/test/resources/META-INF/native-image/reflect-config.json @@ -23,6 +23,14 @@ "methods":[{"name":"","parameterTypes":[] }]} , { +"name": "com.google.cloud.firestore.it.ITTracingTest$Pojo", +"allDeclaredConstructors": true, +"allPublicConstructors": true, +"allDeclaredMethods": true, +"allPublicMethods": true, +"allDeclaredFields": true, +"allPublicFields": true}, +{ "name":"com.google.cloud.firestore.LocalFirestoreHelper$FooList", "methods":[{"name":"","parameterTypes":[] }]} ,