diff --git a/src/client-side-metrics/gcp-metrics-handler.ts b/src/client-side-metrics/gcp-metrics-handler.ts index 7ecc29ce1..6a9231760 100644 --- a/src/client-side-metrics/gcp-metrics-handler.ts +++ b/src/client-side-metrics/gcp-metrics-handler.ts @@ -31,6 +31,7 @@ const { } = require('@opentelemetry/sdk-metrics'); import * as os from 'os'; import * as crypto from 'crypto'; +import {MethodName} from './client-side-metrics-attributes'; /** * Generates a unique client identifier string. @@ -251,10 +252,16 @@ export class GCPMetricsHandler implements IMetricsHandler { status: data.status, ...commonAttributes, }); - otelInstruments.firstResponseLatencies.record(data.firstResponseLatency, { - status: data.status, - ...commonAttributes, - }); + if ( + data.metricsCollectorData.method === MethodName.READ_ROWS || + data.metricsCollectorData.method === MethodName.READ_ROW + ) { + otelInstruments.firstResponseLatencies.record(data.firstResponseLatency, { + status: data.status, + ...commonAttributes, + }); + } + if (data.applicationLatency) { otelInstruments.applicationBlockingLatencies.record( data.applicationLatency, diff --git a/src/client-side-metrics/operation-metrics-collector.ts b/src/client-side-metrics/operation-metrics-collector.ts index 930ba070e..4b14bc0ea 100644 --- a/src/client-side-metrics/operation-metrics-collector.ts +++ b/src/client-side-metrics/operation-metrics-collector.ts @@ -181,10 +181,7 @@ export class OperationMetricsCollector { }) => { this.onStatusMetadataReceived(status); }, - ) - .on('data', () => { - this.onResponse(); - }); + ); } /** @@ -310,7 +307,7 @@ export class OperationMetricsCollector { client_name: `nodejs-bigtable/${version}`, operationLatency: totalMilliseconds, retryCount: this.attemptCount - 1, - firstResponseLatency: this.firstResponseLatency ?? undefined, + firstResponseLatency: this.firstResponseLatency ?? 0, applicationLatency: applicationLatency ?? 0, }); } diff --git a/src/utils/createReadStreamInternal.ts b/src/utils/createReadStreamInternal.ts index 73d6f1f99..7adee66df 100644 --- a/src/utils/createReadStreamInternal.ts +++ b/src/utils/createReadStreamInternal.ts @@ -320,6 +320,10 @@ export function createReadStreamInternal( gaxOpts, retryOpts, }); + requestStream.on('data', () => { + // This handler is necessary for recording firstResponseLatencies. + metricsCollector.onResponse(); + }); activeRequestStream = requestStream!; diff --git a/system-test/client-side-metrics-all-methods.ts b/system-test/client-side-metrics-all-methods.ts index 8b64746a1..e50b57931 100644 --- a/system-test/client-side-metrics-all-methods.ts +++ b/system-test/client-side-metrics-all-methods.ts @@ -37,6 +37,7 @@ import { import {ClientOptions} from 'google-gax'; import {ClientSideMetricsConfigManager} from '../src/client-side-metrics/metrics-config-manager'; import {MetricServiceClient} from '@google-cloud/monitoring'; +import {MethodName} from '../src/client-side-metrics/client-side-metrics-attributes'; const SECOND_PROJECT_ID = 'cfdb-sdk-node-tests'; @@ -66,6 +67,25 @@ function getHandlerFromExporter(Exporter: typeof CloudMonitoringExporter) { }).GCPMetricsHandler; } +function checkFirstResponseLatency(requestHandled: OnOperationCompleteData) { + assert( + Object.prototype.hasOwnProperty.call( + requestHandled, + 'firstResponseLatency', + ), + ); + if ( + requestHandled.metricsCollectorData.method === MethodName.READ_ROWS || + requestHandled.metricsCollectorData.method === MethodName.READ_ROW + ) { + assert(requestHandled.firstResponseLatency); + assert(requestHandled.firstResponseLatency > 0); + } else { + assert.strictEqual(requestHandled.firstResponseLatency, 0); + } + delete requestHandled.firstResponseLatency; +} + function readRowsAssertionCheck( projectId: string, requestsHandled: (OnOperationCompleteData | OnAttemptCompleteData)[] = [], @@ -98,11 +118,10 @@ function readRowsAssertionCheck( const secondRequest = requestsHandled[1] as any; // We would expect these parameters to be different every time so delete // them from the comparison after checking they exist. + checkFirstResponseLatency(secondRequest); assert(secondRequest.operationLatency); - assert(secondRequest.firstResponseLatency); - assert.strictEqual(secondRequest.applicationLatency, 0); delete secondRequest.operationLatency; - delete secondRequest.firstResponseLatency; + assert(secondRequest.applicationLatency < 10); delete secondRequest.applicationLatency; delete secondRequest.metricsCollectorData.appProfileId; assert.deepStrictEqual(secondRequest, { @@ -144,11 +163,10 @@ function readRowsAssertionCheck( const fourthRequest = requestsHandled[3] as any; // We would expect these parameters to be different every time so delete // them from the comparison after checking they exist. + checkFirstResponseLatency(fourthRequest); assert(fourthRequest.operationLatency); - assert(fourthRequest.firstResponseLatency); - assert.strictEqual(fourthRequest.applicationLatency, 0); + assert(fourthRequest.applicationLatency < 10); delete fourthRequest.operationLatency; - delete fourthRequest.firstResponseLatency; delete fourthRequest.applicationLatency; delete fourthRequest.metricsCollectorData.appProfileId; assert.deepStrictEqual(fourthRequest, {