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

Commit a719d9f

Browse files
danieljbrucedanieljbruce
authored
fix: Correct project id on client side metrics by avoiding getProjectId calls with the metric service client (#1757)
## Description The project id being recorded for client side metrics is incorrect. The client side metrics monitoring tests are showing that the project id the metrics get recorded for does not actually match the project id the client makes a grpc call for. Specifically, values recorded for `bigtable.googleapis.com/frontend_server/handler_latencies` don't match values recorded for `bigtable.googleapis.com/internal/client/attempt_latencies`, but they should match. There is some speculation that since `getProjectId` calls on the `MetricServiceClient` determine the projectId recorded for client side metrics that the incorrect projectId is due to the differences between this `getProjectId` call on the `MetricServiceClient` and the way the project id is fetched on the Bigtable client. Therefore, we introduce this PR to record a projectId that is certain to be an exact match of the project id on the Bigtable client. ## Impact This has a high probability of correcting the project id for the client side metrics ## Testing The fixtures have changed to accommodate for the fact that we now pass the projectId through the OTEL stack.
1 parent 3e532ab commit a719d9f

12 files changed

Lines changed: 67 additions & 44 deletions

‎src/client-side-metrics/exporter.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ function getIntegerPoints(dataPoint: DataPoint<number>) {
123123
* send to the Google Cloud Monitoring dashboard
124124
*/
125125
function getResource(
126-
projectId: string,
127126
dataPoint:
128127
| DataPoint<number>
129128
| DataPoint<Histogram>
@@ -132,7 +131,7 @@ function getResource(
132131
const resourceLabels = {
133132
cluster: dataPoint.attributes.cluster,
134133
instance: dataPoint.attributes.instanceId,
135-
project_id: projectId,
134+
project_id: dataPoint.attributes.projectId,
136135
table: dataPoint.attributes.table,
137136
zone: dataPoint.attributes.zone,
138137
};
@@ -183,7 +182,6 @@ function getMetric(
183182
* metric attributes, data points, and aggregation information, into an object
184183
* that conforms to the expected request format of the Cloud Monitoring API.
185184
*
186-
* @param projectId
187185
* @param {ResourceMetrics} exportArgs - The OpenTelemetry metrics data to be converted. This
188186
* object contains resource attributes, scope information, and a list of
189187
* metrics with their associated data points.
@@ -211,16 +209,17 @@ function getMetric(
211209
*
212210
*
213211
*/
214-
export function metricsToRequest(
215-
projectId: string,
216-
exportArgs: ResourceMetrics,
217-
) {
212+
export function metricsToRequest(exportArgs: ResourceMetrics) {
218213
const timeSeriesArray = [];
214+
let projectId;
219215
for (const scopeMetrics of exportArgs.scopeMetrics) {
220216
for (const scopeMetric of scopeMetrics.metrics) {
221217
for (const dataPoint of scopeMetric.dataPoints) {
218+
if (!projectId) {
219+
projectId = dataPoint.attributes.projectId;
220+
}
222221
const metric = getMetric(scopeMetric.descriptor.name, dataPoint);
223-
const resource = getResource(projectId, dataPoint);
222+
const resource = getResource(dataPoint);
224223
if (isCounterValue(dataPoint)) {
225224
timeSeriesArray.push({
226225
metric,
@@ -310,8 +309,7 @@ export class CloudMonitoringExporter extends MetricExporter {
310309
): Promise<void> {
311310
(async () => {
312311
try {
313-
const projectId = await this.client.getProjectId();
314-
const request = metricsToRequest(projectId, metrics);
312+
const request = metricsToRequest(metrics);
315313
await this.client.createServiceTimeSeries(
316314
request as ICreateTimeSeriesRequest,
317315
);

‎src/client-side-metrics/gcp-metrics-handler.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ export class GCPMetricsHandler implements IMetricsHandler {
238238
method: data.metricsCollectorData.method,
239239
client_uid: this.clientUid,
240240
client_name: data.client_name,
241+
projectId: data.metricsCollectorData.projectId,
241242
instanceId: data.metricsCollectorData.instanceId,
242243
table: data.metricsCollectorData.table,
243244
cluster: data.metricsCollectorData.cluster,
@@ -285,6 +286,7 @@ export class GCPMetricsHandler implements IMetricsHandler {
285286
client_uid: this.clientUid,
286287
status: data.status,
287288
client_name: data.client_name,
289+
projectId: data.metricsCollectorData.projectId,
288290
instanceId: data.metricsCollectorData.instanceId,
289291
table: data.metricsCollectorData.table,
290292
cluster: data.metricsCollectorData.cluster,

‎src/client-side-metrics/metrics-handler.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {MethodName, StreamingState} from './client-side-metrics-attributes';
2121
*/
2222

2323
type IMetricsCollectorData = {
24+
projectId: string;
2425
instanceId: string;
2526
table: string;
2627
cluster?: string;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ export class OperationMetricsCollector {
155155
cluster: this.cluster || '<unspecified>',
156156
zone: this.zone || 'global',
157157
method: this.methodName,
158+
projectId: this.tabularApiSurface.bigtable.projectId as string,
158159
},
159160
appProfileId ? {app_profile: appProfileId} : {},
160161
);

‎system-test/client-side-metrics-all-methods.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ function readRowsAssertionCheck(
118118
cluster: 'fake-cluster3',
119119
zone: 'us-west1-c',
120120
method,
121+
projectId,
121122
},
122-
projectId,
123123
});
124124
const secondRequest = requestsHandled[1] as any;
125125
// We would expect these parameters to be different every time so delete
@@ -140,8 +140,8 @@ function readRowsAssertionCheck(
140140
zone: 'us-west1-c',
141141
method,
142142
table: 'my-table',
143+
projectId,
143144
},
144-
projectId,
145145
retryCount: 0,
146146
});
147147
// We would expect these parameters to be different every time so delete
@@ -163,8 +163,8 @@ function readRowsAssertionCheck(
163163
cluster: 'fake-cluster3',
164164
zone: 'us-west1-c',
165165
method,
166+
projectId,
166167
},
167-
projectId,
168168
});
169169
const fourthRequest = requestsHandled[3] as any;
170170
// We would expect these parameters to be different every time so delete
@@ -185,8 +185,8 @@ function readRowsAssertionCheck(
185185
zone: 'us-west1-c',
186186
method,
187187
table: 'my-table2',
188+
projectId,
188189
},
189-
projectId,
190190
retryCount: 0,
191191
});
192192
}

‎system-test/client-side-metrics.ts‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ function readRowsAssertionCheck(
184184
cluster: 'fake-cluster3',
185185
zone: 'us-west1-c',
186186
method,
187+
projectId,
187188
},
188-
projectId,
189189
});
190190
const secondRequest = requestsHandled[1] as any;
191191
// We would expect these parameters to be different every time so delete
@@ -207,8 +207,8 @@ function readRowsAssertionCheck(
207207
zone: 'us-west1-c',
208208
method,
209209
table: 'my-table',
210+
projectId,
210211
},
211-
projectId,
212212
retryCount: 0,
213213
});
214214
// We would expect these parameters to be different every time so delete
@@ -230,8 +230,8 @@ function readRowsAssertionCheck(
230230
cluster: 'fake-cluster3',
231231
zone: 'us-west1-c',
232232
method,
233+
projectId,
233234
},
234-
projectId,
235235
});
236236
const fourthRequest = requestsHandled[3] as any;
237237
// We would expect these parameters to be different every time so delete
@@ -253,8 +253,8 @@ function readRowsAssertionCheck(
253253
zone: 'us-west1-c',
254254
method,
255255
table: 'my-table2',
256+
projectId,
256257
},
257-
projectId,
258258
retryCount: 0,
259259
});
260260
}
@@ -731,8 +731,8 @@ describe('Bigtable/ClientSideMetrics', () => {
731731
cluster: 'fake-cluster3',
732732
zone: 'us-west1-c',
733733
method: 'Bigtable.ReadRows',
734+
projectId,
734735
},
735-
projectId,
736736
});
737737
const secondRequest = requestsHandled[1] as any;
738738
// We would expect these parameters to be different every time so delete
@@ -754,8 +754,8 @@ describe('Bigtable/ClientSideMetrics', () => {
754754
zone: 'us-west1-c',
755755
method: 'Bigtable.ReadRows',
756756
table: 'my-table',
757+
projectId,
757758
},
758-
projectId,
759759
retryCount: 0,
760760
});
761761
// We would expect these parameters to be different every time so delete
@@ -777,8 +777,8 @@ describe('Bigtable/ClientSideMetrics', () => {
777777
cluster: 'fake-cluster3',
778778
zone: 'us-west1-c',
779779
method: 'Bigtable.ReadRows',
780+
projectId,
780781
},
781-
projectId,
782782
});
783783
const fourthRequest = requestsHandled[3] as any;
784784
// We would expect these parameters to be different every time so delete
@@ -800,8 +800,8 @@ describe('Bigtable/ClientSideMetrics', () => {
800800
zone: 'us-west1-c',
801801
method: 'Bigtable.ReadRows',
802802
table: 'my-table2',
803+
projectId,
803804
},
804-
projectId,
805805
retryCount: 0,
806806
});
807807
}
@@ -820,7 +820,6 @@ describe('Bigtable/ClientSideMetrics', () => {
820820
) {
821821
const compareValue = [
822822
{
823-
projectId,
824823
serverLatency: undefined,
825824
attemptLatency: 23000,
826825
connectivityErrorCount: 0,
@@ -833,10 +832,10 @@ describe('Bigtable/ClientSideMetrics', () => {
833832
cluster: '<unspecified>',
834833
zone: 'global',
835834
method: 'Bigtable.ReadRows',
835+
projectId,
836836
},
837837
},
838838
{
839-
projectId,
840839
status: 'OK',
841840
streaming: 'true',
842841
metricsCollectorData: {
@@ -845,6 +844,7 @@ describe('Bigtable/ClientSideMetrics', () => {
845844
cluster: '<unspecified>',
846845
zone: 'global',
847846
method: 'Bigtable.ReadRows',
847+
projectId,
848848
},
849849
client_name: 'nodejs-bigtable',
850850
operationLatency: 25000,
@@ -853,7 +853,6 @@ describe('Bigtable/ClientSideMetrics', () => {
853853
applicationLatency: 18000, // From the stream for loop
854854
},
855855
{
856-
projectId,
857856
attemptLatency: 2000,
858857
serverLatency: undefined,
859858
connectivityErrorCount: 0,
@@ -866,10 +865,10 @@ describe('Bigtable/ClientSideMetrics', () => {
866865
cluster: '<unspecified>',
867866
zone: 'global',
868867
method: 'Bigtable.ReadRows',
868+
projectId,
869869
},
870870
},
871871
{
872-
projectId,
873872
status: 'OK',
874873
streaming: 'true',
875874
metricsCollectorData: {
@@ -878,6 +877,7 @@ describe('Bigtable/ClientSideMetrics', () => {
878877
cluster: '<unspecified>',
879878
zone: 'global',
880879
method: 'Bigtable.ReadRows',
880+
projectId,
881881
},
882882
client_name: 'nodejs-bigtable',
883883
operationLatency: 4000,
@@ -903,7 +903,6 @@ describe('Bigtable/ClientSideMetrics', () => {
903903
) {
904904
const compareValue = [
905905
{
906-
projectId,
907906
serverLatency: undefined,
908907
attemptLatency: 28000,
909908
connectivityErrorCount: 0,
@@ -916,10 +915,10 @@ describe('Bigtable/ClientSideMetrics', () => {
916915
cluster: '<unspecified>',
917916
zone: 'global',
918917
method: 'Bigtable.ReadRows',
918+
projectId,
919919
},
920920
},
921921
{
922-
projectId,
923922
status: 'OK',
924923
streaming: 'true',
925924
metricsCollectorData: {
@@ -928,6 +927,7 @@ describe('Bigtable/ClientSideMetrics', () => {
928927
cluster: '<unspecified>',
929928
zone: 'global',
930929
method: 'Bigtable.ReadRows',
930+
projectId,
931931
},
932932
client_name: 'nodejs-bigtable',
933933
operationLatency: 30000,
@@ -936,7 +936,6 @@ describe('Bigtable/ClientSideMetrics', () => {
936936
applicationLatency: 16000, // From the stream for loop
937937
},
938938
{
939-
projectId,
940939
attemptLatency: 2000,
941940
serverLatency: undefined,
942941
connectivityErrorCount: 0,
@@ -949,10 +948,10 @@ describe('Bigtable/ClientSideMetrics', () => {
949948
cluster: '<unspecified>',
950949
zone: 'global',
951950
method: 'Bigtable.ReadRows',
951+
projectId,
952952
},
953953
},
954954
{
955-
projectId,
956955
status: 'OK',
957956
streaming: 'true',
958957
metricsCollectorData: {
@@ -961,6 +960,7 @@ describe('Bigtable/ClientSideMetrics', () => {
961960
cluster: '<unspecified>',
962961
zone: 'global',
963962
method: 'Bigtable.ReadRows',
963+
projectId,
964964
},
965965
client_name: 'nodejs-bigtable',
966966
operationLatency: 4000,

0 commit comments

Comments
 (0)