diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptions.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptions.java index 12c55011fd17..60be883975d2 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptions.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptions.java @@ -17,9 +17,9 @@ package com.google.cloud.datastore; import com.google.api.core.BetaApi; +import com.google.common.base.Preconditions; import io.opentelemetry.api.OpenTelemetry; import javax.annotation.Nonnull; -import javax.annotation.Nullable; /** * Represents the options that are used to configure the use of OpenTelemetry for telemetry @@ -29,14 +29,15 @@ public class DatastoreOpenTelemetryOptions { private final boolean tracingEnabled; private final boolean metricsEnabled; private final boolean exportBuiltinMetricsToGoogleCloudMonitoring; - private final @Nullable OpenTelemetry openTelemetry; + private final OpenTelemetry openTelemetry; DatastoreOpenTelemetryOptions(Builder builder) { this.tracingEnabled = builder.tracingEnabled; this.metricsEnabled = builder.metricsEnabled; this.exportBuiltinMetricsToGoogleCloudMonitoring = builder.exportBuiltinMetricsToGoogleCloudMonitoring; - this.openTelemetry = builder.openTelemetry; + this.openTelemetry = + builder.openTelemetry == null ? OpenTelemetry.noop() : builder.openTelemetry; } /** @@ -88,11 +89,13 @@ public boolean isExportBuiltinMetricsToGoogleCloudMonitoring() { } /** - * Returns the custom {@link OpenTelemetry} instance, if one was provided. + * Returns the configured custom {@link OpenTelemetry} instance. * - * @return the custom {@link OpenTelemetry} instance, or {@code null} if none was provided. + * @return the configured {@link OpenTelemetry} instance, or the global instance if a custom one + * was not provided. If there is no global instance, then {@code OpenTelemetry.noop()} is + * returned. */ - @Nullable + @Nonnull public OpenTelemetry getOpenTelemetry() { return openTelemetry; } @@ -123,7 +126,7 @@ public static class Builder { private boolean metricsEnabled; private boolean exportBuiltinMetricsToGoogleCloudMonitoring; - @Nullable private OpenTelemetry openTelemetry; + private OpenTelemetry openTelemetry; private Builder() { tracingEnabled = false; @@ -159,8 +162,8 @@ public DatastoreOpenTelemetryOptions.Builder setTracingEnabled(boolean enabled) * @param enabled Whether metrics should be enabled. * @return this builder instance. */ - @Nonnull - DatastoreOpenTelemetryOptions.Builder setMetricsEnabled(boolean enabled) { + @BetaApi + public DatastoreOpenTelemetryOptions.Builder setMetricsEnabled(boolean enabled) { this.metricsEnabled = enabled; return this; } @@ -193,6 +196,7 @@ public DatastoreOpenTelemetryOptions.Builder setExportBuiltinMetricsToGoogleClou @Nonnull public DatastoreOpenTelemetryOptions.Builder setOpenTelemetry( @Nonnull OpenTelemetry openTelemetry) { + Preconditions.checkNotNull(openTelemetry, "OpenTelemetry instance cannot be null"); this.openTelemetry = openTelemetry; return this; } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java index b3eda8632be9..ac755ddb0f81 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java @@ -122,7 +122,7 @@ static DatastoreMetricsRecorder getInstance( // Note: Metrics will not be sent if an emulator is enabled. if (otelOptions.isMetricsEnabled()) { OpenTelemetry customOtel = otelOptions.getOpenTelemetry(); - if (customOtel == null) { + if (customOtel.getMeterProvider() == OpenTelemetry.noop().getMeterProvider()) { customOtel = GlobalOpenTelemetry.get(); } recorders.add( diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/EnabledTraceUtil.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/EnabledTraceUtil.java index 3dc8395d170b..df79a922ba48 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/EnabledTraceUtil.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/EnabledTraceUtil.java @@ -22,9 +22,6 @@ import com.google.api.core.ApiFutures; import com.google.api.core.InternalApi; import com.google.cloud.datastore.DatastoreOptions; -import com.google.cloud.datastore.telemetry.TraceUtil.Context; -import com.google.cloud.datastore.telemetry.TraceUtil.Scope; -import com.google.cloud.datastore.telemetry.TraceUtil.Span; import com.google.common.base.Throwables; import io.grpc.ManagedChannelBuilder; import io.opentelemetry.api.GlobalOpenTelemetry; @@ -51,16 +48,12 @@ public class EnabledTraceUtil implements TraceUtil { private final DatastoreOptions datastoreOptions; EnabledTraceUtil(DatastoreOptions datastoreOptions) { - OpenTelemetry openTelemetry = datastoreOptions.getOpenTelemetryOptions().getOpenTelemetry(); - - // If tracing is enabled, but an OpenTelemetry instance is not provided, fall back - // to using GlobalOpenTelemetry. - if (openTelemetry == null) { - openTelemetry = GlobalOpenTelemetry.get(); - } - this.datastoreOptions = datastoreOptions; - this.openTelemetry = openTelemetry; + OpenTelemetry otel = datastoreOptions.getOpenTelemetryOptions().getOpenTelemetry(); + if (otel.getTracerProvider() == TracerProvider.noop()) { + otel = GlobalOpenTelemetry.get(); + } + this.openTelemetry = otel; this.tracer = openTelemetry.getTracer(LIBRARY_NAME); } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptionsTestHelper.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptionsTestHelper.java deleted file mode 100644 index 5c6d138d47b7..000000000000 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOpenTelemetryOptionsTestHelper.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2026 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.datastore; - -import io.opentelemetry.api.OpenTelemetry; - -// TODO(lawrenceqiu): This is a temporary class used to enabled metrics while `setMetricsEnabled` -// is package private. This is to be removed later. -public class DatastoreOpenTelemetryOptionsTestHelper { - public static DatastoreOpenTelemetryOptions withMetricsEnabled(OpenTelemetry openTelemetry) { - return DatastoreOpenTelemetryOptions.newBuilder() - .setMetricsEnabled(true) - .setOpenTelemetry(openTelemetry) - .build(); - } - - public static DatastoreOpenTelemetryOptions withMetricsEnabled() { - return DatastoreOpenTelemetryOptions.newBuilder().setMetricsEnabled(true).build(); - } -} diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index 930526a88788..e3c55b349c49 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; @@ -159,6 +160,45 @@ public void testTelemetrySignalsMixedEnabled() { assertTrue(o2.isEnabled()); } + @Test + public void testOpenTelemetryMetricsAndCloudMonitoringMixed() { + DatastoreOpenTelemetryOptions o1 = + DatastoreOpenTelemetryOptions.newBuilder() + .setMetricsEnabled(true) + .setExportBuiltinMetricsToGoogleCloudMonitoring(false) + .build(); + assertTrue(o1.isMetricsEnabled()); + assertFalse(o1.isExportBuiltinMetricsToGoogleCloudMonitoring()); + assertTrue(o1.isEnabled()); + + DatastoreOpenTelemetryOptions o2 = + DatastoreOpenTelemetryOptions.newBuilder() + .setMetricsEnabled(false) + .setExportBuiltinMetricsToGoogleCloudMonitoring(true) + .build(); + assertFalse(o2.isMetricsEnabled()); + assertTrue(o2.isExportBuiltinMetricsToGoogleCloudMonitoring()); + assertFalse(o2.isEnabled()); + } + + @Test + public void testOpenTelemetryOptionsDefaultInstance() { + DatastoreOpenTelemetryOptions telemetryOptions = + DatastoreOpenTelemetryOptions.newBuilder().build(); + assertThat(telemetryOptions.getOpenTelemetry()) + .isSameInstanceAs(io.opentelemetry.api.OpenTelemetry.noop()); + } + + @Test + public void testOpenTelemetryOptionsSetNullThrowsNPE() { + try { + DatastoreOpenTelemetryOptions.newBuilder().setOpenTelemetry(null); + fail("Expected NullPointerException"); + } catch (NullPointerException e) { + assertThat(e.getMessage()).isEqualTo("OpenTelemetry instance cannot be null"); + } + } + @Test public void testNamespace() { assertTrue(options.build().getNamespace().isEmpty()); diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/ITDatastoreBuiltInAndCustomMetrics.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java similarity index 97% rename from java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/ITDatastoreBuiltInAndCustomMetrics.java rename to java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java index 655095202176..2cfd1036ec6d 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/ITDatastoreBuiltInAndCustomMetrics.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java @@ -14,13 +14,18 @@ * limitations under the License. */ -package com.google.cloud.datastore; +package com.google.cloud.datastore.it; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static org.junit.Assume.assumeNotNull; import com.google.cloud.TransportOptions; +import com.google.cloud.datastore.Datastore; +import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; +import com.google.cloud.datastore.DatastoreOptions; +import com.google.cloud.datastore.Entity; +import com.google.cloud.datastore.Key; import com.google.cloud.datastore.telemetry.TelemetryConstants; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; @@ -59,16 +64,15 @@ */ @RunWith(Parameterized.class) @SuppressWarnings("checkstyle:abbreviationaswordinname") -public class ITDatastoreBuiltInAndCustomMetrics { +public class ITDatastoreClientSideMetrics { private static final String PROJECT_ID = System.getenv("GOOGLE_CLOUD_PROJECT"); private static final String DATABASE_ID = System.getenv().getOrDefault("DATASTORE_DATABASE_ID", ""); - private boolean isDatastoreClosed = false; private final TransportOptions transportOptions; - public ITDatastoreBuiltInAndCustomMetrics(TransportOptions transportOptions) { + public ITDatastoreClientSideMetrics(TransportOptions transportOptions) { this.transportOptions = transportOptions; } @@ -134,7 +138,7 @@ public void setUp() { @After public void tearDown() throws Exception { - if (datastore != null && !isDatastoreClosed) { + if (datastore != null) { Key key = datastore.newKeyFactory().setKind(kind).newKey("metrics-it-entity"); try { datastore.delete(key); diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java index f495e0dfa7c9..71390957a04d 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java @@ -109,4 +109,45 @@ public void openTelemetryRecorderCreatedWithExplicitOpenTelemetry() { assertThat(recorder.getOpenTelemetry()).isSameInstanceAs(openTelemetry); } + + @Test + public void builtInDisabledAndCustomEnabled_returnsOneRecorder() { + DatastoreOptions options = + baseOptions() + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptions.newBuilder() + .setExportBuiltinMetricsToGoogleCloudMonitoring(false) + .setMetricsEnabled(true) + .setOpenTelemetry(OpenTelemetry.noop()) + .build()) + .build(); + OpenTelemetry builtInOtel = EasyMock.createMock(OpenTelemetry.class); + EasyMock.replay(builtInOtel); + + DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options, builtInOtel); + assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class); + CompositeDatastoreMetricsRecorder compositeRecorder = + (CompositeDatastoreMetricsRecorder) recorder; + assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(1); + } + + @Test + public void bothEnabled_returnsTwoRecorders() { + DatastoreOptions options = + baseOptions() + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptions.newBuilder() + .setExportBuiltinMetricsToGoogleCloudMonitoring(true) + .setMetricsEnabled(true) + .setOpenTelemetry(OpenTelemetry.noop()) + .build()) + .build(); + OpenTelemetry builtInOtel = OpenTelemetry.noop(); + + DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options, builtInOtel); + assertThat(recorder).isInstanceOf(CompositeDatastoreMetricsRecorder.class); + CompositeDatastoreMetricsRecorder compositeRecorder = + (CompositeDatastoreMetricsRecorder) recorder; + assertThat(compositeRecorder.getMetricRecorders().size()).isEqualTo(2); + } }