From 03aabb28995ea3ed98713db1833d41d09cd8a481 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 31 Mar 2026 23:41:25 -0400 Subject: [PATCH 1/9] chore(datastore): Update generated stubs and native image config Update DatastoreAdminStubSettings and DatastoreStubSettings to include the library version via Version.VERSION. Add the two new Version.java files that hold the library version constant. Update native image reflect-config.json for both the admin and core stub packages. --- java-datastore/README.md | 8 +-- .../v1/stub/DatastoreAdminStubSettings.java | 1 + .../datastore/admin/v1/stub/Version.java | 27 ++++++++ .../v1/stub/DatastoreStubSettings.java | 1 + .../cloud/datastore/v1/stub/Version.java | 27 ++++++++ .../reflect-config.json | 63 +++++++++++++++++++ .../reflect-config.json | 63 +++++++++++++++++++ 7 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/Version.java create mode 100644 java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/Version.java diff --git a/java-datastore/README.md b/java-datastore/README.md index 09c3c2fe77c4..96518194208a 100644 --- a/java-datastore/README.md +++ b/java-datastore/README.md @@ -49,20 +49,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.78.0') +implementation platform('com.google.cloud:libraries-bom:26.79.0') implementation 'com.google.cloud:google-cloud-datastore' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-datastore:2.36.0' +implementation 'com.google.cloud:google-cloud-datastore:2.37.0' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.36.0" +libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.37.0" ``` ## Authentication @@ -474,7 +474,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [javadocs]: https://cloud.google.com/java/docs/reference/google-cloud-datastore/latest/history [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-datastore.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.36.0 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.37.0 [authentication]: https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/DatastoreAdminStubSettings.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/DatastoreAdminStubSettings.java index bcc30a0ec207..3ee4ba3a68f0 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/DatastoreAdminStubSettings.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/DatastoreAdminStubSettings.java @@ -410,6 +410,7 @@ protected LibraryMetadata getLibraryMetadata() { return LibraryMetadata.newBuilder() .setArtifactName("com.google.cloud:google-cloud-datastore") .setRepository("googleapis/google-cloud-java") + .setVersion(Version.VERSION) .build(); } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/Version.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/Version.java new file mode 100644 index 000000000000..01384f20a2d0 --- /dev/null +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/admin/v1/stub/Version.java @@ -0,0 +1,27 @@ +/* + * 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 + * + * https://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.admin.v1.stub; + +import com.google.api.core.InternalApi; + +@InternalApi("For internal use only") +final class Version { + // {x-version-update-start:google-cloud-datastore:current} + static final String VERSION = "0.0.0-SNAPSHOT"; + // {x-version-update-end} + +} diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/DatastoreStubSettings.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/DatastoreStubSettings.java index c8f2e8fed41b..66f1ecb4a25e 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/DatastoreStubSettings.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/DatastoreStubSettings.java @@ -297,6 +297,7 @@ protected LibraryMetadata getLibraryMetadata() { return LibraryMetadata.newBuilder() .setArtifactName("com.google.cloud:google-cloud-datastore") .setRepository("googleapis/google-cloud-java") + .setVersion(Version.VERSION) .build(); } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/Version.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/Version.java new file mode 100644 index 000000000000..25bc2b3a32cb --- /dev/null +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/v1/stub/Version.java @@ -0,0 +1,27 @@ +/* + * 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 + * + * https://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.v1.stub; + +import com.google.api.core.InternalApi; + +@InternalApi("For internal use only") +final class Version { + // {x-version-update-start:google-cloud-datastore:current} + static final String VERSION = "0.0.0-SNAPSHOT"; + // {x-version-update-end} + +} diff --git a/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.admin.v1/reflect-config.json b/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.admin.v1/reflect-config.json index f5c285bf778d..e8ab4bbb345d 100644 --- a/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.admin.v1/reflect-config.json +++ b/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.admin.v1/reflect-config.json @@ -1,4 +1,58 @@ [ + { + "name": "com.google.api.BatchingConfigProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingConfigProto$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingDescriptorProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingDescriptorProto$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingSettingsProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingSettingsProto$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, { "name": "com.google.api.ClientLibraryDestination", "queryAllDeclaredConstructors": true, @@ -116,6 +170,15 @@ "allDeclaredClasses": true, "allPublicClasses": true }, + { + "name": "com.google.api.FlowControlLimitExceededBehaviorProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, { "name": "com.google.api.GoSettings", "queryAllDeclaredConstructors": true, diff --git a/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.v1/reflect-config.json b/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.v1/reflect-config.json index 2d53b9a26e97..f119fd95459e 100644 --- a/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.v1/reflect-config.json +++ b/java-datastore/google-cloud-datastore/src/main/resources/META-INF/native-image/com.google.cloud.datastore.v1/reflect-config.json @@ -1,4 +1,58 @@ [ + { + "name": "com.google.api.BatchingConfigProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingConfigProto$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingDescriptorProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingDescriptorProto$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingSettingsProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.BatchingSettingsProto$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, { "name": "com.google.api.ClientLibraryDestination", "queryAllDeclaredConstructors": true, @@ -116,6 +170,15 @@ "allDeclaredClasses": true, "allPublicClasses": true }, + { + "name": "com.google.api.FlowControlLimitExceededBehaviorProto", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, { "name": "com.google.api.GoSettings", "queryAllDeclaredConstructors": true, From 25af60d5e10ce28cc66a32ee99a5a377985fcc12 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 31 Mar 2026 23:48:40 -0400 Subject: [PATCH 2/9] chore(datastore): Rename internal metrics recorder hierarchy and fix gRPC transport coverage Replace the old MetricsRecorder / OpenTelemetryMetricsRecorder / NoOpMetricsRecorder types with the new DatastoreMetricsRecorder family, which extends GAX's MetricsRecorder interface for a unified recording contract. Key changes: - Delete MetricsRecorder.java, OpenTelemetryMetricsRecorder.java, NoOpMetricsRecorder.java and their tests - Add DatastoreMetricsRecorder interface (with simple getInstance() that returns an OTel recorder when metrics are enabled, NoOp otherwise) - Add NoOpDatastoreMetricsRecorder, OpenTelemetryDatastoreMetricsRecorder, and DatastoreMetricsRecorderTest - Remove the !GRPC transport guard from TelemetryUtils.recordOperationMetrics() and attemptMetricsCallable() so all transports record metrics uniformly - Remove the isHttpTransport field from RetryAndTraceDatastoreRpcDecorator and DatastoreImpl; remove buildMetricsTracerFactory() from GrpcDatastoreRpc - Update TelemetryConstants with the new METRIC_PREFIX, DATASTORE_METER_NAME, and typed AttributeKey constants needed by the new recorder classes - Update DatastoreOptions to pass the full DatastoreOptions to getInstance() so the recorder factory can inspect credentials and project at creation time --- .../google/cloud/datastore/DatastoreImpl.java | 44 +++---- .../cloud/datastore/DatastoreOptions.java | 12 +- .../RetryAndTraceDatastoreRpcDecorator.java | 36 ++---- .../datastore/spi/v1/GrpcDatastoreRpc.java | 39 ------ .../telemetry/DatastoreMetricsRecorder.java | 72 +++++++++++ .../datastore/telemetry/MetricsRecorder.java | 74 ----------- ...java => NoOpDatastoreMetricsRecorder.java} | 8 +- ...penTelemetryDatastoreMetricsRecorder.java} | 75 +++-------- .../telemetry/TelemetryConstants.java | 120 ++++++++++++++---- .../datastore/telemetry/TelemetryUtils.java | 57 +++++---- .../datastore/DatastoreImplMetricsTest.java | 29 +---- .../DatastoreMetricsRecorderTest.java | 96 ++++++++++++++ .../telemetry/MetricsRecorderTest.java | 65 ---------- ...elemetryDatastoreMetricsRecorderTest.java} | 16 ++- 14 files changed, 362 insertions(+), 381 deletions(-) create mode 100644 java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java delete mode 100644 java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/MetricsRecorder.java rename java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/{NoOpMetricsRecorder.java => NoOpDatastoreMetricsRecorder.java} (86%) rename java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/{OpenTelemetryMetricsRecorder.java => OpenTelemetryDatastoreMetricsRecorder.java} (53%) create mode 100644 java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java delete mode 100644 java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/MetricsRecorderTest.java rename java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/{OpenTelemetryMetricsRecorderTest.java => OpenTelemetryDatastoreMetricsRecorderTest.java} (94%) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java index d4e64fd0e39b..2a884d5f04bb 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java @@ -48,12 +48,11 @@ import com.google.cloud.TransportOptions; import com.google.cloud.datastore.execution.AggregationQueryExecutor; import com.google.cloud.datastore.spi.v1.DatastoreRpc; -import com.google.cloud.datastore.telemetry.MetricsRecorder; +import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder; import com.google.cloud.datastore.telemetry.TelemetryConstants; import com.google.cloud.datastore.telemetry.TelemetryUtils; import com.google.cloud.datastore.telemetry.TraceUtil; import com.google.cloud.datastore.telemetry.TraceUtil.Scope; -import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; @@ -101,16 +100,14 @@ final class DatastoreImpl extends BaseService implements Datas private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil = getOptions().getTraceUtil(); - private final MetricsRecorder metricsRecorder = getOptions().getMetricsRecorder(); - private final boolean isHttpTransport; - + private final DatastoreMetricsRecorder datastoreMetricsRecorder = + getOptions().getMetricsRecorder(); private final ReadOptionProtoPreparer readOptionProtoPreparer; private final AggregationQueryExecutor aggregationQueryExecutor; DatastoreImpl(DatastoreOptions options) { super(options); this.datastoreRpc = options.getDatastoreRpcV1(); - this.isHttpTransport = options.getTransportOptions() instanceof HttpTransportOptions; retrySettings = MoreObjects.firstNonNull(options.getRetrySettings(), ServiceOptions.getNoRetrySettings()); @@ -122,7 +119,7 @@ final class DatastoreImpl extends BaseService implements Datas .setTraceUtil(otelTraceUtil) .setRetrySettings(retrySettings) .setDatastoreOptions(options) - .setMetricsRecorder(metricsRecorder) + .setMetricsRecorder(datastoreMetricsRecorder) .build(), options); } @@ -185,7 +182,7 @@ public boolean isClosed() { static class ReadWriteTransactionCallable implements Callable { private final Datastore datastore; private final TransactionCallable callable; - private final MetricsRecorder metricsRecorder; + private final DatastoreMetricsRecorder datastoreMetricsRecorder; private volatile TransactionOptions options; private volatile Transaction transaction; @@ -193,11 +190,11 @@ static class ReadWriteTransactionCallable implements Callable { Datastore datastore, TransactionCallable callable, TransactionOptions options, - MetricsRecorder metricsRecorder) { + DatastoreMetricsRecorder datastoreMetricsRecorder) { this.datastore = datastore; this.callable = callable; this.options = options; - this.metricsRecorder = metricsRecorder; + this.datastoreMetricsRecorder = datastoreMetricsRecorder; this.transaction = null; } @@ -222,7 +219,7 @@ public T call() throws DatastoreException { // or from `transaction.commit()`. If there is an exception thrown from either call site, // then the transaction is still active. Check if it is still active (e.g. not commited) // and roll back the transaction. - if (transaction.isActive()) { + if (transaction != null && transaction.isActive()) { transaction.rollback(); } throw DatastoreException.propagateUserException(ex); @@ -231,10 +228,11 @@ public T call() throws DatastoreException { // If the transaction is active, then commit the rollback. If it was already successfully // rolled back, the transaction is inactive (prevents rolling back an already rolled back // transaction). - if (transaction.isActive()) { + if (transaction != null && transaction.isActive()) { transaction.rollback(); } - if (options != null + if (transaction != null + && options != null && options.getModeCase().equals(TransactionOptions.ModeCase.READ_WRITE)) { setPrevTransactionId(transaction.getTransactionId()); } @@ -257,7 +255,7 @@ private void recordAttempt(String status, TransportOptions transportOptions) { attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(transportOptions)); - metricsRecorder.recordTransactionAttemptCount(1, attributes); + datastoreMetricsRecorder.recordTransactionAttemptCount(1, attributes); } } @@ -272,7 +270,8 @@ public T runInTransaction( TraceUtil.Span span = otelTraceUtil.startSpan(SPAN_NAME_TRANSACTION_RUN); ReadWriteTransactionCallable baseCallable = - new ReadWriteTransactionCallable<>(this, callable, transactionOptions, metricsRecorder); + new ReadWriteTransactionCallable<>( + this, callable, transactionOptions, datastoreMetricsRecorder); Callable transactionCallable = baseCallable; if (getOptions().getOpenTelemetryOptions().isTracingEnabled()) { @@ -302,7 +301,7 @@ public T runInTransaction( attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(getOptions().getTransportOptions())); - metricsRecorder.recordTransactionLatency(latencyMs, attributes); + datastoreMetricsRecorder.recordTransactionLatency(latencyMs, attributes); span.end(); } } @@ -805,15 +804,13 @@ private T runWithObservability( ResultRetryAlgorithm exceptionHandler) { com.google.cloud.datastore.telemetry.TraceUtil.Span span = otelTraceUtil.startSpan(spanName); - // Gax already records operation and attempt metrics. Since Datastore HttpJson does not - // integrate with Gax, manually instrument these metrics when using HttpJson for parity - Stopwatch operationStopwatch = isHttpTransport ? Stopwatch.createStarted() : null; + Stopwatch operationStopwatch = Stopwatch.createStarted(); String operationStatus = StatusCode.Code.OK.toString(); DatastoreOptions options = getOptions(); Callable attemptCallable = TelemetryUtils.attemptMetricsCallable( - callable, metricsRecorder, options, isHttpTransport, methodName); + callable, datastoreMetricsRecorder, options, methodName); try (TraceUtil.Scope ignored = span.makeCurrent()) { return RetryHelper.runWithRetries( attemptCallable, retrySettings, exceptionHandler, options.getClock()); @@ -823,12 +820,7 @@ private T runWithObservability( throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - metricsRecorder, - options, - isHttpTransport, - operationStopwatch, - methodName, - operationStatus); + datastoreMetricsRecorder, options, operationStopwatch, methodName, operationStatus); span.end(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 1cd8e4038314..8bd0ea0e93f0 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -31,7 +31,7 @@ import com.google.cloud.datastore.spi.v1.DatastoreRpc; import com.google.cloud.datastore.spi.v1.GrpcDatastoreRpc; import com.google.cloud.datastore.spi.v1.HttpDatastoreRpc; -import com.google.cloud.datastore.telemetry.MetricsRecorder; +import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder; import com.google.cloud.datastore.v1.DatastoreSettings; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; @@ -65,7 +65,7 @@ public class DatastoreOptions extends ServiceOptions { @@ -193,7 +193,7 @@ public Builder setDatabaseId(String databaseId) { } /** - * Sets the {@link DatastoreOpenTelemetryOptions} to be used for this Firestore instance. + * Sets the {@link DatastoreOpenTelemetryOptions} to be used for this Datastore instance. * * @param openTelemetryOptions The `DatastoreOpenTelemetryOptions` to use. */ @@ -223,7 +223,7 @@ private DatastoreOptions(Builder builder) { ? builder.openTelemetryOptions : DatastoreOpenTelemetryOptions.newBuilder().build(); this.traceUtil = com.google.cloud.datastore.telemetry.TraceUtil.getInstance(this); - this.metricsRecorder = MetricsRecorder.getInstance(openTelemetryOptions); + this.datastoreMetricsRecorder = DatastoreMetricsRecorder.getInstance(this); namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace()); databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID); diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java index 397015c5ef4e..1c2f9030c327 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java @@ -24,12 +24,11 @@ import com.google.cloud.RetryHelper; import com.google.cloud.RetryHelper.RetryHelperException; import com.google.cloud.datastore.spi.v1.DatastoreRpc; -import com.google.cloud.datastore.telemetry.MetricsRecorder; -import com.google.cloud.datastore.telemetry.NoOpMetricsRecorder; +import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder; +import com.google.cloud.datastore.telemetry.NoOpDatastoreMetricsRecorder; import com.google.cloud.datastore.telemetry.TelemetryConstants; import com.google.cloud.datastore.telemetry.TelemetryUtils; import com.google.cloud.datastore.telemetry.TraceUtil; -import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; import com.google.datastore.v1.AllocateIdsRequest; @@ -62,8 +61,7 @@ public class RetryAndTraceDatastoreRpcDecorator implements DatastoreRpc { private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil; private final RetrySettings retrySettings; private final DatastoreOptions datastoreOptions; - private final MetricsRecorder metricsRecorder; - private final boolean isHttpTransport; + private final DatastoreMetricsRecorder datastoreMetricsRecorder; @ObsoleteApi("Prefer to create RetryAndTraceDatastoreRpcDecorator via the Builder") public RetryAndTraceDatastoreRpcDecorator( @@ -75,8 +73,7 @@ public RetryAndTraceDatastoreRpcDecorator( this.retrySettings = retrySettings; this.datastoreOptions = datastoreOptions; this.otelTraceUtil = otelTraceUtil; - this.metricsRecorder = new NoOpMetricsRecorder(); - this.isHttpTransport = datastoreOptions.getTransportOptions() instanceof HttpTransportOptions; + this.datastoreMetricsRecorder = new NoOpDatastoreMetricsRecorder(); } private RetryAndTraceDatastoreRpcDecorator(Builder builder) { @@ -84,8 +81,7 @@ private RetryAndTraceDatastoreRpcDecorator(Builder builder) { this.otelTraceUtil = builder.otelTraceUtil; this.retrySettings = builder.retrySettings; this.datastoreOptions = builder.datastoreOptions; - this.metricsRecorder = builder.metricsRecorder; - this.isHttpTransport = builder.isHttpTransport; + this.datastoreMetricsRecorder = builder.datastoreMetricsRecorder; } public static Builder newBuilder() { @@ -99,8 +95,7 @@ public static class Builder { private DatastoreOptions datastoreOptions; // Defaults configured for this class - private MetricsRecorder metricsRecorder = new NoOpMetricsRecorder(); - private boolean isHttpTransport = false; + private DatastoreMetricsRecorder datastoreMetricsRecorder = new NoOpDatastoreMetricsRecorder(); private Builder() {} @@ -124,9 +119,10 @@ public Builder setDatastoreOptions(DatastoreOptions datastoreOptions) { return this; } - public Builder setMetricsRecorder(MetricsRecorder metricsRecorder) { - Preconditions.checkNotNull(metricsRecorder, "metricsRecorder can not be null"); - this.metricsRecorder = metricsRecorder; + public Builder setMetricsRecorder(DatastoreMetricsRecorder datastoreMetricsRecorder) { + Preconditions.checkNotNull( + datastoreMetricsRecorder, "datastoreMetricsRecorder can not be null"); + this.datastoreMetricsRecorder = datastoreMetricsRecorder; return this; } @@ -135,7 +131,6 @@ public RetryAndTraceDatastoreRpcDecorator build() { Preconditions.checkNotNull(otelTraceUtil, "otelTraceUtil is required"); Preconditions.checkNotNull(retrySettings, "retrySettings is required"); Preconditions.checkNotNull(datastoreOptions, "datastoreOptions is required"); - this.isHttpTransport = datastoreOptions.getTransportOptions() instanceof HttpTransportOptions; return new RetryAndTraceDatastoreRpcDecorator(this); } } @@ -207,12 +202,12 @@ public O invokeRpc(Callable block, String startSpan) { O invokeRpc(Callable block, String startSpan, String methodName) { TraceUtil.Span span = otelTraceUtil.startSpan(startSpan); - Stopwatch stopwatch = isHttpTransport ? Stopwatch.createStarted() : null; + Stopwatch stopwatch = Stopwatch.createStarted(); String operationStatus = StatusCode.Code.UNKNOWN.toString(); try (TraceUtil.Scope ignored = span.makeCurrent()) { Callable callable = TelemetryUtils.attemptMetricsCallable( - block, metricsRecorder, datastoreOptions, isHttpTransport, methodName); + block, datastoreMetricsRecorder, datastoreOptions, methodName); O result = RetryHelper.runWithRetries( callable, this.retrySettings, EXCEPTION_HANDLER, this.datastoreOptions.getClock()); @@ -224,12 +219,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - metricsRecorder, - datastoreOptions, - isHttpTransport, - stopwatch, - methodName, - operationStatus); + datastoreMetricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus); span.end(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index cd4d660bc5a0..0947d4a8b4a9 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -31,12 +31,9 @@ import com.google.api.gax.rpc.HeaderProvider; import com.google.api.gax.rpc.NoHeaderProvider; import com.google.api.gax.rpc.TransportChannel; -import com.google.api.gax.tracing.MetricsTracerFactory; -import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; -import com.google.cloud.datastore.telemetry.TelemetryConstants; import com.google.cloud.datastore.v1.DatastoreSettings; import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.datastore.v1.stub.GrpcDatastoreStub; @@ -61,12 +58,8 @@ import io.grpc.CallOptions; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; -import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.api.OpenTelemetry; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; -import java.util.Map; @InternalApi public class GrpcDatastoreRpc implements DatastoreRpc { @@ -95,44 +88,12 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { .build()) .build()); - // Hook into Gax's Metrics collection framework - MetricsTracerFactory metricsTracerFactory = buildMetricsTracerFactory(datastoreOptions); - if (metricsTracerFactory != null) { - builder.setTracerFactory(metricsTracerFactory); - } - datastoreStub = GrpcDatastoreStub.create(builder.build()); } catch (IOException e) { throw new IOException(e); } } - /** - * Build the MetricsTracerFactory to hook into Gax's Otel Framework. Only hooks into Gax on two - * conditions: 1. OpenTelemetry instance is passed in by the user 2. Metrics are enabled - * - *

Sets default attributes to be recorded as part of the metrics. - */ - static MetricsTracerFactory buildMetricsTracerFactory(DatastoreOptions datastoreOptions) { - if (!datastoreOptions.getOpenTelemetryOptions().isMetricsEnabled()) { - return null; - } - OpenTelemetry openTelemetry = datastoreOptions.getOpenTelemetryOptions().getOpenTelemetry(); - if (openTelemetry == null) { - openTelemetry = GlobalOpenTelemetry.get(); - } - OpenTelemetryMetricsRecorder gaxMetricsRecorder = - new OpenTelemetryMetricsRecorder(openTelemetry, TelemetryConstants.SERVICE_NAME); - Map attributes = new HashMap<>(); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, datastoreOptions.getProjectId()); - if (!Strings.isNullOrEmpty(datastoreOptions.getDatabaseId())) { - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, datastoreOptions.getDatabaseId()); - } - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, "grpc"); - return new MetricsTracerFactory(gaxMetricsRecorder, attributes); - } - @Override public void close() throws Exception { if (!closed) { 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 new file mode 100644 index 000000000000..32a9c7cb5ede --- /dev/null +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java @@ -0,0 +1,72 @@ +/* + * 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.telemetry; + +import com.google.api.core.InternalExtensionOnly; +import com.google.api.gax.tracing.MetricsRecorder; +import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; +import com.google.cloud.datastore.DatastoreOptions; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.OpenTelemetry; +import java.util.Map; +import javax.annotation.Nonnull; + +/** + * Interface to record Datastore-specific and standard RPC metrics. + * + *

This interface extends {@link MetricsRecorder} from the GAX library to provide a unified + * recording contract that covers both generic RPC metrics (like latency and attempt counts) and + * Datastore-specific operations (like transactions). + * + *

Warning: This is intended to be an internal API and is not intended for external use. + * This is public solely for implementation purposes and does not promise any backwards + * compatibility. + */ +@InternalExtensionOnly +public interface DatastoreMetricsRecorder extends MetricsRecorder { + + /** Records the total latency of a transaction in milliseconds. */ + void recordTransactionLatency(double latencyMs, Map attributes); + + /** Records the number of attempts a transaction took. */ + void recordTransactionAttemptCount(long count, Map attributes); + + /** + * Returns a {@link DatastoreMetricsRecorder} instance based on the provided {@link + * DatastoreOptions}. + * + *

If the user has enabled metrics and provided an {@link OpenTelemetry} instance (or {@link + * GlobalOpenTelemetry} is used as fallback), an {@link OpenTelemetryDatastoreMetricsRecorder} is + * returned. Otherwise a {@link NoOpDatastoreMetricsRecorder} is returned. + * + * @param datastoreOptions the {@link DatastoreOptions} configuring the Datastore client + * @return a {@link DatastoreMetricsRecorder} for the configured backend + */ + static DatastoreMetricsRecorder getInstance(@Nonnull DatastoreOptions datastoreOptions) { + DatastoreOpenTelemetryOptions otelOptions = datastoreOptions.getOpenTelemetryOptions(); + + if (otelOptions.isMetricsEnabled()) { + OpenTelemetry customOtel = otelOptions.getOpenTelemetry(); + if (customOtel == null) { + customOtel = GlobalOpenTelemetry.get(); + } + return new OpenTelemetryDatastoreMetricsRecorder(customOtel, TelemetryConstants.METRIC_PREFIX); + } + + return new NoOpDatastoreMetricsRecorder(); + } +} diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/MetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/MetricsRecorder.java deleted file mode 100644 index a71cb9b7c25c..000000000000 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/MetricsRecorder.java +++ /dev/null @@ -1,74 +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.telemetry; - -import com.google.api.core.InternalExtensionOnly; -import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; -import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.api.OpenTelemetry; -import java.util.Map; -import javax.annotation.Nonnull; - -/** - * Interface to record specific metric operations. - * - *

Warning: This is intended to be an internal API and is not intended for external use. - * This is public solely for implementation purposes and does not promise any backwards - * compatibility. - */ -@InternalExtensionOnly -public interface MetricsRecorder { - /** Records the total latency of a transaction in milliseconds. */ - void recordTransactionLatency(double latencyMs, Map attributes); - - /** Records the number of attempts a transaction took. */ - void recordTransactionAttemptCount(long count, Map attributes); - - /** Records the latency of a single RPC attempt in milliseconds. */ - void recordAttemptLatency(double latencyMs, Map attributes); - - /** Records the count of a single RPC attempt. */ - void recordAttemptCount(long count, Map attributes); - - /** Records the total latency of an operation (including retries) in milliseconds. */ - void recordOperationLatency(double latencyMs, Map attributes); - - /** Records the count of an operation. */ - void recordOperationCount(long count, Map attributes); - - /** - * Returns a {@link MetricsRecorder} instance based on the provided OpenTelemetry options. - * - * @param options The {@link com.google.cloud.datastore.DatastoreOpenTelemetryOptions} configuring - * telemetry. - * @return An {@link OpenTelemetryMetricsRecorder} if metrics are enabled, otherwise a {@link - * NoOpMetricsRecorder}. - */ - static MetricsRecorder getInstance(@Nonnull DatastoreOpenTelemetryOptions options) { - boolean isMetricsEnabled = options.isMetricsEnabled(); - - if (isMetricsEnabled) { - OpenTelemetry openTelemetry = options.getOpenTelemetry(); - if (openTelemetry == null) { - return new OpenTelemetryMetricsRecorder(GlobalOpenTelemetry.get()); - } - return new OpenTelemetryMetricsRecorder(openTelemetry); - } else { - return new NoOpMetricsRecorder(); - } - } -} diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java similarity index 86% rename from java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpMetricsRecorder.java rename to java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java index 1523e569505f..caa3bc6f1ec5 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java @@ -20,15 +20,17 @@ import java.util.Map; /** - * Metrics recorder implementation, used to stub out metrics instrumentation when metrics are - * disabled. + * A no-op implementation of {@link DatastoreMetricsRecorder}. + * + *

Used to stub out metrics instrumentation when metrics are disabled or when no valid + * recorder could be initialized. * *

WARNING: This class is intended for internal use only. It was made public to be used across * packages as a default. It should not be used by external customers and its API may change without * notice. */ @InternalApi -public class NoOpMetricsRecorder implements MetricsRecorder { +public class NoOpDatastoreMetricsRecorder implements DatastoreMetricsRecorder { @Override public void recordTransactionLatency(double latencyMs, Map attributes) { diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java similarity index 53% rename from java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryMetricsRecorder.java rename to java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java index 6314bbc6413c..8a39db46721c 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java @@ -16,6 +16,7 @@ package com.google.cloud.datastore.telemetry; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; @@ -26,22 +27,32 @@ import javax.annotation.Nonnull; /** - * OpenTelemetry metrics recorder implementation, used to record metrics when metrics are enabled. + * OpenTelemetry implementation for recording Datastore metrics. + * + *

This class follows a two-tier hierarchy: + * + *

    + *
  1. Inheritance (GAX Alignment): It extends {@link OpenTelemetryMetricsRecorder} from + * the GAX library. This allows it to inherit the standardized implementation for common RPC + * metrics like {@code operation_latency} and {@code attempt_count} without duplicating logic. + *
  2. Implementation (Datastore Specifics): It implements {@link DatastoreMetricsRecorder} + * to provide specialized recording for Datastore-only concepts, such as {@code + * transaction_latency} and {@code transaction_attempt_count}. + *
*/ -class OpenTelemetryMetricsRecorder implements MetricsRecorder { +class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder + implements DatastoreMetricsRecorder { + private final OpenTelemetry openTelemetry; private final DoubleHistogram transactionLatency; private final LongCounter transactionAttemptCount; - private final DoubleHistogram attemptLatency; - private final LongCounter attemptCount; - private final DoubleHistogram operationLatency; - private final LongCounter operationCount; - OpenTelemetryMetricsRecorder(@Nonnull OpenTelemetry openTelemetry) { + OpenTelemetryDatastoreMetricsRecorder(@Nonnull OpenTelemetry openTelemetry, String metricPrefix) { + super(openTelemetry, metricPrefix); this.openTelemetry = openTelemetry; - Meter meter = openTelemetry.getMeter(TelemetryConstants.METER_NAME); + Meter meter = openTelemetry.getMeter(TelemetryConstants.DATASTORE_METER_NAME); this.transactionLatency = meter @@ -55,34 +66,6 @@ class OpenTelemetryMetricsRecorder implements MetricsRecorder { .counterBuilder(TelemetryConstants.METRIC_NAME_TRANSACTION_ATTEMPT_COUNT) .setDescription("Number of attempts to commit a transaction") .build(); - - this.attemptLatency = - meter - .histogramBuilder(TelemetryConstants.METRIC_NAME_ATTEMPT_LATENCY) - .setDescription("Latency of a single RPC attempt") - .setUnit("ms") - .build(); - - this.attemptCount = - meter - .counterBuilder(TelemetryConstants.METRIC_NAME_ATTEMPT_COUNT) - .setDescription("Number of RPC attempts") - .setUnit("1") - .build(); - - this.operationLatency = - meter - .histogramBuilder(TelemetryConstants.METRIC_NAME_OPERATION_LATENCY) - .setDescription("Total latency of an operation including retries") - .setUnit("ms") - .build(); - - this.operationCount = - meter - .counterBuilder(TelemetryConstants.METRIC_NAME_OPERATION_COUNT) - .setDescription("Number of operations") - .setUnit("1") - .build(); } OpenTelemetry getOpenTelemetry() { @@ -99,26 +82,6 @@ public void recordTransactionAttemptCount(long count, Map attrib transactionAttemptCount.add(count, toOtelAttributes(attributes)); } - @Override - public void recordAttemptLatency(double latencyMs, Map attributes) { - attemptLatency.record(latencyMs, toOtelAttributes(attributes)); - } - - @Override - public void recordAttemptCount(long count, Map attributes) { - attemptCount.add(count, toOtelAttributes(attributes)); - } - - @Override - public void recordOperationLatency(double latencyMs, Map attributes) { - operationLatency.record(latencyMs, toOtelAttributes(attributes)); - } - - @Override - public void recordOperationCount(long count, Map attributes) { - operationCount.add(count, toOtelAttributes(attributes)); - } - private static Attributes toOtelAttributes(Map attributes) { AttributesBuilder builder = Attributes.builder(); if (attributes != null) { diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java index cd98de7e28b4..3ce84b5b74c7 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java @@ -17,8 +17,12 @@ package com.google.cloud.datastore.telemetry; import com.google.api.core.InternalApi; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.cloud.TransportOptions; import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.common.collect.ImmutableSet; +import io.opentelemetry.api.common.AttributeKey; +import java.util.Set; /** * Internal telemetry constants shared between OpenTelemetry tracing and metrics. @@ -29,12 +33,86 @@ */ @InternalApi public class TelemetryConstants { + // Built-in metrics constants for Cloud Monitoring export + public static final String METRIC_PREFIX = "custom.googleapis.com/internal/client"; + public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; + public static final String DATASTORE_METER_NAME = "java-datastore"; + + // Monitored resource type for Cloud Monitoring + public static final String DATASTORE_RESOURCE_TYPE = "global"; + + // Resource label keys for the monitored resource + public static final String RESOURCE_LABEL_PROJECT_ID = "project_id"; + public static final String RESOURCE_LABEL_DATABASE_ID = "database_id"; + public static final String RESOURCE_LABEL_LOCATION = "location"; + public static final Set DATASTORE_RESOURCE_LABELS = + ImmutableSet.of(RESOURCE_LABEL_PROJECT_ID); + + // Resource attribute keys (used on OTel Resource) + public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); + public static final AttributeKey DATABASE_ID_KEY = AttributeKey.stringKey("database_id"); + public static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); + + // Metric attribute keys (used on metric data points) + public static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); + public static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); + public static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); + public static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); + public static final AttributeKey DATABASE_KEY = AttributeKey.stringKey("database_id"); + public static final AttributeKey LIBRARY_VERSION_KEY = + AttributeKey.stringKey("library_version"); + public static final AttributeKey TRANSPORT_KEY = AttributeKey.stringKey("transport"); + public static final AttributeKey SERVICE_KEY = AttributeKey.stringKey("service"); + + public static final String SERVICE_VALUE = "datastore.googleapis.com"; - // TODO(lawrenceqiu): For now, use `custom.googleapis.com` until metrics can be written to - // datastore domain - public static final String SERVICE_NAME = "custom.googleapis.com"; - static final String METER_NAME = "com.google.cloud.datastore"; - + /** + * The allowlist of metric attributes that are permitted on every exported data point. + * + *

Cloud Monitoring is strict about label schemas: exporting a label that was not present when + * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to + * fail. Registering OTel Views that filter to this set (see {@link DatastoreBuiltInMetricsView}) + * prevents unexpected labels from leaking into the export and causing silent data loss. + * + *

Using {@code AttributeKey} (wildcard) rather than the raw {@code AttributeKey} type + * preserves generic type safety while still allowing keys of any value type in the set. + */ + public static final Set> COMMON_ATTRIBUTES = + ImmutableSet.of( + CLIENT_UID_KEY, + CLIENT_NAME_KEY, + METHOD_KEY, + STATUS_KEY, + DATABASE_KEY, + LIBRARY_VERSION_KEY, + TRANSPORT_KEY, + SERVICE_KEY); + + // Metric names (short names, without prefix) + public static final String METRIC_NAME_SHORT_OPERATION_LATENCY = "operation_latency"; + public static final String METRIC_NAME_SHORT_ATTEMPT_LATENCY = "attempt_latency"; + public static final String METRIC_NAME_SHORT_OPERATION_COUNT = "operation_count"; + public static final String METRIC_NAME_SHORT_ATTEMPT_COUNT = "attempt_count"; + public static final String METRIC_NAME_SHORT_TRANSACTION_LATENCY = "transaction_latency"; + public static final String METRIC_NAME_SHORT_TRANSACTION_ATTEMPT_COUNT = + "transaction_attempt_count"; + + // Metrics collected at the GAX layer vs Datastore SDK layer + public static final Set GAX_METRICS = + ImmutableSet.of( + METRIC_NAME_SHORT_OPERATION_LATENCY, + METRIC_NAME_SHORT_ATTEMPT_LATENCY, + METRIC_NAME_SHORT_OPERATION_COUNT, + METRIC_NAME_SHORT_ATTEMPT_COUNT); + + public static final Set DATASTORE_METRICS = + ImmutableSet.of( + METRIC_NAME_SHORT_TRANSACTION_LATENCY, METRIC_NAME_SHORT_TRANSACTION_ATTEMPT_COUNT); + + // Environment variable to enable/disable built-in metrics + public static final String ENABLE_METRICS_ENV_VAR = "DATASTORE_ENABLE_METRICS"; + + // Existing attribute key constants (string-based, used by MetricsHelper/TelemetryUtils) public static final String ATTRIBUTES_KEY_DOCUMENT_COUNT = "doc_count"; public static final String ATTRIBUTES_KEY_TRANSACTIONAL = "transactional"; public static final String ATTRIBUTES_KEY_TRANSACTION_ID = "transaction_id"; @@ -62,35 +140,23 @@ public class TelemetryConstants { /** Metric name for the total latency of a transaction. */ public static final String METRIC_NAME_TRANSACTION_LATENCY = - SERVICE_NAME + "/client/transaction_latency"; + METRIC_PREFIX + "/transaction_latency"; /** Metric name for the number of attempts a transaction took. */ public static final String METRIC_NAME_TRANSACTION_ATTEMPT_COUNT = - SERVICE_NAME + "/client/transaction_attempt_count"; + METRIC_PREFIX + "/transaction_attempt_count"; - /** - * Metric name for the total latency of an operation (one full RPC call including retries). Note: - * This does not have the /client prefix to match Gax's format. - */ - public static final String METRIC_NAME_OPERATION_LATENCY = SERVICE_NAME + "/operation_latency"; + /** Metric name for the total latency of an operation (one full RPC call including retries). */ + public static final String METRIC_NAME_OPERATION_LATENCY = METRIC_PREFIX + "/operation_latency"; - /** - * Metric name for the latency of a single RPC attempt. Note: This does not have the /client - * prefix to match Gax's format. - */ - public static final String METRIC_NAME_ATTEMPT_LATENCY = SERVICE_NAME + "/attempt_latency"; + /** Metric name for the latency of a single RPC attempt. */ + public static final String METRIC_NAME_ATTEMPT_LATENCY = METRIC_PREFIX + "/attempt_latency"; - /** - * Metric name for the count of operations. Note: This does not have the /client prefix to match - * Gax's format. - */ - public static final String METRIC_NAME_OPERATION_COUNT = SERVICE_NAME + "/operation_count"; + /** Metric name for the count of operations. */ + public static final String METRIC_NAME_OPERATION_COUNT = METRIC_PREFIX + "/operation_count"; - /** - * Metric name for the count of RPC attempts. Note: This does not have the /client prefix to match - * Gax's format. - */ - public static final String METRIC_NAME_ATTEMPT_COUNT = SERVICE_NAME + "/attempt_count"; + /** Metric name for the count of RPC attempts. */ + public static final String METRIC_NAME_ATTEMPT_COUNT = METRIC_PREFIX + "/attempt_count"; // This is intentionally different from the `SERVICE_NAME` constant as it matches Gax's logic for // method name. diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java index 7a69cb0157d6..c24e63ac3c60 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java @@ -17,6 +17,7 @@ package com.google.cloud.datastore.telemetry; import com.google.api.core.InternalApi; +import com.google.api.gax.core.GaxProperties; import com.google.api.gax.rpc.StatusCode; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; @@ -54,65 +55,64 @@ public static Map buildMetricAttributes( attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(datastoreOptions.getTransportOptions())); + attributes.put( + TelemetryConstants.ATTRIBUTES_KEY_LIBRARY_VERSION, + GaxProperties.getLibraryVersion(DatastoreOptions.class)); return attributes; } /** - * Method to record operation level metrics for HttpJson transport. This method should be called - * after the entire operation across all retry attempts has completed. + * Records operation-level metrics. This method should be called after the entire operation across + * all retry attempts has completed. + * + *

Metrics are recorded for all transport types (gRPC and HTTP). Previously this method only + * recorded metrics for HTTP transport because gRPC metrics were collected separately through + * GAX's {@code MetricsTracerFactory}. That mechanism was removed in favour of recording at this + * single, transport-agnostic layer so that the built-in Cloud Monitoring exporter captures every + * operation regardless of transport. * - * @param metricsRecorder The metrics recorder. + * @param datastoreMetricsRecorder The metrics recorder. * @param datastoreOptions The DatastoreOptions object. - * @param isHttpTransport Whether the current transport is HTTP. * @param operationStopwatch The stopwatch tracking the duration of the entire operation. * @param methodName The name of the API method. * @param status The final status of the operation after all retries. */ public static void recordOperationMetrics( - MetricsRecorder metricsRecorder, + DatastoreMetricsRecorder datastoreMetricsRecorder, DatastoreOptions datastoreOptions, - boolean isHttpTransport, Stopwatch operationStopwatch, String methodName, String status) { - // Operation metrics are only recorded for HttpJson transport as Gax already records - // operation metrics for gRPC transport. This prevents metrics from being recorded twice - // for gRPC transport. - if (!isHttpTransport) { - return; - } if (methodName != null) { Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); - metricsRecorder.recordOperationLatency( + datastoreMetricsRecorder.recordOperationLatency( operationStopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); - metricsRecorder.recordOperationCount(1, attributes); + datastoreMetricsRecorder.recordOperationCount(1, attributes); } } /** - * Wraps a callable with logic to record attempt-level metrics for HttpJson transport. Attempt - * metrics are recorded for each individual execution of the callable, regardless of whether it - * succeeds or fails. + * Wraps a callable with logic to record attempt-level metrics. Attempt metrics are recorded for + * each individual execution of the callable, regardless of whether it succeeds or fails. + * + *

Metrics are recorded for all transport types (gRPC and HTTP). Previously this wrapper only + * recorded metrics for HTTP transport because gRPC metrics were collected separately through + * GAX's {@code MetricsTracerFactory}. That mechanism was removed in favour of recording at this + * single, transport-agnostic layer so that the built-in Cloud Monitoring exporter captures every + * attempt regardless of transport. * * @param callable The original callable to execute. - * @param metricsRecorder The metrics recorder. + * @param datastoreMetricsRecorder The metrics recorder. * @param datastoreOptions The DatastoreOptions object. - * @param isHttpTransport Whether the current transport is HTTP. * @param methodName The name of the API method. * @param The return type of the callable. * @return A wrapped callable that includes attempt-level metrics recording. */ public static Callable attemptMetricsCallable( Callable callable, - MetricsRecorder metricsRecorder, + DatastoreMetricsRecorder datastoreMetricsRecorder, DatastoreOptions datastoreOptions, - boolean isHttpTransport, String methodName) { - // Attempt metrics are already recorded by Gax for gRPC transport. This - // prevents the metrics from being recorded twice for gRPC transport. - if (!isHttpTransport) { - return callable; - } return () -> { Stopwatch stopwatch = Stopwatch.createStarted(); String status = StatusCode.Code.UNKNOWN.toString(); @@ -126,8 +126,9 @@ public static Callable attemptMetricsCallable( } finally { Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); - metricsRecorder.recordAttemptLatency(stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); - metricsRecorder.recordAttemptCount(1, attributes); + datastoreMetricsRecorder.recordAttemptLatency( + stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); + datastoreMetricsRecorder.recordAttemptCount(1, attributes); } }; } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java index b8129af13d37..599275110e4f 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java @@ -23,6 +23,7 @@ import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.spi.DatastoreRpcFactory; import com.google.cloud.datastore.spi.v1.DatastoreRpc; +import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder; import com.google.cloud.datastore.telemetry.TelemetryConstants; import com.google.datastore.v1.BeginTransactionRequest; import com.google.datastore.v1.BeginTransactionResponse; @@ -55,7 +56,7 @@ /** * Tests for transaction metrics recording in {@link DatastoreImpl}. These tests verify that * transaction latency and per-attempt metrics are correctly recorded via the {@link - * com.google.cloud.datastore.telemetry.MetricsRecorder}. + * DatastoreMetricsRecorder}. */ @RunWith(Parameterized.class) public class DatastoreImplMetricsTest { @@ -533,19 +534,6 @@ public void lookup_recordsOperationAndAttemptMetrics() { Collection metrics = metricReader.collectAllMetrics(); - // Gax already records operation and attempt metrics natively for the gRPC transport. - // DatastoreImpl explicitly avoids recording them here to prevent double-counting. - // Since this unit test bypasses the GAX networking layer by mocking DatastoreRpc, - // we assert that no local duplicate metrics are emitted by DatastoreImpl for gRPC, - // and skip the rest of the assertions. - if (TelemetryConstants.Transport.GRPC.equals(transport)) { - Optional operationLatency = - findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_LATENCY); - assertThat(operationLatency.isPresent()).isFalse(); - EasyMock.verify(rpcMock); - return; - } - // Verify operation latency Optional operationLatency = findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_LATENCY); @@ -631,19 +619,6 @@ public void lookup_recordsFailureStatusOnError() { Collection metrics = metricReader.collectAllMetrics(); - // Gax already records operation and attempt metrics natively for the gRPC transport. - // DatastoreImpl explicitly avoids recording them here to prevent double-counting. - // Since this unit test bypasses the GAX networking layer by mocking DatastoreRpc, - // we assert that no local duplicate metrics are emitted by DatastoreImpl for gRPC, - // and skip the rest of the assertions. - if (TelemetryConstants.Transport.GRPC.equals(transport)) { - Optional operationLatency = - findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_LATENCY); - assertThat(operationLatency.isPresent()).isFalse(); - EasyMock.verify(rpcMock); - return; - } - // Verify operation latency with UNAVAILABLE status Optional operationLatency = findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_LATENCY); 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 new file mode 100644 index 000000000000..1c1f76ddc156 --- /dev/null +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorderTest.java @@ -0,0 +1,96 @@ +/* + * 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.telemetry; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.cloud.NoCredentials; +import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; +import com.google.cloud.datastore.DatastoreOpenTelemetryOptionsTestHelper; +import com.google.cloud.datastore.DatastoreOptions; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link DatastoreMetricsRecorder#getInstance(DatastoreOptions)}. */ +@RunWith(JUnit4.class) +public class DatastoreMetricsRecorderTest { + + private static final String PROJECT_ID = "test-project"; + + private DatastoreOptions.Builder baseOptions() { + return DatastoreOptions.newBuilder() + .setProjectId(PROJECT_ID) + .setCredentials(NoCredentials.getInstance()); + } + + @Test + public void defaultOptions_returnsNoOp() { + // metricsEnabled defaults to false, so getInstance() should return NoOp + DatastoreOptions options = baseOptions().build(); + DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options); + assertThat(recorder).isInstanceOf(NoOpDatastoreMetricsRecorder.class); + } + + @Test + public void tracingEnabledButMetricsDisabled_returnsNoOp() { + // Enabling tracing alone should not enable metrics + DatastoreOptions options = + baseOptions() + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptions.newBuilder().setTracingEnabled(true).build()) + .build(); + DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options); + assertThat(recorder).isInstanceOf(NoOpDatastoreMetricsRecorder.class); + } + + @Test + public void metricsEnabled_withCustomOtel_returnsOpenTelemetryRecorder() { + InMemoryMetricReader metricReader = InMemoryMetricReader.create(); + SdkMeterProvider meterProvider = + SdkMeterProvider.builder().registerMetricReader(metricReader).build(); + OpenTelemetry openTelemetry = + OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build(); + + // Use DatastoreOpenTelemetryOptionsTestHelper since setMetricsEnabled() is package-private + // and this test lives in the telemetry sub-package. + DatastoreOptions options = + baseOptions() + .setOpenTelemetryOptions( + DatastoreOpenTelemetryOptionsTestHelper.withMetricsEnabled(openTelemetry)) + .build(); + DatastoreMetricsRecorder recorder = DatastoreMetricsRecorder.getInstance(options); + assertThat(recorder).isInstanceOf(OpenTelemetryDatastoreMetricsRecorder.class); + } + + @Test + public void openTelemetryRecorderCreatedWithExplicitOpenTelemetry() { + InMemoryMetricReader metricReader = InMemoryMetricReader.create(); + SdkMeterProvider meterProvider = + SdkMeterProvider.builder().registerMetricReader(metricReader).build(); + OpenTelemetry openTelemetry = + OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build(); + + OpenTelemetryDatastoreMetricsRecorder recorder = + new OpenTelemetryDatastoreMetricsRecorder(openTelemetry, TelemetryConstants.METRIC_PREFIX); + + assertThat(recorder.getOpenTelemetry()).isSameInstanceAs(openTelemetry); + } +} diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/MetricsRecorderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/MetricsRecorderTest.java deleted file mode 100644 index 51c24b8df30f..000000000000 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/MetricsRecorderTest.java +++ /dev/null @@ -1,65 +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.telemetry; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.cloud.datastore.DatastoreOpenTelemetryOptions; -import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for {@link MetricsRecorder#getInstance(DatastoreOpenTelemetryOptions)}. */ -@RunWith(JUnit4.class) -public class MetricsRecorderTest { - - // TODO(lawrenceqiu): For now, the default behavior is no-op. Add a test for - // instance being OpenTelemetryMetricsRecorder later (visibility changes) - @Test - public void defaultOptionsReturnNoOp() { - DatastoreOpenTelemetryOptions options = DatastoreOpenTelemetryOptions.newBuilder().build(); - MetricsRecorder recorder = MetricsRecorder.getInstance(options); - assertThat(recorder).isInstanceOf(NoOpMetricsRecorder.class); - } - - @Test - public void tracingEnabledButMetricsDisabledReturnsNoOp() { - // Enabling tracing alone should not enable metrics - DatastoreOpenTelemetryOptions options = - DatastoreOpenTelemetryOptions.newBuilder().setTracingEnabled(true).build(); - MetricsRecorder recorder = MetricsRecorder.getInstance(options); - assertThat(recorder).isInstanceOf(NoOpMetricsRecorder.class); - } - - // TODO(lawrenceqiu): Temporary test to ensure that OpenTelemetryMetricsRecorder can - // be created by the DatastoreOpenTelemetryOptions and creates with Otel object - @Test - public void openTelemetryRecorderCreatedWithExplicitOpenTelemetry() { - InMemoryMetricReader metricReader = InMemoryMetricReader.create(); - SdkMeterProvider meterProvider = - SdkMeterProvider.builder().registerMetricReader(metricReader).build(); - OpenTelemetry openTelemetry = - OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build(); - - OpenTelemetryMetricsRecorder recorder = new OpenTelemetryMetricsRecorder(openTelemetry); - - assertThat(recorder.getOpenTelemetry()).isSameInstanceAs(openTelemetry); - } -} diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryMetricsRecorderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java similarity index 94% rename from java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryMetricsRecorderTest.java rename to java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java index 1233e4f1bfac..77cc7c7a32c3 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryMetricsRecorderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java @@ -35,10 +35,10 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class OpenTelemetryMetricsRecorderTest { +public class OpenTelemetryDatastoreMetricsRecorderTest { private InMemoryMetricReader metricReader; - private OpenTelemetryMetricsRecorder recorder; + private OpenTelemetryDatastoreMetricsRecorder recorder; @Before public void setUp() { @@ -47,7 +47,8 @@ public void setUp() { SdkMeterProvider.builder().registerMetricReader(metricReader).build(); OpenTelemetry openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build(); - recorder = new OpenTelemetryMetricsRecorder(openTelemetry); + recorder = + new OpenTelemetryDatastoreMetricsRecorder(openTelemetry, TelemetryConstants.METRIC_PREFIX); } @Test @@ -179,7 +180,7 @@ public void recordAttemptLatency_recordsHistogramWithAttributes() { .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()).isEqualTo("Latency of a single RPC attempt"); + assertThat(metric.getDescription()).isEqualTo("Time an individual attempt took"); assertThat(metric.getUnit()).isEqualTo("ms"); HistogramPointData point = @@ -210,7 +211,7 @@ public void recordAttemptCount_recordsCounterWithAttributes() { .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()).isEqualTo("Number of RPC attempts"); + assertThat(metric.getDescription()).isEqualTo("Number of Attempts"); LongPointData point = metric.getLongSumData().getPoints().stream().findFirst().orElse(null); assertThat(point).isNotNull(); @@ -234,7 +235,8 @@ public void recordOperationLatency_recordsHistogramWithAttributes() { assertThat(metric).isNotNull(); assertThat(metric.getDescription()) - .isEqualTo("Total latency of an operation including retries"); + .isEqualTo( + "Total time until final operation success or failure, including retries and backoff."); assertThat(metric.getUnit()).isEqualTo("ms"); HistogramPointData point = @@ -261,7 +263,7 @@ public void recordOperationCount_recordsCounterWithAttributes() { .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()).isEqualTo("Number of operations"); + assertThat(metric.getDescription()).isEqualTo("Number of Operations"); LongPointData point = metric.getLongSumData().getPoints().stream().findFirst().orElse(null); assertThat(point).isNotNull(); From f2f1e9bf21561cc52bbc8cdc4e28fad07e1caa92 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 1 Apr 2026 13:16:12 -0400 Subject: [PATCH 3/9] chore: Fix variable names --- .../google/cloud/datastore/DatastoreImpl.java | 23 +++++++-------- .../RetryAndTraceDatastoreRpcDecorator.java | 10 +++---- .../telemetry/DatastoreMetricsRecorder.java | 3 +- .../NoOpDatastoreMetricsRecorder.java | 4 +-- .../telemetry/TelemetryConstants.java | 6 +--- .../datastore/telemetry/TelemetryUtils.java | 29 +++++++------------ 6 files changed, 30 insertions(+), 45 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java index 2a884d5f04bb..c4fa583a17e6 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java @@ -100,8 +100,7 @@ final class DatastoreImpl extends BaseService implements Datas private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil = getOptions().getTraceUtil(); - private final DatastoreMetricsRecorder datastoreMetricsRecorder = - getOptions().getMetricsRecorder(); + private final DatastoreMetricsRecorder metricsRecorder = getOptions().getMetricsRecorder(); private final ReadOptionProtoPreparer readOptionProtoPreparer; private final AggregationQueryExecutor aggregationQueryExecutor; @@ -119,7 +118,7 @@ final class DatastoreImpl extends BaseService implements Datas .setTraceUtil(otelTraceUtil) .setRetrySettings(retrySettings) .setDatastoreOptions(options) - .setMetricsRecorder(datastoreMetricsRecorder) + .setMetricsRecorder(metricsRecorder) .build(), options); } @@ -182,7 +181,7 @@ public boolean isClosed() { static class ReadWriteTransactionCallable implements Callable { private final Datastore datastore; private final TransactionCallable callable; - private final DatastoreMetricsRecorder datastoreMetricsRecorder; + private final DatastoreMetricsRecorder metricsRecorder; private volatile TransactionOptions options; private volatile Transaction transaction; @@ -190,11 +189,11 @@ static class ReadWriteTransactionCallable implements Callable { Datastore datastore, TransactionCallable callable, TransactionOptions options, - DatastoreMetricsRecorder datastoreMetricsRecorder) { + DatastoreMetricsRecorder metricsRecorder) { this.datastore = datastore; this.callable = callable; this.options = options; - this.datastoreMetricsRecorder = datastoreMetricsRecorder; + this.metricsRecorder = metricsRecorder; this.transaction = null; } @@ -255,7 +254,7 @@ private void recordAttempt(String status, TransportOptions transportOptions) { attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(transportOptions)); - datastoreMetricsRecorder.recordTransactionAttemptCount(1, attributes); + metricsRecorder.recordTransactionAttemptCount(1, attributes); } } @@ -270,8 +269,7 @@ public T runInTransaction( TraceUtil.Span span = otelTraceUtil.startSpan(SPAN_NAME_TRANSACTION_RUN); ReadWriteTransactionCallable baseCallable = - new ReadWriteTransactionCallable<>( - this, callable, transactionOptions, datastoreMetricsRecorder); + new ReadWriteTransactionCallable<>(this, callable, transactionOptions, metricsRecorder); Callable transactionCallable = baseCallable; if (getOptions().getOpenTelemetryOptions().isTracingEnabled()) { @@ -301,7 +299,7 @@ public T runInTransaction( attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(getOptions().getTransportOptions())); - datastoreMetricsRecorder.recordTransactionLatency(latencyMs, attributes); + metricsRecorder.recordTransactionLatency(latencyMs, attributes); span.end(); } } @@ -809,8 +807,7 @@ private T runWithObservability( DatastoreOptions options = getOptions(); Callable attemptCallable = - TelemetryUtils.attemptMetricsCallable( - callable, datastoreMetricsRecorder, options, methodName); + TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, options, methodName); try (TraceUtil.Scope ignored = span.makeCurrent()) { return RetryHelper.runWithRetries( attemptCallable, retrySettings, exceptionHandler, options.getClock()); @@ -820,7 +817,7 @@ private T runWithObservability( throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - datastoreMetricsRecorder, options, operationStopwatch, methodName, operationStatus); + metricsRecorder, options, operationStopwatch, methodName, operationStatus); span.end(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java index 1c2f9030c327..1b3cbd6b2b78 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java @@ -61,7 +61,7 @@ public class RetryAndTraceDatastoreRpcDecorator implements DatastoreRpc { private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil; private final RetrySettings retrySettings; private final DatastoreOptions datastoreOptions; - private final DatastoreMetricsRecorder datastoreMetricsRecorder; + private final DatastoreMetricsRecorder metricsRecorder; @ObsoleteApi("Prefer to create RetryAndTraceDatastoreRpcDecorator via the Builder") public RetryAndTraceDatastoreRpcDecorator( @@ -73,7 +73,7 @@ public RetryAndTraceDatastoreRpcDecorator( this.retrySettings = retrySettings; this.datastoreOptions = datastoreOptions; this.otelTraceUtil = otelTraceUtil; - this.datastoreMetricsRecorder = new NoOpDatastoreMetricsRecorder(); + this.metricsRecorder = new NoOpDatastoreMetricsRecorder(); } private RetryAndTraceDatastoreRpcDecorator(Builder builder) { @@ -81,7 +81,7 @@ private RetryAndTraceDatastoreRpcDecorator(Builder builder) { this.otelTraceUtil = builder.otelTraceUtil; this.retrySettings = builder.retrySettings; this.datastoreOptions = builder.datastoreOptions; - this.datastoreMetricsRecorder = builder.datastoreMetricsRecorder; + this.metricsRecorder = builder.datastoreMetricsRecorder; } public static Builder newBuilder() { @@ -207,7 +207,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { try (TraceUtil.Scope ignored = span.makeCurrent()) { Callable callable = TelemetryUtils.attemptMetricsCallable( - block, datastoreMetricsRecorder, datastoreOptions, methodName); + block, metricsRecorder, datastoreOptions, methodName); O result = RetryHelper.runWithRetries( callable, this.retrySettings, EXCEPTION_HANDLER, this.datastoreOptions.getClock()); @@ -219,7 +219,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - datastoreMetricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus); + metricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus); span.end(); } } 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 32a9c7cb5ede..e1e18b3104f6 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 @@ -64,7 +64,8 @@ static DatastoreMetricsRecorder getInstance(@Nonnull DatastoreOptions datastoreO if (customOtel == null) { customOtel = GlobalOpenTelemetry.get(); } - return new OpenTelemetryDatastoreMetricsRecorder(customOtel, TelemetryConstants.METRIC_PREFIX); + return new OpenTelemetryDatastoreMetricsRecorder( + customOtel, TelemetryConstants.METRIC_PREFIX); } return new NoOpDatastoreMetricsRecorder(); diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java index caa3bc6f1ec5..a3cf325acc93 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java @@ -22,8 +22,8 @@ /** * A no-op implementation of {@link DatastoreMetricsRecorder}. * - *

Used to stub out metrics instrumentation when metrics are disabled or when no valid - * recorder could be initialized. + *

Used to stub out metrics instrumentation when metrics are disabled or when no valid recorder + * could be initialized. * *

WARNING: This class is intended for internal use only. It was made public to be used across * packages as a default. It should not be used by external customers and its API may change without diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java index 3ce84b5b74c7..9d11f630db4b 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java @@ -71,11 +71,7 @@ public class TelemetryConstants { * *

Cloud Monitoring is strict about label schemas: exporting a label that was not present when * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to - * fail. Registering OTel Views that filter to this set (see {@link DatastoreBuiltInMetricsView}) - * prevents unexpected labels from leaking into the export and causing silent data loss. - * - *

Using {@code AttributeKey} (wildcard) rather than the raw {@code AttributeKey} type - * preserves generic type safety while still allowing keys of any value type in the set. + * fail. */ public static final Set> COMMON_ATTRIBUTES = ImmutableSet.of( diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java index c24e63ac3c60..2db9a1e2c211 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java @@ -65,29 +65,25 @@ public static Map buildMetricAttributes( * Records operation-level metrics. This method should be called after the entire operation across * all retry attempts has completed. * - *

Metrics are recorded for all transport types (gRPC and HTTP). Previously this method only - * recorded metrics for HTTP transport because gRPC metrics were collected separately through - * GAX's {@code MetricsTracerFactory}. That mechanism was removed in favour of recording at this - * single, transport-agnostic layer so that the built-in Cloud Monitoring exporter captures every - * operation regardless of transport. + *

Metrics are recorded for both transport types (gRPC and HTTP). * - * @param datastoreMetricsRecorder The metrics recorder. + * @param metricsRecorder The metrics recorder. * @param datastoreOptions The DatastoreOptions object. * @param operationStopwatch The stopwatch tracking the duration of the entire operation. * @param methodName The name of the API method. * @param status The final status of the operation after all retries. */ public static void recordOperationMetrics( - DatastoreMetricsRecorder datastoreMetricsRecorder, + DatastoreMetricsRecorder metricsRecorder, DatastoreOptions datastoreOptions, Stopwatch operationStopwatch, String methodName, String status) { if (methodName != null) { Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); - datastoreMetricsRecorder.recordOperationLatency( + metricsRecorder.recordOperationLatency( operationStopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); - datastoreMetricsRecorder.recordOperationCount(1, attributes); + metricsRecorder.recordOperationCount(1, attributes); } } @@ -95,14 +91,10 @@ public static void recordOperationMetrics( * Wraps a callable with logic to record attempt-level metrics. Attempt metrics are recorded for * each individual execution of the callable, regardless of whether it succeeds or fails. * - *

Metrics are recorded for all transport types (gRPC and HTTP). Previously this wrapper only - * recorded metrics for HTTP transport because gRPC metrics were collected separately through - * GAX's {@code MetricsTracerFactory}. That mechanism was removed in favour of recording at this - * single, transport-agnostic layer so that the built-in Cloud Monitoring exporter captures every - * attempt regardless of transport. + *

Metrics are recorded for both transport types (gRPC and HTTP). * * @param callable The original callable to execute. - * @param datastoreMetricsRecorder The metrics recorder. + * @param metricsRecorder The metrics recorder. * @param datastoreOptions The DatastoreOptions object. * @param methodName The name of the API method. * @param The return type of the callable. @@ -110,7 +102,7 @@ public static void recordOperationMetrics( */ public static Callable attemptMetricsCallable( Callable callable, - DatastoreMetricsRecorder datastoreMetricsRecorder, + DatastoreMetricsRecorder metricsRecorder, DatastoreOptions datastoreOptions, String methodName) { return () -> { @@ -126,9 +118,8 @@ public static Callable attemptMetricsCallable( } finally { Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); - datastoreMetricsRecorder.recordAttemptLatency( - stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); - datastoreMetricsRecorder.recordAttemptCount(1, attributes); + metricsRecorder.recordAttemptLatency(stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); + metricsRecorder.recordAttemptCount(1, attributes); } }; } From ae438e8208f22e591af2aeebec2c23e1c53d4820 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 1 Apr 2026 13:16:12 -0400 Subject: [PATCH 4/9] chore: Fix variable names --- .../google/cloud/datastore/DatastoreImpl.java | 23 +++++++-------- .../cloud/datastore/DatastoreOptions.java | 6 ++-- .../RetryAndTraceDatastoreRpcDecorator.java | 19 ++++++------ .../telemetry/DatastoreMetricsRecorder.java | 3 +- .../NoOpDatastoreMetricsRecorder.java | 4 +-- .../telemetry/TelemetryConstants.java | 6 +--- .../datastore/telemetry/TelemetryUtils.java | 29 +++++++------------ 7 files changed, 37 insertions(+), 53 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java index 2a884d5f04bb..c4fa583a17e6 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java @@ -100,8 +100,7 @@ final class DatastoreImpl extends BaseService implements Datas private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil = getOptions().getTraceUtil(); - private final DatastoreMetricsRecorder datastoreMetricsRecorder = - getOptions().getMetricsRecorder(); + private final DatastoreMetricsRecorder metricsRecorder = getOptions().getMetricsRecorder(); private final ReadOptionProtoPreparer readOptionProtoPreparer; private final AggregationQueryExecutor aggregationQueryExecutor; @@ -119,7 +118,7 @@ final class DatastoreImpl extends BaseService implements Datas .setTraceUtil(otelTraceUtil) .setRetrySettings(retrySettings) .setDatastoreOptions(options) - .setMetricsRecorder(datastoreMetricsRecorder) + .setMetricsRecorder(metricsRecorder) .build(), options); } @@ -182,7 +181,7 @@ public boolean isClosed() { static class ReadWriteTransactionCallable implements Callable { private final Datastore datastore; private final TransactionCallable callable; - private final DatastoreMetricsRecorder datastoreMetricsRecorder; + private final DatastoreMetricsRecorder metricsRecorder; private volatile TransactionOptions options; private volatile Transaction transaction; @@ -190,11 +189,11 @@ static class ReadWriteTransactionCallable implements Callable { Datastore datastore, TransactionCallable callable, TransactionOptions options, - DatastoreMetricsRecorder datastoreMetricsRecorder) { + DatastoreMetricsRecorder metricsRecorder) { this.datastore = datastore; this.callable = callable; this.options = options; - this.datastoreMetricsRecorder = datastoreMetricsRecorder; + this.metricsRecorder = metricsRecorder; this.transaction = null; } @@ -255,7 +254,7 @@ private void recordAttempt(String status, TransportOptions transportOptions) { attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(transportOptions)); - datastoreMetricsRecorder.recordTransactionAttemptCount(1, attributes); + metricsRecorder.recordTransactionAttemptCount(1, attributes); } } @@ -270,8 +269,7 @@ public T runInTransaction( TraceUtil.Span span = otelTraceUtil.startSpan(SPAN_NAME_TRANSACTION_RUN); ReadWriteTransactionCallable baseCallable = - new ReadWriteTransactionCallable<>( - this, callable, transactionOptions, datastoreMetricsRecorder); + new ReadWriteTransactionCallable<>(this, callable, transactionOptions, metricsRecorder); Callable transactionCallable = baseCallable; if (getOptions().getOpenTelemetryOptions().isTracingEnabled()) { @@ -301,7 +299,7 @@ public T runInTransaction( attributes.put( TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, TelemetryConstants.getTransportName(getOptions().getTransportOptions())); - datastoreMetricsRecorder.recordTransactionLatency(latencyMs, attributes); + metricsRecorder.recordTransactionLatency(latencyMs, attributes); span.end(); } } @@ -809,8 +807,7 @@ private T runWithObservability( DatastoreOptions options = getOptions(); Callable attemptCallable = - TelemetryUtils.attemptMetricsCallable( - callable, datastoreMetricsRecorder, options, methodName); + TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, options, methodName); try (TraceUtil.Scope ignored = span.makeCurrent()) { return RetryHelper.runWithRetries( attemptCallable, retrySettings, exceptionHandler, options.getClock()); @@ -820,7 +817,7 @@ private T runWithObservability( throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - datastoreMetricsRecorder, options, operationStopwatch, methodName, operationStatus); + metricsRecorder, options, operationStopwatch, methodName, operationStatus); span.end(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 8bd0ea0e93f0..461252ac2980 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -65,7 +65,7 @@ public class DatastoreOptions extends ServiceOptions { @@ -223,7 +223,7 @@ private DatastoreOptions(Builder builder) { ? builder.openTelemetryOptions : DatastoreOpenTelemetryOptions.newBuilder().build(); this.traceUtil = com.google.cloud.datastore.telemetry.TraceUtil.getInstance(this); - this.datastoreMetricsRecorder = DatastoreMetricsRecorder.getInstance(this); + this.metricsRecorder = DatastoreMetricsRecorder.getInstance(this); namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace()); databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID); diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java index 1c2f9030c327..946ed471ff62 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java @@ -61,7 +61,7 @@ public class RetryAndTraceDatastoreRpcDecorator implements DatastoreRpc { private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil; private final RetrySettings retrySettings; private final DatastoreOptions datastoreOptions; - private final DatastoreMetricsRecorder datastoreMetricsRecorder; + private final DatastoreMetricsRecorder metricsRecorder; @ObsoleteApi("Prefer to create RetryAndTraceDatastoreRpcDecorator via the Builder") public RetryAndTraceDatastoreRpcDecorator( @@ -73,7 +73,7 @@ public RetryAndTraceDatastoreRpcDecorator( this.retrySettings = retrySettings; this.datastoreOptions = datastoreOptions; this.otelTraceUtil = otelTraceUtil; - this.datastoreMetricsRecorder = new NoOpDatastoreMetricsRecorder(); + this.metricsRecorder = new NoOpDatastoreMetricsRecorder(); } private RetryAndTraceDatastoreRpcDecorator(Builder builder) { @@ -81,7 +81,7 @@ private RetryAndTraceDatastoreRpcDecorator(Builder builder) { this.otelTraceUtil = builder.otelTraceUtil; this.retrySettings = builder.retrySettings; this.datastoreOptions = builder.datastoreOptions; - this.datastoreMetricsRecorder = builder.datastoreMetricsRecorder; + this.metricsRecorder = builder.metricsRecorder; } public static Builder newBuilder() { @@ -95,7 +95,7 @@ public static class Builder { private DatastoreOptions datastoreOptions; // Defaults configured for this class - private DatastoreMetricsRecorder datastoreMetricsRecorder = new NoOpDatastoreMetricsRecorder(); + private DatastoreMetricsRecorder metricsRecorder = new NoOpDatastoreMetricsRecorder(); private Builder() {} @@ -119,10 +119,9 @@ public Builder setDatastoreOptions(DatastoreOptions datastoreOptions) { return this; } - public Builder setMetricsRecorder(DatastoreMetricsRecorder datastoreMetricsRecorder) { - Preconditions.checkNotNull( - datastoreMetricsRecorder, "datastoreMetricsRecorder can not be null"); - this.datastoreMetricsRecorder = datastoreMetricsRecorder; + public Builder setMetricsRecorder(DatastoreMetricsRecorder metricsRecorder) { + Preconditions.checkNotNull(metricsRecorder, "metricsRecorder can not be null"); + this.metricsRecorder = metricsRecorder; return this; } @@ -207,7 +206,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { try (TraceUtil.Scope ignored = span.makeCurrent()) { Callable callable = TelemetryUtils.attemptMetricsCallable( - block, datastoreMetricsRecorder, datastoreOptions, methodName); + block, metricsRecorder, datastoreOptions, methodName); O result = RetryHelper.runWithRetries( callable, this.retrySettings, EXCEPTION_HANDLER, this.datastoreOptions.getClock()); @@ -219,7 +218,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - datastoreMetricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus); + metricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus); span.end(); } } 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 32a9c7cb5ede..e1e18b3104f6 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 @@ -64,7 +64,8 @@ static DatastoreMetricsRecorder getInstance(@Nonnull DatastoreOptions datastoreO if (customOtel == null) { customOtel = GlobalOpenTelemetry.get(); } - return new OpenTelemetryDatastoreMetricsRecorder(customOtel, TelemetryConstants.METRIC_PREFIX); + return new OpenTelemetryDatastoreMetricsRecorder( + customOtel, TelemetryConstants.METRIC_PREFIX); } return new NoOpDatastoreMetricsRecorder(); diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java index caa3bc6f1ec5..a3cf325acc93 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/NoOpDatastoreMetricsRecorder.java @@ -22,8 +22,8 @@ /** * A no-op implementation of {@link DatastoreMetricsRecorder}. * - *

Used to stub out metrics instrumentation when metrics are disabled or when no valid - * recorder could be initialized. + *

Used to stub out metrics instrumentation when metrics are disabled or when no valid recorder + * could be initialized. * *

WARNING: This class is intended for internal use only. It was made public to be used across * packages as a default. It should not be used by external customers and its API may change without diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java index 3ce84b5b74c7..9d11f630db4b 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java @@ -71,11 +71,7 @@ public class TelemetryConstants { * *

Cloud Monitoring is strict about label schemas: exporting a label that was not present when * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to - * fail. Registering OTel Views that filter to this set (see {@link DatastoreBuiltInMetricsView}) - * prevents unexpected labels from leaking into the export and causing silent data loss. - * - *

Using {@code AttributeKey} (wildcard) rather than the raw {@code AttributeKey} type - * preserves generic type safety while still allowing keys of any value type in the set. + * fail. */ public static final Set> COMMON_ATTRIBUTES = ImmutableSet.of( diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java index c24e63ac3c60..2db9a1e2c211 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java @@ -65,29 +65,25 @@ public static Map buildMetricAttributes( * Records operation-level metrics. This method should be called after the entire operation across * all retry attempts has completed. * - *

Metrics are recorded for all transport types (gRPC and HTTP). Previously this method only - * recorded metrics for HTTP transport because gRPC metrics were collected separately through - * GAX's {@code MetricsTracerFactory}. That mechanism was removed in favour of recording at this - * single, transport-agnostic layer so that the built-in Cloud Monitoring exporter captures every - * operation regardless of transport. + *

Metrics are recorded for both transport types (gRPC and HTTP). * - * @param datastoreMetricsRecorder The metrics recorder. + * @param metricsRecorder The metrics recorder. * @param datastoreOptions The DatastoreOptions object. * @param operationStopwatch The stopwatch tracking the duration of the entire operation. * @param methodName The name of the API method. * @param status The final status of the operation after all retries. */ public static void recordOperationMetrics( - DatastoreMetricsRecorder datastoreMetricsRecorder, + DatastoreMetricsRecorder metricsRecorder, DatastoreOptions datastoreOptions, Stopwatch operationStopwatch, String methodName, String status) { if (methodName != null) { Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); - datastoreMetricsRecorder.recordOperationLatency( + metricsRecorder.recordOperationLatency( operationStopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); - datastoreMetricsRecorder.recordOperationCount(1, attributes); + metricsRecorder.recordOperationCount(1, attributes); } } @@ -95,14 +91,10 @@ public static void recordOperationMetrics( * Wraps a callable with logic to record attempt-level metrics. Attempt metrics are recorded for * each individual execution of the callable, regardless of whether it succeeds or fails. * - *

Metrics are recorded for all transport types (gRPC and HTTP). Previously this wrapper only - * recorded metrics for HTTP transport because gRPC metrics were collected separately through - * GAX's {@code MetricsTracerFactory}. That mechanism was removed in favour of recording at this - * single, transport-agnostic layer so that the built-in Cloud Monitoring exporter captures every - * attempt regardless of transport. + *

Metrics are recorded for both transport types (gRPC and HTTP). * * @param callable The original callable to execute. - * @param datastoreMetricsRecorder The metrics recorder. + * @param metricsRecorder The metrics recorder. * @param datastoreOptions The DatastoreOptions object. * @param methodName The name of the API method. * @param The return type of the callable. @@ -110,7 +102,7 @@ public static void recordOperationMetrics( */ public static Callable attemptMetricsCallable( Callable callable, - DatastoreMetricsRecorder datastoreMetricsRecorder, + DatastoreMetricsRecorder metricsRecorder, DatastoreOptions datastoreOptions, String methodName) { return () -> { @@ -126,9 +118,8 @@ public static Callable attemptMetricsCallable( } finally { Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); - datastoreMetricsRecorder.recordAttemptLatency( - stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); - datastoreMetricsRecorder.recordAttemptCount(1, attributes); + metricsRecorder.recordAttemptLatency(stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); + metricsRecorder.recordAttemptCount(1, attributes); } }; } From 29f7ce1d4eb576df16ad78e76f6739b024d98239 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 1 Apr 2026 14:14:33 -0400 Subject: [PATCH 5/9] chore: Remove metrics sample --- .../datastore/samples/MetricsSample.java | 72 ------------------- 1 file changed, 72 deletions(-) delete mode 100644 java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/samples/MetricsSample.java diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/samples/MetricsSample.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/samples/MetricsSample.java deleted file mode 100644 index e2dcbcc455e4..000000000000 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/samples/MetricsSample.java +++ /dev/null @@ -1,72 +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.samples; - -import com.google.cloud.datastore.Datastore; -import com.google.cloud.datastore.DatastoreOptions; -import com.google.cloud.datastore.Entity; -import com.google.cloud.datastore.Key; - -/** A simple sample to verify that Datastore metrics are being recorded and exported correctly. */ -public class MetricsSample { - - public static void main(String[] args) throws Exception { - String projectId = System.getenv("GOOGLE_CLOUD_PROJECT"); - if (projectId == null || projectId.isEmpty()) { - System.err.println("Error: GOOGLE_CLOUD_PROJECT environment variable is not set."); - System.exit(1); - } - - System.out.println("Starting MetricsSample for project: " + projectId); - - // Initialize Datastore client with metrics enabled by default. - Datastore datastore = - DatastoreOptions.newBuilder().setProjectId(projectId).build().getService(); - - System.out.println("Executing operations to generate metrics..."); - - // 1. Basic Key Lookup - Key key = datastore.newKeyFactory().setKind("MetricsTask").newKey("sample-task"); - datastore.get(key); - System.out.println("Performed lookup."); - - // 2. Transactional Read/Write - datastore.runInTransaction( - tx -> { - Entity entity = - Entity.newBuilder(key) - .set("description", "Recorded via MetricsSample") - .set("timestamp", System.currentTimeMillis()) - .build(); - tx.put(entity); - return null; - }); - System.out.println("Performed transaction."); - - // 3. Batch operation - datastore.delete(key); - System.out.println("Performed delete."); - - System.out.println("Waiting 65 seconds for metrics to be flushed to Cloud Monitoring..."); - // PeriodicMetricReader default interval is 60s. - Thread.sleep(65000); - - System.out.println("Sample finished. Check the Cloud Monitoring dashboard for:"); - System.out.println(" - firestore.googleapis.com/internal/client/operation_latency"); - System.out.println(" - firestore.googleapis.com/internal/client/transaction_latency"); - } -} From 0bf197b54a1c60b472ceb79fa1b282944872dacd Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 1 Apr 2026 22:48:23 -0400 Subject: [PATCH 6/9] chore: Update to remove the extra attributes --- .../google/cloud/datastore/DatastoreImpl.java | 35 ++--- .../RetryAndTraceDatastoreRpcDecorator.java | 5 +- ...OpenTelemetryDatastoreMetricsRecorder.java | 32 +++++ .../telemetry/TelemetryConstants.java | 128 +++++++----------- .../datastore/telemetry/TelemetryUtils.java | 28 +--- .../datastore/DatastoreImplMetricsTest.java | 68 ++++------ 6 files changed, 119 insertions(+), 177 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java index c4fa583a17e6..12de6668c307 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java @@ -45,7 +45,6 @@ import com.google.cloud.RetryHelper; import com.google.cloud.RetryHelper.RetryHelperException; import com.google.cloud.ServiceOptions; -import com.google.cloud.TransportOptions; import com.google.cloud.datastore.execution.AggregationQueryExecutor; import com.google.cloud.datastore.spi.v1.DatastoreRpc; import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder; @@ -74,7 +73,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -223,7 +221,7 @@ public T call() throws DatastoreException { } throw DatastoreException.propagateUserException(ex); } finally { - recordAttempt(attemptStatus, datastore.getOptions().getTransportOptions()); + recordAttempt(attemptStatus); // If the transaction is active, then commit the rollback. If it was already successfully // rolled back, the transaction is inactive (prevents rolling back an already rolled back // transaction). @@ -242,18 +240,10 @@ public T call() throws DatastoreException { * Records a single transaction commit attempt with the given status code. This is called once * per invocation of {@link #call()}, capturing the outcome of each individual commit attempt. */ - private void recordAttempt(String status, TransportOptions transportOptions) { - Map attributes = new HashMap<>(); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_STATUS, status); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_METHOD, TelemetryConstants.METHOD_TRANSACTION_COMMIT); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, datastore.getOptions().getProjectId()); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, datastore.getOptions().getDatabaseId()); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, - TelemetryConstants.getTransportName(transportOptions)); + private void recordAttempt(String status) { + Map attributes = + TelemetryUtils.buildMetricAttributes( + TelemetryConstants.METHOD_TRANSACTION_COMMIT, status); metricsRecorder.recordTransactionAttemptCount(1, attributes); } } @@ -290,15 +280,8 @@ public T runInTransaction( throw DatastoreException.translateAndThrow(e); } finally { long latencyMs = stopwatch.elapsed(TimeUnit.MILLISECONDS); - Map attributes = new HashMap<>(); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_STATUS, status); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_METHOD, TelemetryConstants.METHOD_TRANSACTION_RUN); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, getOptions().getProjectId()); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, getOptions().getDatabaseId()); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, - TelemetryConstants.getTransportName(getOptions().getTransportOptions())); + Map attributes = + TelemetryUtils.buildMetricAttributes(TelemetryConstants.METHOD_TRANSACTION_RUN, status); metricsRecorder.recordTransactionLatency(latencyMs, attributes); span.end(); } @@ -807,7 +790,7 @@ private T runWithObservability( DatastoreOptions options = getOptions(); Callable attemptCallable = - TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, options, methodName); + TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, methodName); try (TraceUtil.Scope ignored = span.makeCurrent()) { return RetryHelper.runWithRetries( attemptCallable, retrySettings, exceptionHandler, options.getClock()); @@ -817,7 +800,7 @@ private T runWithObservability( throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - metricsRecorder, options, operationStopwatch, methodName, operationStatus); + metricsRecorder, operationStopwatch, methodName, operationStatus); span.end(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java index 946ed471ff62..515e431af431 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java @@ -205,8 +205,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { String operationStatus = StatusCode.Code.UNKNOWN.toString(); try (TraceUtil.Scope ignored = span.makeCurrent()) { Callable callable = - TelemetryUtils.attemptMetricsCallable( - block, metricsRecorder, datastoreOptions, methodName); + TelemetryUtils.attemptMetricsCallable(block, metricsRecorder, methodName); O result = RetryHelper.runWithRetries( callable, this.retrySettings, EXCEPTION_HANDLER, this.datastoreOptions.getClock()); @@ -218,7 +217,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - metricsRecorder, datastoreOptions, stopwatch, methodName, operationStatus); + metricsRecorder, stopwatch, methodName, operationStatus); span.end(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java index 8a39db46721c..4b72280b1e98 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java @@ -45,9 +45,16 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder private final OpenTelemetry openTelemetry; + // Datastore-specific transaction metrics (registered under the Datastore meter). private final DoubleHistogram transactionLatency; private final LongCounter transactionAttemptCount; + // GAX operation/attempt latency metrics re-registered under the Datastore meter with the + // plural names required by the internal Cloud Monitoring descriptor. These override the + // singular-named histograms registered by the parent GAX class. + private final DoubleHistogram operationLatency; + private final DoubleHistogram attemptLatency; + OpenTelemetryDatastoreMetricsRecorder(@Nonnull OpenTelemetry openTelemetry, String metricPrefix) { super(openTelemetry, metricPrefix); this.openTelemetry = openTelemetry; @@ -66,12 +73,37 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder .counterBuilder(TelemetryConstants.METRIC_NAME_TRANSACTION_ATTEMPT_COUNT) .setDescription("Number of attempts to commit a transaction") .build(); + + this.operationLatency = + meter + .histogramBuilder(TelemetryConstants.METRIC_NAME_OPERATION_LATENCY) + .setDescription( + "Total time until final operation success or failure, including retries and backoff.") + .setUnit("ms") + .build(); + + this.attemptLatency = + meter + .histogramBuilder(TelemetryConstants.METRIC_NAME_ATTEMPT_LATENCY) + .setDescription("Time an individual attempt took") + .setUnit("ms") + .build(); } OpenTelemetry getOpenTelemetry() { return openTelemetry; } + @Override + public void recordOperationLatency(double latencyMs, Map attributes) { + operationLatency.record(latencyMs, toOtelAttributes(attributes)); + } + + @Override + public void recordAttemptLatency(double latencyMs, Map attributes) { + attemptLatency.record(latencyMs, toOtelAttributes(attributes)); + } + @Override public void recordTransactionLatency(double latencyMs, Map attributes) { transactionLatency.record(latencyMs, toOtelAttributes(attributes)); diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java index 9d11f630db4b..ead362722820 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java @@ -17,9 +17,6 @@ package com.google.cloud.datastore.telemetry; import com.google.api.core.InternalApi; -import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; -import com.google.cloud.TransportOptions; -import com.google.cloud.grpc.GrpcTransportOptions; import com.google.common.collect.ImmutableSet; import io.opentelemetry.api.common.AttributeKey; import java.util.Set; @@ -33,80 +30,24 @@ */ @InternalApi public class TelemetryConstants { - // Built-in metrics constants for Cloud Monitoring export + + // The Firestore namespace has not been deployed yet. Must target the custom namespace + // until this is implemented. public static final String METRIC_PREFIX = "custom.googleapis.com/internal/client"; - public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME; public static final String DATASTORE_METER_NAME = "java-datastore"; // Monitored resource type for Cloud Monitoring public static final String DATASTORE_RESOURCE_TYPE = "global"; // Resource label keys for the monitored resource + // The Firestore namespace has not been deployed yet. Must target the global + // Monitored Resource until this is implemented. public static final String RESOURCE_LABEL_PROJECT_ID = "project_id"; public static final String RESOURCE_LABEL_DATABASE_ID = "database_id"; public static final String RESOURCE_LABEL_LOCATION = "location"; public static final Set DATASTORE_RESOURCE_LABELS = - ImmutableSet.of(RESOURCE_LABEL_PROJECT_ID); - - // Resource attribute keys (used on OTel Resource) - public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); - public static final AttributeKey DATABASE_ID_KEY = AttributeKey.stringKey("database_id"); - public static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); - - // Metric attribute keys (used on metric data points) - public static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); - public static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); - public static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); - public static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); - public static final AttributeKey DATABASE_KEY = AttributeKey.stringKey("database_id"); - public static final AttributeKey LIBRARY_VERSION_KEY = - AttributeKey.stringKey("library_version"); - public static final AttributeKey TRANSPORT_KEY = AttributeKey.stringKey("transport"); - public static final AttributeKey SERVICE_KEY = AttributeKey.stringKey("service"); - - public static final String SERVICE_VALUE = "datastore.googleapis.com"; - - /** - * The allowlist of metric attributes that are permitted on every exported data point. - * - *

Cloud Monitoring is strict about label schemas: exporting a label that was not present when - * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to - * fail. - */ - public static final Set> COMMON_ATTRIBUTES = - ImmutableSet.of( - CLIENT_UID_KEY, - CLIENT_NAME_KEY, - METHOD_KEY, - STATUS_KEY, - DATABASE_KEY, - LIBRARY_VERSION_KEY, - TRANSPORT_KEY, - SERVICE_KEY); - - // Metric names (short names, without prefix) - public static final String METRIC_NAME_SHORT_OPERATION_LATENCY = "operation_latency"; - public static final String METRIC_NAME_SHORT_ATTEMPT_LATENCY = "attempt_latency"; - public static final String METRIC_NAME_SHORT_OPERATION_COUNT = "operation_count"; - public static final String METRIC_NAME_SHORT_ATTEMPT_COUNT = "attempt_count"; - public static final String METRIC_NAME_SHORT_TRANSACTION_LATENCY = "transaction_latency"; - public static final String METRIC_NAME_SHORT_TRANSACTION_ATTEMPT_COUNT = - "transaction_attempt_count"; - - // Metrics collected at the GAX layer vs Datastore SDK layer - public static final Set GAX_METRICS = - ImmutableSet.of( - METRIC_NAME_SHORT_OPERATION_LATENCY, - METRIC_NAME_SHORT_ATTEMPT_LATENCY, - METRIC_NAME_SHORT_OPERATION_COUNT, - METRIC_NAME_SHORT_ATTEMPT_COUNT); - - public static final Set DATASTORE_METRICS = ImmutableSet.of( - METRIC_NAME_SHORT_TRANSACTION_LATENCY, METRIC_NAME_SHORT_TRANSACTION_ATTEMPT_COUNT); - - // Environment variable to enable/disable built-in metrics - public static final String ENABLE_METRICS_ENV_VAR = "DATASTORE_ENABLE_METRICS"; + RESOURCE_LABEL_PROJECT_ID, RESOURCE_LABEL_DATABASE_ID, RESOURCE_LABEL_LOCATION); // Existing attribute key constants (string-based, used by MetricsHelper/TelemetryUtils) public static final String ATTRIBUTES_KEY_DOCUMENT_COUNT = "doc_count"; @@ -130,23 +71,58 @@ public class TelemetryConstants { /** Attribute key for the Datastore database ID. */ public static final String ATTRIBUTES_KEY_DATABASE_ID = "database_id"; - public static final String ATTRIBUTES_KEY_LIBRARY_VERSION = "library_version"; + // Resource attribute keys (used on OTel Resource) + public static final AttributeKey PROJECT_ID_KEY = AttributeKey.stringKey("project_id"); + public static final AttributeKey DATABASE_ID_KEY = AttributeKey.stringKey("database_id"); + public static final AttributeKey LOCATION_ID_KEY = AttributeKey.stringKey("location"); + + // Metric attribute keys (used on metric data points) + public static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); + public static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); + public static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); + public static final AttributeKey SERVICE_KEY = AttributeKey.stringKey("service"); + + public static final String SERVICE_VALUE = "datastore.googleapis.com"; - public static final String ATTRIBUTES_KEY_TRANSPORT = "transport"; + /** String key for the {@code service} metric attribute (value: {@code "service"}). */ + public static final String ATTRIBUTES_KEY_SERVICE = SERVICE_KEY.getKey(); + + /** + * The allowlist of metric attributes that are permitted on every exported data point. + * + *

Cloud Monitoring is strict about label schemas: exporting a label that was not present when + * the metric descriptor was first created will cause the entire {@code createTimeSeries} call to + * fail. Only {@code status}, {@code method}, {@code service}, and {@code client_uid} are + * accepted; all other attributes must be omitted from every {@code record*()} call. + */ + public static final Set> COMMON_ATTRIBUTES = + ImmutableSet.of(CLIENT_UID_KEY, METHOD_KEY, STATUS_KEY, SERVICE_KEY); /** Metric name for the total latency of a transaction. */ public static final String METRIC_NAME_TRANSACTION_LATENCY = - METRIC_PREFIX + "/transaction_latency"; + METRIC_PREFIX + "/transaction_latencies"; /** Metric name for the number of attempts a transaction took. */ public static final String METRIC_NAME_TRANSACTION_ATTEMPT_COUNT = METRIC_PREFIX + "/transaction_attempt_count"; - /** Metric name for the total latency of an operation (one full RPC call including retries). */ - public static final String METRIC_NAME_OPERATION_LATENCY = METRIC_PREFIX + "/operation_latency"; + /** + * Metric name for the total latency of an operation (one full RPC call including retries). + * + *

The plural form ({@code operation_latencies}) is intentional: it matches the internal Cloud + * Monitoring metric descriptor name. {@link OpenTelemetryDatastoreMetricsRecorder} overrides the + * inherited GAX method to record to this name rather than the singular GAX default. + */ + public static final String METRIC_NAME_OPERATION_LATENCY = METRIC_PREFIX + "/operation_latencies"; - /** Metric name for the latency of a single RPC attempt. */ - public static final String METRIC_NAME_ATTEMPT_LATENCY = METRIC_PREFIX + "/attempt_latency"; + /** + * Metric name for the latency of a single RPC attempt. + * + *

The plural form ({@code attempt_latencies}) is intentional: it matches the internal Cloud + * Monitoring metric descriptor name. {@link OpenTelemetryDatastoreMetricsRecorder} overrides the + * inherited GAX method to record to this name rather than the singular GAX default. + */ + public static final String METRIC_NAME_ATTEMPT_LATENCY = METRIC_PREFIX + "/attempt_latencies"; /** Metric name for the count of operations. */ public static final String METRIC_NAME_OPERATION_COUNT = METRIC_PREFIX + "/operation_count"; @@ -191,13 +167,5 @@ public String getTransport() { } } - public static String getTransportName(TransportOptions transportOptions) { - if (transportOptions instanceof GrpcTransportOptions) { - return Transport.GRPC.getTransport(); - } else { - return Transport.HTTP.getTransport(); - } - } - private TelemetryConstants() {} } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java index 2db9a1e2c211..79fc8f0ec2af 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java @@ -17,10 +17,8 @@ package com.google.cloud.datastore.telemetry; import com.google.api.core.InternalApi; -import com.google.api.gax.core.GaxProperties; import com.google.api.gax.rpc.StatusCode; import com.google.cloud.datastore.DatastoreException; -import com.google.cloud.datastore.DatastoreOptions; import com.google.common.base.Stopwatch; import java.util.HashMap; import java.util.Map; @@ -40,24 +38,15 @@ private TelemetryUtils() {} /** * Method to build a map of attributes to be used across both operation and attempt level metrics. * - * @param datastoreOptions The DatastoreOptions object. * @param methodName The name of the API method. * @param status The status of the operation or attempt. * @return The map of attributes. */ - public static Map buildMetricAttributes( - DatastoreOptions datastoreOptions, String methodName, String status) { + public static Map buildMetricAttributes(String methodName, String status) { Map attributes = new HashMap<>(); attributes.put(TelemetryConstants.ATTRIBUTES_KEY_METHOD, methodName); attributes.put(TelemetryConstants.ATTRIBUTES_KEY_STATUS, status); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, datastoreOptions.getProjectId()); - attributes.put(TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, datastoreOptions.getDatabaseId()); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_TRANSPORT, - TelemetryConstants.getTransportName(datastoreOptions.getTransportOptions())); - attributes.put( - TelemetryConstants.ATTRIBUTES_KEY_LIBRARY_VERSION, - GaxProperties.getLibraryVersion(DatastoreOptions.class)); + attributes.put(TelemetryConstants.ATTRIBUTES_KEY_SERVICE, TelemetryConstants.SERVICE_VALUE); return attributes; } @@ -68,19 +57,17 @@ public static Map buildMetricAttributes( *

Metrics are recorded for both transport types (gRPC and HTTP). * * @param metricsRecorder The metrics recorder. - * @param datastoreOptions The DatastoreOptions object. * @param operationStopwatch The stopwatch tracking the duration of the entire operation. * @param methodName The name of the API method. * @param status The final status of the operation after all retries. */ public static void recordOperationMetrics( DatastoreMetricsRecorder metricsRecorder, - DatastoreOptions datastoreOptions, Stopwatch operationStopwatch, String methodName, String status) { if (methodName != null) { - Map attributes = buildMetricAttributes(datastoreOptions, methodName, status); + Map attributes = buildMetricAttributes(methodName, status); metricsRecorder.recordOperationLatency( operationStopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordOperationCount(1, attributes); @@ -95,16 +82,12 @@ public static void recordOperationMetrics( * * @param callable The original callable to execute. * @param metricsRecorder The metrics recorder. - * @param datastoreOptions The DatastoreOptions object. * @param methodName The name of the API method. * @param The return type of the callable. * @return A wrapped callable that includes attempt-level metrics recording. */ public static Callable attemptMetricsCallable( - Callable callable, - DatastoreMetricsRecorder metricsRecorder, - DatastoreOptions datastoreOptions, - String methodName) { + Callable callable, DatastoreMetricsRecorder metricsRecorder, String methodName) { return () -> { Stopwatch stopwatch = Stopwatch.createStarted(); String status = StatusCode.Code.UNKNOWN.toString(); @@ -116,8 +99,7 @@ public static Callable attemptMetricsCallable( status = DatastoreException.extractStatusCode(e); throw e; } finally { - Map attributes = - buildMetricAttributes(datastoreOptions, methodName, status); + Map attributes = buildMetricAttributes(methodName, status); metricsRecorder.recordAttemptLatency(stopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java index 599275110e4f..4a1c73db6f47 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java @@ -161,11 +161,7 @@ public void runInTransaction_recordsLatencyOnSuccess() { .isTrue(); assertThat( dataContainsStringAttribute( - point, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - point, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + point, TelemetryConstants.ATTRIBUTES_KEY_SERVICE, TelemetryConstants.SERVICE_VALUE)) .isTrue(); EasyMock.verify(rpcMock); @@ -204,11 +200,7 @@ public void runInTransaction_recordsPerAttemptCountOnSuccess() { .isTrue(); assertThat( dataContainsStringAttribute( - point, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - point, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + point, TelemetryConstants.ATTRIBUTES_KEY_SERVICE, TelemetryConstants.SERVICE_VALUE)) .isTrue(); EasyMock.verify(rpcMock); @@ -286,11 +278,9 @@ public Integer run(DatastoreReaderWriter transaction) { .isTrue(); assertThat( dataContainsStringAttribute( - abortedPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - abortedPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + abortedPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); LongPointData okPoint = @@ -317,11 +307,9 @@ public Integer run(DatastoreReaderWriter transaction) { .isTrue(); assertThat( dataContainsStringAttribute( - okPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - okPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + okPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); // Verify latency was recorded with OK (overall transaction succeeded) @@ -349,11 +337,9 @@ public Integer run(DatastoreReaderWriter transaction) { .isTrue(); assertThat( dataContainsStringAttribute( - latencyPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - latencyPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + latencyPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); EasyMock.verify(rpcMock); @@ -422,11 +408,9 @@ public Integer run(DatastoreReaderWriter transaction) { .isTrue(); assertThat( dataContainsStringAttribute( - abortedPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - abortedPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + abortedPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); LongPointData cancelledPoint = @@ -454,11 +438,9 @@ public Integer run(DatastoreReaderWriter transaction) { .isTrue(); assertThat( dataContainsStringAttribute( - cancelledPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - cancelledPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + cancelledPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); // Verify latency was recorded with the failure status code @@ -487,11 +469,9 @@ public Integer run(DatastoreReaderWriter transaction) { .isTrue(); assertThat( dataContainsStringAttribute( - latencyPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - latencyPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + latencyPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); } @@ -558,11 +538,9 @@ public void lookup_recordsOperationAndAttemptMetrics() { .isTrue(); assertThat( dataContainsStringAttribute( - opLatencyPoint, TelemetryConstants.ATTRIBUTES_KEY_PROJECT_ID, PROJECT_ID)) - .isTrue(); - assertThat( - dataContainsStringAttribute( - opLatencyPoint, TelemetryConstants.ATTRIBUTES_KEY_DATABASE_ID, DATABASE_ID)) + opLatencyPoint, + TelemetryConstants.ATTRIBUTES_KEY_SERVICE, + TelemetryConstants.SERVICE_VALUE)) .isTrue(); // Verify operation count From 5dc702f128a8b4743f37481730b91f54e84b546b Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Thu, 2 Apr 2026 00:21:01 -0400 Subject: [PATCH 7/9] chore: Remove duplicate declaration of attempt and operation metrics --- ...OpenTelemetryDatastoreMetricsRecorder.java | 36 +++---------------- ...TelemetryDatastoreMetricsRecorderTest.java | 15 +++----- 2 files changed, 10 insertions(+), 41 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java index 4b72280b1e98..550dc1df9a7d 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java @@ -49,12 +49,11 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder private final DoubleHistogram transactionLatency; private final LongCounter transactionAttemptCount; - // GAX operation/attempt latency metrics re-registered under the Datastore meter with the - // plural names required by the internal Cloud Monitoring descriptor. These override the - // singular-named histograms registered by the parent GAX class. - private final DoubleHistogram operationLatency; - private final DoubleHistogram attemptLatency; - + // Note: Standard GAX RPC metrics (operation_latency, attempt_latency, etc.) are handled by the + // base OpenTelemetryMetricsRecorder class. Those metrics are inherited from the parent classes. + // However, the internal metrics expect plural suffixes (e.g. `latencies` instead of `latency`). + // The discrepancy between the singular GAX names and the plural internal Cloud Monitoring names + // is handled by configuring OpenTelemetry Views. OpenTelemetryDatastoreMetricsRecorder(@Nonnull OpenTelemetry openTelemetry, String metricPrefix) { super(openTelemetry, metricPrefix); this.openTelemetry = openTelemetry; @@ -73,37 +72,12 @@ class OpenTelemetryDatastoreMetricsRecorder extends OpenTelemetryMetricsRecorder .counterBuilder(TelemetryConstants.METRIC_NAME_TRANSACTION_ATTEMPT_COUNT) .setDescription("Number of attempts to commit a transaction") .build(); - - this.operationLatency = - meter - .histogramBuilder(TelemetryConstants.METRIC_NAME_OPERATION_LATENCY) - .setDescription( - "Total time until final operation success or failure, including retries and backoff.") - .setUnit("ms") - .build(); - - this.attemptLatency = - meter - .histogramBuilder(TelemetryConstants.METRIC_NAME_ATTEMPT_LATENCY) - .setDescription("Time an individual attempt took") - .setUnit("ms") - .build(); } OpenTelemetry getOpenTelemetry() { return openTelemetry; } - @Override - public void recordOperationLatency(double latencyMs, Map attributes) { - operationLatency.record(latencyMs, toOtelAttributes(attributes)); - } - - @Override - public void recordAttemptLatency(double latencyMs, Map attributes) { - attemptLatency.record(latencyMs, toOtelAttributes(attributes)); - } - @Override public void recordTransactionLatency(double latencyMs, Map attributes) { transactionLatency.record(latencyMs, toOtelAttributes(attributes)); diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java index 77cc7c7a32c3..a76e17dc2da9 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorderTest.java @@ -175,12 +175,11 @@ public void recordAttemptLatency_recordsHistogramWithAttributes() { Collection metrics = metricReader.collectAllMetrics(); MetricData metric = metrics.stream() - .filter(m -> m.getName().equals(TelemetryConstants.METRIC_NAME_ATTEMPT_LATENCY)) + .filter(m -> m.getName().equals(TelemetryConstants.METRIC_PREFIX + "/attempt_latency")) .findFirst() .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()).isEqualTo("Time an individual attempt took"); assertThat(metric.getUnit()).isEqualTo("ms"); HistogramPointData point = @@ -206,12 +205,11 @@ public void recordAttemptCount_recordsCounterWithAttributes() { Collection metrics = metricReader.collectAllMetrics(); MetricData metric = metrics.stream() - .filter(m -> m.getName().equals(TelemetryConstants.METRIC_NAME_ATTEMPT_COUNT)) + .filter(m -> m.getName().equals(TelemetryConstants.METRIC_PREFIX + "/attempt_count")) .findFirst() .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()).isEqualTo("Number of Attempts"); LongPointData point = metric.getLongSumData().getPoints().stream().findFirst().orElse(null); assertThat(point).isNotNull(); @@ -229,14 +227,12 @@ public void recordOperationLatency_recordsHistogramWithAttributes() { Collection metrics = metricReader.collectAllMetrics(); MetricData metric = metrics.stream() - .filter(m -> m.getName().equals(TelemetryConstants.METRIC_NAME_OPERATION_LATENCY)) + .filter( + m -> m.getName().equals(TelemetryConstants.METRIC_PREFIX + "/operation_latency")) .findFirst() .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()) - .isEqualTo( - "Total time until final operation success or failure, including retries and backoff."); assertThat(metric.getUnit()).isEqualTo("ms"); HistogramPointData point = @@ -258,12 +254,11 @@ public void recordOperationCount_recordsCounterWithAttributes() { Collection metrics = metricReader.collectAllMetrics(); MetricData metric = metrics.stream() - .filter(m -> m.getName().equals(TelemetryConstants.METRIC_NAME_OPERATION_COUNT)) + .filter(m -> m.getName().equals(TelemetryConstants.METRIC_PREFIX + "/operation_count")) .findFirst() .orElse(null); assertThat(metric).isNotNull(); - assertThat(metric.getDescription()).isEqualTo("Number of Operations"); LongPointData point = metric.getLongSumData().getPoints().stream().findFirst().orElse(null); assertThat(point).isNotNull(); From c82e1d82359664b878ea5b9f6b93f250eb4eaf53 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Thu, 2 Apr 2026 01:14:15 -0400 Subject: [PATCH 8/9] test: update DatastoreImplMetricsTest to expect singular GAX metric names --- .../cloud/datastore/DatastoreImplMetricsTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java index 4a1c73db6f47..8a611526f0c3 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java @@ -516,7 +516,7 @@ public void lookup_recordsOperationAndAttemptMetrics() { // Verify operation latency Optional operationLatency = - findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_LATENCY); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/operation_latency"); assertThat(operationLatency.isPresent()).isTrue(); HistogramPointData opLatencyPoint = operationLatency.get().getHistogramData().getPoints().stream() @@ -545,12 +545,12 @@ public void lookup_recordsOperationAndAttemptMetrics() { // Verify operation count Optional operationCount = - findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_COUNT); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/operation_count"); assertThat(operationCount.isPresent()).isTrue(); // Verify attempt latency Optional attemptLatency = - findMetric(metrics, TelemetryConstants.METRIC_NAME_ATTEMPT_LATENCY); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/attempt_latency"); assertThat(attemptLatency.isPresent()).isTrue(); HistogramPointData attLatencyPoint = attemptLatency.get().getHistogramData().getPoints().stream() @@ -567,7 +567,7 @@ public void lookup_recordsOperationAndAttemptMetrics() { // Verify attempt count Optional attemptCount = - findMetric(metrics, TelemetryConstants.METRIC_NAME_ATTEMPT_COUNT); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/attempt_count"); assertThat(attemptCount.isPresent()).isTrue(); EasyMock.verify(rpcMock); @@ -599,7 +599,7 @@ public void lookup_recordsFailureStatusOnError() { // Verify operation latency with UNAVAILABLE status Optional operationLatency = - findMetric(metrics, TelemetryConstants.METRIC_NAME_OPERATION_LATENCY); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/operation_latency"); assertThat(operationLatency.isPresent()).isTrue(); HistogramPointData opLatencyPoint = operationLatency.get().getHistogramData().getPoints().stream() @@ -621,7 +621,7 @@ public void lookup_recordsFailureStatusOnError() { // Verify attempt metrics were also recorded with UNAVAILABLE Optional attemptCount = - findMetric(metrics, TelemetryConstants.METRIC_NAME_ATTEMPT_COUNT); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/attempt_count"); assertThat(attemptCount.isPresent()).isTrue(); EasyMock.verify(rpcMock); From b36e61c91118ac3e89fac0d76522c5d08323cfe1 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Thu, 2 Apr 2026 11:14:56 -0400 Subject: [PATCH 9/9] chore: Remove unnecessary transaction null check --- .../java/com/google/cloud/datastore/DatastoreImpl.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java index 12de6668c307..c7b938e5edcb 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java @@ -216,7 +216,7 @@ public T call() throws DatastoreException { // or from `transaction.commit()`. If there is an exception thrown from either call site, // then the transaction is still active. Check if it is still active (e.g. not commited) // and roll back the transaction. - if (transaction != null && transaction.isActive()) { + if (transaction.isActive()) { transaction.rollback(); } throw DatastoreException.propagateUserException(ex); @@ -225,11 +225,10 @@ public T call() throws DatastoreException { // If the transaction is active, then commit the rollback. If it was already successfully // rolled back, the transaction is inactive (prevents rolling back an already rolled back // transaction). - if (transaction != null && transaction.isActive()) { + if (transaction.isActive()) { transaction.rollback(); } - if (transaction != null - && options != null + if (options != null && options.getModeCase().equals(TransactionOptions.ModeCase.READ_WRITE)) { setPrevTransactionId(transaction.getTransactionId()); }