Skip to content

Commit ee00469

Browse files
authored
test: The test collecting CSM for a second project should record metrics for the second project (#1634)
## Description As indicated by [this test run](https://btx.cloud.google.com/invocations/d46a4997-1695-4527-b934-77b7a53e369c/targets/cloud-devrel%2Fclient-libraries%2Fnodejs%2Fpresubmit%2Fgoogleapis%2Fnodejs-bigtable%2Fnode18%2Fsystem-test/log) 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](https://togithub.com/user-attachments/assets/94b50dea-c9e5-4e0c-a4dc-98a066d2cc9d) ## Impact Unblocks the CI pipeline. ## Testing The tests were changed to mock a more realistic situation.
1 parent 6cb7cdd commit ee00469

File tree

1 file changed

+17
-10
lines changed

1 file changed

+17
-10
lines changed

system-test/client-side-metrics.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
OnAttemptCompleteData,
3131
OnOperationCompleteData,
3232
} from '../src/client-side-metrics/metrics-handler';
33-
import {ClientOptions, ServiceError} from 'google-gax';
33+
import {ClientOptions} from 'google-gax';
3434
import {ClientSideMetricsConfigManager} from '../src/client-side-metrics/metrics-config-manager';
3535
import {MetricServiceClient} from '@google-cloud/monitoring';
3636

@@ -41,13 +41,15 @@ function getFakeBigtable(
4141
metricsHandlerClass: typeof GCPMetricsHandler | typeof TestMetricsHandler,
4242
apiEndpoint?: string,
4343
) {
44-
const metricHandler = new metricsHandlerClass({
45-
apiEndpoint,
46-
} as unknown as ClientOptions & {value: string});
47-
const newClient = new Bigtable({
44+
// Normally the options passed into the client are passed into the metrics
45+
// handler so when we mock out the metrics handler, it really should have
46+
// the same options that are passed into the client.
47+
const options = {
4848
projectId,
4949
apiEndpoint,
50-
});
50+
};
51+
const metricHandler = new metricsHandlerClass(options);
52+
const newClient = new Bigtable(options);
5153
newClient._metricsConfigManager = new ClientSideMetricsConfigManager([
5254
metricHandler,
5355
]);
@@ -205,7 +207,7 @@ function checkSingleRowCall(
205207
* Cloud Monitoring.
206208
*/
207209
async function checkForPublishedMetrics(projectId: string) {
208-
const monitoringClient = new MetricServiceClient(); // Correct instantiation
210+
const monitoringClient = new MetricServiceClient({projectId}); // Correct instantiation
209211
const now = Math.floor(Date.now() / 1000);
210212
const filters = [
211213
'metric.type="bigtable.googleapis.com/client/attempt_latencies"',
@@ -329,9 +331,14 @@ describe('Bigtable/ClientSideMetrics', () => {
329331
// result from calling export was successful.
330332
assert.strictEqual(result.code, 0);
331333
resultCallback({code: 0});
332-
void checkForPublishedMetrics(projectId).then(() => {
333-
done();
334-
});
334+
void checkForPublishedMetrics(projectId)
335+
.then(() => {
336+
done();
337+
})
338+
.catch(err => {
339+
done(new Error('Metrics have not been published'));
340+
done(err);
341+
});
335342
} catch (error) {
336343
// The code here isn't 0 so we report the original error to the mocha test runner.
337344
done(result);

0 commit comments

Comments
 (0)