Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

Commit 3e532ab

Browse files
danieljbrucedanieljbruce
authored
fix: Client side metrics should record the version number of @google-cloud/bigtable not the customer package version (#1752)
## Description A bug was discovered where the client side metrics being recorded on the backend service had the wrong version number. The version number being recorded actually matched the version number on the customer's package.json file instead of the version number of the Node Bigtable package being used. To correct this, the line of code at https://togithub.com/googleapis/nodejs-bigtable/blob/2a960922747ed0f8d0e798113fd4434852382217/src/client-side-metrics/operation-metrics-collector.ts#L52 which is currently reading the package.json file relative to the working directory of the node process should instead read the package.json file location relative to the location of operation-metrics-collector.ts so that the version number of the Node Bigtable dependency is always recorded instead of the version number from the customer's package.json file. ## Impact Corrects the bug and ensures that the right version number is always recorded for client side metrics. ## Files affected **samples/readSnippets:** A code snippet that will run in the samples test and fail if the metrics handler receives the wrong version of the client library **samples/test/reads.js:** The new samples test that runs the code snippet in readSnippets and fails if the metrics handler receives the wrong version of the client library **src/client-side-metrics/operation-metrics-collector.ts:** The source code changes that ensure the version number recorded will always match the Bigtable client library version number **test-common/test-metrics-handler.ts:** A new class, similar to the existing class for testing data sent to the metrics handler that also preserves the version name when it reaches the metrics handler ## Testing A new system test or unit test will not catch this bug since their working directories all reference the package.json file in the nodejs-bigtable project. So instead, I created a fake customer project with the script from https://gist.github.com/danieljbruce/bba73fa5d9919866d82b8968ceb45cc3 which gets the wrong version number at https://togithub.com/googleapis/nodejs-bigtable/blob/2a960922747ed0f8d0e798113fd4434852382217/src/client-side-metrics/operation-metrics-collector.ts#L52, but gets the right version number when this line of code is corrected with the changes in this PR. I also found a way to create a samples test which will fail without the source code changes from this PR. It should ensure going forward that the right version gets recorded for client side metrics.
1 parent be5b598 commit 3e532ab

4 files changed

Lines changed: 116 additions & 2 deletions

File tree

‎src/client-side-metrics/operation-metrics-collector.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ export interface ITabularApiSurface {
5050
};
5151
}
5252

53-
const packageJSON = fs.readFileSync('package.json');
54-
const version = JSON.parse(packageJSON.toString()).version;
53+
const {version} = require('../../../package.json');
5554

5655
// MetricsCollectorState is a list of states that the metrics collector can be in.
5756
// Tracking the OperationMetricsCollector state is done so that the

‎test/metrics-collector/version/get-version-script.js‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
const path = require('path');
16+
const fs = require('fs');
17+
const {
18+
ClientSideMetricsConfigManager,
19+
} = require('../../../build/src/client-side-metrics/metrics-config-manager.js'); // eslint-disable-line
20+
const {Bigtable} = require('../../../build/src'); // eslint-disable-line
21+
22+
async function main() {
23+
/**
24+
* A test implementation of the IMetricsHandler interface. Used for testing purposes.
25+
* It logs the metrics and attributes received by the onOperationComplete and onAttemptComplete methods.
26+
* Doesn't replace the name of the client like the other testing class.
27+
*/
28+
class TestMetricsHandlerKeepName {
29+
constructor() {
30+
this.messages = {value: ''};
31+
this.projectId = 'projectId';
32+
this.requestsHandled = [];
33+
}
34+
/**
35+
* Logs the metrics and attributes received for an operation completion.
36+
* @param {OnOperationCompleteData} data Metrics related to the completed operation.
37+
*/
38+
onOperationComplete(data) {
39+
const dataWithProject = Object.assign({projectId: this.projectId}, data);
40+
this.requestsHandled.push(dataWithProject);
41+
this.messages.value += 'Recording parameters for onOperationComplete:\n';
42+
this.messages.value += `${JSON.stringify(dataWithProject)}\n`;
43+
}
44+
45+
/**
46+
* Logs the metrics and attributes received for an attempt completion.
47+
* @param {OnOperationCompleteData} data Metrics related to the completed attempt.
48+
*/
49+
onAttemptComplete(data) {
50+
const dataWithProject = Object.assign({projectId: this.projectId}, data);
51+
this.requestsHandled.push(dataWithProject);
52+
this.messages.value += 'Recording parameters for onAttemptComplete:\n';
53+
this.messages.value += `${JSON.stringify(dataWithProject)}\n`;
54+
}
55+
}
56+
57+
const packagePath = path.join(__dirname, '../../../package.json');
58+
59+
// Read the file using the absolute path
60+
const packageJSON = fs.readFileSync(packagePath);
61+
const expectedVersion = JSON.parse(packageJSON.toString()).version;
62+
63+
const fakeBigtable = new Bigtable();
64+
const testMetricsHandler = new TestMetricsHandlerKeepName();
65+
fakeBigtable._metricsConfigManager = new ClientSideMetricsConfigManager([
66+
testMetricsHandler,
67+
]);
68+
69+
const instance = fakeBigtable.instance('fake-instance-id');
70+
const table = instance.table('fake-table-id');
71+
try {
72+
await table.getRows();
73+
} catch (e) {
74+
// Suppress the error.
75+
// We just made this call so that the test metrics handler would
76+
// collect grpc response data.
77+
}
78+
if (
79+
testMetricsHandler.requestsHandled[0].client_name !==
80+
`nodejs-bigtable/${expectedVersion}`
81+
) {
82+
throw Error('The wrong version is being recorded');
83+
}
84+
}
85+
main();

‎test/metrics-collector/version/package.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"version": "fake-version-number",
3+
"private": true,
4+
"engines": {
5+
"node": ">=18"
6+
}
7+
}

‎test/metrics-collector/version/version.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// eslint-disable-next-line n/no-extraneous-import
16+
import {describe} from 'mocha';
17+
import {execSync} from 'node:child_process';
18+
19+
describe('Bigtable/CSMVersion', () => {
20+
it('Fetches the right client side metrics version', async () => {
21+
execSync('cd test/metrics-collector/version && node get-version-script');
22+
});
23+
});

0 commit comments

Comments
 (0)