Feat/read modify write row metrics isolated test style changes #1642
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces a new system test for client-side metrics related to the ReadModifyWriteRow operation. The test (`system-test/isolated-read-modify-write-row-metrics.ts`) is designed to be self-contained: - It uses a `TestMetricsHandler` to capture metrics locally. - It simulates the gRPC call for ReadModifyWriteRow to avoid reliance on a running emulator for its core assertions regarding metrics collection. - It verifies that zone, cluster, and server timing are correctly extracted from simulated metadata and reported to the metrics handler. The previous non-isolated test file for this functionality has been removed. A minor modification was also made to `system-test/bigtable.ts` to ensure the global Bigtable client (used in suite-level before/after hooks) is initialized with a dummy project ID and emulator endpoint. This addresses potential 'project ID not found' errors when an emulator is used, although current test environment issues indicate the emulator is not connected.
Refactors the system test for ReadModifyWriteRow client-side metrics to use a mock gRPC server and custom interceptors. Key changes in `system-test/read-modify-write-row-interceptors.ts`: - A mock Bigtable gRPC service is implemented, providing a controllable `ReadModifyWriteRow` endpoint that can return specific responses, initial metadata (for server-timing), and trailing metadata (for zone/cluster via x-cbt-op-details). - A custom gRPC interceptor provider, `createMetricsInterceptorProvider`, is introduced. This interceptor is attached to the `bigtable.request` call and invokes the lifecycle methods of an `OperationMetricsCollector`. - The test now creates a `fakeReadModifyWriteRow` method on a Table instance. This method makes a `bigtable.request` call (configured with the custom interceptor) to the mock server. - Assertions verify that the `TestMetricsHandler` correctly receives metrics populated by the `OperationMetricsCollector` via the interceptor pipeline, including data derived from the mock server's metadata (zone, cluster, server timing). This approach allows for more thorough testing of the metrics collection mechanism, including the interceptor integration, in a controlled environment.
This commit refactors the ReadModifyWriteRow client metrics system test
(`system-test/read-modify-write-row-interceptors.ts`) to improve how the
`OperationMetricsCollector` lifecycle is managed in conjunction with the
mock gRPC server and custom interceptor.
Changes:
- The `createMetricsInterceptorProvider` function has been simplified. The
interceptor it creates is now solely responsible for relaying in-flight
gRPC data (initial metadata, response messages, final status with trailing
metadata) to the `OperationMetricsCollector` by calling `onMetadataReceived`,
`onResponse`, and `onStatusMetadataReceived`.
- The `fakeReadModifyWriteRow` method, which orchestrates the test call,
now explicitly manages the `OperationMetricsCollector`'s broader lifecycle.
It calls `onOperationStart` and `onAttemptStart` *before* the `bigtable.request()`
call to the mock server. *After* the request promise settles (either resolves
or rejects), it calls `onAttemptComplete` and `onOperationComplete` with the
final status of the RPC attempt.
This revised structure more accurately reflects the division of responsibilities:
- The calling code (simulated by `fakeReadModifyWriteRow`) manages the
overall operation and attempt lifecycle with the collector.
- The gRPC interceptor observes and passes along the data specific to the
RPC attempt as it flows through.
This should lead to more accurate testing of the metrics collection pipeline.
This commit adds detailed console.log statements throughout the `system-test/read-modify-write-row-interceptors.ts` test file. Logging has been added to: - The `fakeReadModifyWriteRow` method to trace its execution flow, particularly around the `await bigtable.request()` call. - The `createMetricsInterceptorProvider` function and the methods of the gRPC `InterceptingCall` object it returns (e.g., `start`, `onReceiveMetadata`, `onReceiveMessage`, `onReceiveStatus`). - The `MockBigtableService.ReadModifyWriteRow` method to track when the mock server is hit and what it's attempting to send. These logs are intended to help diagnose potential issues with the execution order, such as the `await bigtable.request()` call completing prematurely or interceptor hooks not being triggered as expected. This will be useful once the test suite's environment allows the test file to be reached (i.e., global hooks requiring emulator connection can pass).
…tps://github.com/googleapis/nodejs-bigtable into feat/read-modify-write-row-metrics-isolated-test # Conflicts: # system-test/read-modify-write-row-interceptors.ts
…tps://github.com/googleapis/nodejs-bigtable into feat/read-modify-write-row-metrics-isolated-test
This reverts commit 5a92b25.
This reverts commit a23b5d7.
product-auto-label
bot
added
size: xl
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR intends to stack on top of #1631 and provide some changes to simplify the code.
Impact
Better factoring
Testing
Functionality doesn't change