Skip to content

test: The test collecting CSM for a second project should record metrics for the second project#1634

Merged
gcf-merge-on-green[bot] merged 19 commits intomainfrom
second-project-id-bug-fix
Jul 14, 2025
Merged

test: The test collecting CSM for a second project should record metrics for the second project#1634
gcf-merge-on-green[bot] merged 19 commits intomainfrom
second-project-id-bug-fix

Conversation

@danieljbruce
Copy link
Contributor

@danieljbruce danieljbruce commented Jul 7, 2025

Description

As indicated by this test run the CI pipeline is suddenly failing every time on the "should send the metrics to Google Cloud Monitoring for a ReadRows call with a second project" test. This is because the test incorrectly mocks the metric handler for the fake Bigtable client so that it doesn't include the same project as the Bigtable client. In practice this can never happen because options are always passed down from the Bigtable client to the metrics handler to the exporter as confirmed by "should pass the credentials to the exporter" test.

The assertion error occurs when we fetch the client side metrics, but there isn't any series in the returned array. This is because before with the test we were writing metrics with the default project instead of the secondary project:

image

Impact

Unblocks the CI pipeline.

Testing

The tests were changed to mock a more realistic situation.

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigtable Issues related to the googleapis/nodejs-bigtable API. labels Jul 7, 2025
@danieljbruce danieljbruce changed the title Second project id bug fix test: The test collecting CSM for a second project should record metrics for the second project Jul 7, 2025
@danieljbruce danieljbruce marked this pull request as ready for review July 7, 2025 17:20
@danieljbruce danieljbruce requested a review from a team July 7, 2025 17:20
@danieljbruce danieljbruce requested a review from a team as a code owner July 7, 2025 17:20
const metricHandler = new metricsHandlerClass(
options as unknown as ClientOptions & {value: string},
);
const newClient = new Bigtable(options);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes make it so that now projectId gets passed into the metricsHandlerClass constructor.

.catch(err => {
done(new Error('Metrics have not been published'));
done(err);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch is needed so that we can tell the test runner about the error and fail the test when this doesn't work.

@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: s Pull request size is small. labels Jul 7, 2025
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xl Pull request size is extra large. labels Jul 7, 2025
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: s Pull request size is small. labels Jul 7, 2025
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xl Pull request size is extra large. labels Jul 7, 2025
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: s Pull request size is small. labels Jul 7, 2025
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xl Pull request size is extra large. labels Jul 9, 2025
@danieljbruce danieljbruce added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 14, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 14, 2025
@gcf-merge-on-green gcf-merge-on-green bot merged commit ee00469 into main Jul 14, 2025
20 of 23 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 14, 2025
@gcf-merge-on-green gcf-merge-on-green bot deleted the second-project-id-bug-fix branch July 14, 2025 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/nodejs-bigtable API. size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants