chore(Datastore): Refactor Datastore Metric classes to inherit from Gax#12361
chore(Datastore): Refactor Datastore Metric classes to inherit from Gax#12361
Conversation
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.
…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
…atastore-csm-impl-1
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Datastore metrics implementation to align with the GAX library's telemetry framework. It introduces the DatastoreMetricsRecorder interface, which extends GAX's MetricsRecorder, and updates the SDK to record metrics for both gRPC and HTTP transports. Key changes include the removal of transport-specific metric logic, updates to TelemetryConstants for standardized metric naming, and the addition of a MetricsSample class. A review comment correctly identified a copy-paste error in the new sample file where Firestore-specific metric names were used instead of the correct Datastore prefixes.
...e/google-cloud-datastore/src/main/java/com/google/cloud/datastore/samples/MetricsSample.java
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Datastore metrics implementation to align with the GAX library's telemetry framework. It introduces DatastoreMetricsRecorder, which extends the GAX MetricsRecorder, and updates DatastoreImpl and related classes to record metrics for both gRPC and HTTP transports. Key changes include the addition of new telemetry constants, the implementation of an OpenTelemetry-based recorder, and the removal of transport-specific logic that previously skipped metrics for gRPC. Feedback includes addressing a compilation error caused by a missing constant in TelemetryConstants.java, removing redundant attribute keys, and unifying OpenTelemetry meter names for better consistency.
...oogle-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryUtils.java
Outdated
Show resolved
Hide resolved
...e-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/google/cloud/datastore/telemetry/OpenTelemetryDatastoreMetricsRecorder.java
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Datastore telemetry metrics infrastructure by introducing a new DatastoreMetricsRecorder interface and its corresponding implementations. It replaces the previous MetricsRecorder with DatastoreMetricsRecorder, updates the metric recording logic to support both gRPC and HTTP transports, and aligns metric naming and attribute handling with internal Cloud Monitoring requirements. I have reviewed the changes and identified a stale comment in TelemetryConstants.java that references a removed constant, which should be updated for clarity.
...e-cloud-datastore/src/main/java/com/google/cloud/datastore/telemetry/TelemetryConstants.java
Show resolved
Hide resolved
lqiu96
changed the title
1 participant
Changes