test(bigtable): add integration tests for single-row reads - #14399
Merged
Merged
Conversation
…-row reads - BuiltinMetricsIT: add testInternalMetrics to verify per_connection_error_count is exported to Cloud Monitoring; remove gRPC DirectPath metric tests that were not universally applicable - ReadIT: add three tests (readSingleRowWithReadRow, readSingleRowWithRowKeyQuery, readSingleRowWithRowRangeQuery) covering readRow API, rowKey query, and closed/closed row range query, each asserting the exact cell content returned
mutianf
commented
Sep 16, 2026
mutianf
commented
Sep 16, 2026
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request re-enables the BuiltinMetricsIT integration tests, adds a new test for internal metrics with NotFoundException handling for lazily created descriptors, and introduces three new single-row read tests in ReadIT.java. The review feedback suggests extending the query's time interval in testInternalMetrics to 10 minutes to match the polling timeout and refactoring the repetitive setup code in the new single-row read tests into a helper method to reduce duplication.
- BuiltinMetricsIT: remove table creation in testInternalMetrics (use shared test table; per_connection_error_count is connection-level); extend query interval to 10 min to match polling budget; revert INTERNAL_INSTRUMENT_PREFIX usage in testBuiltinMetricsWithCustomOTEL - ReadIT: extract repeated write+expected-row setup into writeTestRowAndBuildExpected helper
mutianf
commented
Sep 16, 2026
Create a dedicated table with a unique prefix so metrics from this run are isolated by time window from prior runs, and delete the table in a finally block to guarantee cleanup.
Create a dedicated Bigtable instance with a unique name, run operations against it, filter the Cloud Monitoring query on the unique instance ID, and delete the instance in a finally block. This ensures metrics queried belong strictly to this test run and not prior runs on the shared instance.
The OTel instruments are registered under the full bigtable.googleapis.com/internal/client/ name, so the InMemoryMetricReader stores them under that full name. The getMetricData lookup must use the prefix to find them.
sushanb
approved these changes
Sep 16, 2026
The PR should only contain single-row read tests in ReadIT.java. Restore BuiltinMetricsIT.java to its pre-PR state (with @ignore).
mutianf
changed the title
mutianf
enabled auto-merge (squash)
September 16, 2026 19:49
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
readSingleRowWithReadRow—readRow(tableId, rowKey)APIreadSingleRowWithRowKeyQuery—readRows(Query.rowKey(key))readSingleRowWithRowRangeQuery—readRows(Query.range(closed/closed same key))Test plan
ReadITsingle-row tests passed locally (Tests run: 3, Failures: 0, Errors: 0)