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..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 @@ -45,15 +45,13 @@ 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.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; @@ -75,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; @@ -101,16 +98,13 @@ 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 metricsRecorder = 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()); @@ -185,7 +179,7 @@ public boolean isClosed() { static class ReadWriteTransactionCallable implements Callable { private final Datastore datastore; private final TransactionCallable callable; - private final MetricsRecorder metricsRecorder; + private final DatastoreMetricsRecorder metricsRecorder; private volatile TransactionOptions options; private volatile Transaction transaction; @@ -193,7 +187,7 @@ static class ReadWriteTransactionCallable implements Callable { Datastore datastore, TransactionCallable callable, TransactionOptions options, - MetricsRecorder metricsRecorder) { + DatastoreMetricsRecorder metricsRecorder) { this.datastore = datastore; this.callable = callable; this.options = options; @@ -227,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). @@ -245,18 +239,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); } } @@ -293,15 +279,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(); } @@ -805,15 +784,12 @@ 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); + TelemetryUtils.attemptMetricsCallable(callable, metricsRecorder, methodName); try (TraceUtil.Scope ignored = span.makeCurrent()) { return RetryHelper.runWithRetries( attemptCallable, retrySettings, exceptionHandler, options.getClock()); @@ -823,12 +799,7 @@ private T runWithObservability( throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - metricsRecorder, - options, - isHttpTransport, - operationStopwatch, - methodName, - operationStatus); + metricsRecorder, 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..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 @@ -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 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); + TelemetryUtils.attemptMetricsCallable(block, metricsRecorder, methodName); O result = RetryHelper.runWithRetries( callable, this.retrySettings, EXCEPTION_HANDLER, this.datastoreOptions.getClock()); @@ -224,12 +217,7 @@ O invokeRpc(Callable block, String startSpan, String methodName) { throw DatastoreException.translateAndThrow(e); } finally { TelemetryUtils.recordOperationMetrics( - metricsRecorder, - datastoreOptions, - isHttpTransport, - stopwatch, - methodName, - operationStatus); + metricsRecorder, 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..e1e18b3104f6 --- /dev/null +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/DatastoreMetricsRecorder.java @@ -0,0 +1,73 @@ +/* + * 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..a3cf325acc93 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..550dc1df9a7d 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,38 @@ 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; + // Datastore-specific transaction metrics (registered under the Datastore meter). 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) { + // 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; - Meter meter = openTelemetry.getMeter(TelemetryConstants.METER_NAME); + Meter meter = openTelemetry.getMeter(TelemetryConstants.DATASTORE_METER_NAME); this.transactionLatency = meter @@ -55,34 +72,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 +88,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..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,8 +17,9 @@ package com.google.cloud.datastore.telemetry; import com.google.api.core.InternalApi; -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. @@ -30,11 +31,25 @@ @InternalApi public class TelemetryConstants { - // 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 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 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_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"; public static final String ATTRIBUTES_KEY_TRANSACTIONAL = "transactional"; public static final String ATTRIBUTES_KEY_TRANSACTION_ID = "transaction_id"; @@ -56,41 +71,64 @@ 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 ATTRIBUTES_KEY_TRANSPORT = "transport"; + public static final String SERVICE_VALUE = "datastore.googleapis.com"; + + /** 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 = - SERVICE_NAME + "/client/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 = - 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. + * 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 = SERVICE_NAME + "/operation_latency"; + public static final String METRIC_NAME_OPERATION_LATENCY = METRIC_PREFIX + "/operation_latencies"; /** - * Metric name for the latency of a single RPC attempt. Note: This does not have the /client - * prefix to match Gax's format. + * 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 = SERVICE_NAME + "/attempt_latency"; + public static final String METRIC_NAME_ATTEMPT_LATENCY = METRIC_PREFIX + "/attempt_latencies"; - /** - * 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. @@ -129,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 7a69cb0157d6..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 @@ -19,7 +19,6 @@ import com.google.api.core.InternalApi; 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; @@ -39,50 +38,36 @@ 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_SERVICE, TelemetryConstants.SERVICE_VALUE); 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 both transport types (gRPC and HTTP). * * @param metricsRecorder 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, - DatastoreOptions datastoreOptions, - boolean isHttpTransport, + DatastoreMetricsRecorder metricsRecorder, 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); + Map attributes = buildMetricAttributes(methodName, status); metricsRecorder.recordOperationLatency( operationStopwatch.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordOperationCount(1, attributes); @@ -90,29 +75,19 @@ public static void recordOperationMetrics( } /** - * 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 both transport types (gRPC and HTTP). * * @param callable The original callable to execute. * @param metricsRecorder 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, - 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; - } + Callable callable, DatastoreMetricsRecorder metricsRecorder, String methodName) { return () -> { Stopwatch stopwatch = Stopwatch.createStarted(); String status = StatusCode.Code.UNKNOWN.toString(); @@ -124,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 b8129af13d37..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 @@ -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 { @@ -160,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); @@ -203,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); @@ -285,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 = @@ -316,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) @@ -348,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); @@ -421,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 = @@ -453,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 @@ -486,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(); } @@ -533,22 +514,9 @@ 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); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/operation_latency"); assertThat(operationLatency.isPresent()).isTrue(); HistogramPointData opLatencyPoint = operationLatency.get().getHistogramData().getPoints().stream() @@ -570,21 +538,19 @@ 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 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() @@ -601,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); @@ -631,22 +597,9 @@ 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); + findMetric(metrics, TelemetryConstants.METRIC_PREFIX + "/operation_latency"); assertThat(operationLatency.isPresent()).isTrue(); HistogramPointData opLatencyPoint = operationLatency.get().getHistogramData().getPoints().stream() @@ -668,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); 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 93% 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..a76e17dc2da9 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 @@ -174,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("Latency of a single RPC attempt"); assertThat(metric.getUnit()).isEqualTo("ms"); HistogramPointData point = @@ -205,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 RPC attempts"); LongPointData point = metric.getLongSumData().getPoints().stream().findFirst().orElse(null); assertThat(point).isNotNull(); @@ -228,13 +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 latency of an operation including retries"); assertThat(metric.getUnit()).isEqualTo("ms"); HistogramPointData point = @@ -256,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();