Skip to content

Commit 623a140

Browse files
authored
[telemetry improvement]: emit sql_operation, auth_type, driver_connection_params - ship even if null (#396)
* feat(telemetry): populate sql_operation, auth_type, and driver_connection_params Aligns the Node.js telemetry payload with the receiver schema (JDBC parity). Several fields were being collected by the producer/aggregator but dropped on the floor in the exporter: - `sql_operation.operation_detail.operation_type` (CREATE_SESSION / DELETE_SESSION / EXECUTE_STATEMENT / LIST_*) on both connection and statement events. - `sql_operation.is_compressed` on statement events when CloudFetch compression observability is available. - `sql_operation.execution_result` now resolves to `FORMAT_UNSPECIFIED` when result-set metadata isn't available (DDL/DML, or SELECTs closed without fetching), so the `sql_operation` block fires on every statement-complete event instead of being skipped. - Top-level `auth_type` from `DriverConfiguration.authType`. - New `driver_connection_params` block carrying `host_info.host_url`, `http_path`, `enable_arrow`, `enable_direct_results`, `socket_timeout`, `enable_metric_view_metadata`, `cloud_fetch_enabled`, `lz4_enabled`, `retry_max_attempts`, `cloud_fetch_concurrent_downloads`. Both `auth_type` and `driver_connection_params` are gated behind the existing authenticated-export guard (same path as `system_configuration`), since `host_url` and `http_path` are workspace-correlated identifiers that must not ship on the unauthenticated endpoint. Co-authored-by: Isaac * telemetry: restrict driver_connection_params to proto-defined fields Drop four Node-specific fields that aren't in the receiver's `DriverConnectionParameters` proto schema: - cloud_fetch_enabled - lz4_enabled - retry_max_attempts - cloud_fetch_concurrent_downloads These would be ignored at deserialization on the receiver side anyway. Remaining block contains only proto-defined fields: `host_info`, `http_path`, `enable_arrow`, `enable_direct_results`, `socket_timeout`, `enable_metric_view_metadata`. Co-authored-by: Isaac * telemetry: populate mode and use_proxy in driver_connection_params The four Node-specific fields dropped in the previous commit have no proto equivalent (CloudFetch enablement, LZ4, retry config, concurrent downloads aren't tracked at the receiver). But two unrelated proto fields can be filled from existing Node config: - `mode = "THRIFT"` — Node.js always uses Thrift transport (no SEA client). JDBC's `DatabricksClientType` enum is `SEA | THRIFT`. - `use_proxy` — derived from `ConnectionOptions.proxy`. Stored on DBSQLClient at `connect()` time alongside the existing host / httpPath / authType fields, then surfaced through a new `DriverConfiguration.useProxy` field. Co-authored-by: Isaac * telemetry: emit socket_timeout in seconds to match receiver proto The driver tracks socketTimeout in milliseconds, but the receiver proto field socket_timeout is defined in seconds. Convert ms -> s at export so a 15-minute timeout reports as 900s instead of 900000s. Adds regression tests covering the default value and sub-second rounding. Co-authored-by: Isaac
1 parent cd0117f commit 623a140

5 files changed

Lines changed: 107 additions & 3 deletions

File tree

lib/DBSQLClient.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
112112

113113
private authType?: string;
114114

115+
private useProxy?: boolean;
116+
115117
private telemetryClient?: TelemetryClient;
116118

117119
private telemetryEmitter?: TelemetryEventEmitter;
@@ -415,6 +417,7 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
415417
// Connection parameters
416418
httpPath: this.httpPath,
417419
enableMetricViewMetadata: this.config.enableMetricViewMetadata,
420+
useProxy: this.useProxy,
418421
};
419422
}
420423

@@ -594,6 +597,7 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
594597
this.host = options.host;
595598
this.httpPath = options.path;
596599
this.authType = this.mapAuthType(options);
600+
this.useProxy = Boolean(options.proxy);
597601

598602
// Store enableMetricViewMetadata configuration
599603
if (options.enableMetricViewMetadata !== undefined) {

lib/telemetry/DatabricksTelemetryExporter.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,24 @@ interface DatabricksTelemetryLog {
6060
char_set_encoding?: string;
6161
process_name?: string;
6262
};
63+
auth_type?: string;
64+
driver_connection_params?: {
65+
host_info?: { host_url?: string };
66+
http_path?: string;
67+
mode?: string;
68+
use_proxy?: boolean;
69+
enable_arrow?: boolean;
70+
enable_direct_results?: boolean;
71+
socket_timeout?: number;
72+
enable_metric_view_metadata?: boolean;
73+
};
6374
operation_latency_ms?: number;
6475
sql_operation?: {
6576
execution_result?: string;
77+
is_compressed?: boolean;
78+
operation_detail?: {
79+
operation_type?: string;
80+
};
6681
chunk_details?: {
6782
total_chunks_present?: number;
6883
total_chunks_iterated?: number;
@@ -368,6 +383,11 @@ export default class DatabricksTelemetryExporter {
368383
if (metric.latencyMs !== undefined) {
369384
log.entry.sql_driver_log.operation_latency_ms = metric.latencyMs;
370385
}
386+
if (metric.operationType) {
387+
log.entry.sql_driver_log.sql_operation = {
388+
operation_detail: { operation_type: metric.operationType },
389+
};
390+
}
371391
if (metric.driverConfig && includeCorrelation) {
372392
// system_configuration is a high-entropy client fingerprint (OS, arch,
373393
// locale, process, runtime). Only ship on the authenticated path.
@@ -384,15 +404,47 @@ export default class DatabricksTelemetryExporter {
384404
char_set_encoding: metric.driverConfig.charSetEncoding,
385405
process_name: sanitizeProcessName(metric.driverConfig.processName) || undefined,
386406
};
407+
408+
// auth_type and host/http-path are workspace-correlated, so they ride
409+
// the same auth-only path as system_configuration.
410+
if (metric.driverConfig.authType) {
411+
log.entry.sql_driver_log.auth_type = metric.driverConfig.authType;
412+
}
413+
log.entry.sql_driver_log.driver_connection_params = {
414+
host_info: { host_url: this.host },
415+
http_path: metric.driverConfig.httpPath,
416+
mode: 'THRIFT',
417+
use_proxy: metric.driverConfig.useProxy,
418+
enable_arrow: metric.driverConfig.arrowEnabled,
419+
enable_direct_results: metric.driverConfig.directResultsEnabled,
420+
// The proto `socket_timeout` field is defined in seconds, but the driver
421+
// tracks socketTimeout in milliseconds — convert so the receiver records
422+
// the correct unit (e.g. 900000ms -> 900s) instead of treating ms as seconds.
423+
socket_timeout:
424+
typeof metric.driverConfig.socketTimeout === 'number'
425+
? Math.round(metric.driverConfig.socketTimeout / 1000)
426+
: metric.driverConfig.socketTimeout,
427+
enable_metric_view_metadata: metric.driverConfig.enableMetricViewMetadata,
428+
};
387429
}
388430
} else if (metric.metricType === 'statement') {
389431
log.entry.sql_driver_log.operation_latency_ms = metric.latencyMs;
390432

391-
if (metric.resultFormat || metric.chunkCount) {
433+
if (metric.resultFormat || metric.chunkCount || metric.operationType || metric.compressed !== undefined) {
392434
log.entry.sql_driver_log.sql_operation = {
393435
execution_result: metric.resultFormat,
394436
};
395437

438+
if (metric.compressed !== undefined) {
439+
log.entry.sql_driver_log.sql_operation.is_compressed = metric.compressed;
440+
}
441+
442+
if (metric.operationType) {
443+
log.entry.sql_driver_log.sql_operation.operation_detail = {
444+
operation_type: metric.operationType,
445+
};
446+
}
447+
396448
if ((metric.chunkCount ?? 0) > 0) {
397449
log.entry.sql_driver_log.sql_operation.chunk_details = {
398450
total_chunks_present: metric.chunkCount,

lib/telemetry/telemetryTypeMappers.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ export function mapOperationTypeToTelemetryType(operationType?: TOperationType):
5151
/**
5252
* Map Thrift TSparkRowSetType to telemetry ExecutionResult.Format enum string.
5353
*/
54-
export function mapResultFormatToTelemetryType(resultFormat?: TSparkRowSetType): string | undefined {
54+
export function mapResultFormatToTelemetryType(resultFormat?: TSparkRowSetType): string {
5555
if (resultFormat === undefined) {
56-
return undefined;
56+
return 'FORMAT_UNSPECIFIED';
5757
}
5858

5959
switch (resultFormat) {

lib/telemetry/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,9 @@ export interface DriverConfiguration {
319319

320320
/** Whether metric view metadata is enabled */
321321
enableMetricViewMetadata?: boolean;
322+
323+
/** Whether an HTTP/SOCKS proxy is configured on the connection */
324+
useProxy?: boolean;
322325
}
323326

324327
/**

tests/unit/telemetry/DatabricksTelemetryExporter.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,51 @@ describe('DatabricksTelemetryExporter', () => {
162162
});
163163
});
164164

165+
describe('export() - driver_connection_params', () => {
166+
// The driver tracks socketTimeout in milliseconds, but the receiver proto
167+
// defines `socket_timeout` in seconds. Lock in the ms -> s conversion so a
168+
// 15-minute (900000ms) timeout is reported as 900s, not 900000s.
169+
function getConnectionParams(sendRequestStub: sinon.SinonStub): any {
170+
const init = sendRequestStub.firstCall.args[1] as { body: string };
171+
const body = JSON.parse(init.body);
172+
const log = JSON.parse(body.protoLogs[0]);
173+
return log.entry.sql_driver_log.driver_connection_params;
174+
}
175+
176+
function makeConnectionMetric(socketTimeout: number): TelemetryMetric {
177+
return makeMetric({
178+
metricType: 'connection',
179+
driverConfig: {
180+
driverName: 'nodejs-sql-driver',
181+
driverVersion: '1.14.0',
182+
socketTimeout,
183+
} as any,
184+
});
185+
}
186+
187+
it('converts socketTimeout from milliseconds to seconds', async () => {
188+
const context = new ClientContextStub({ telemetryAuthenticatedExport: true } as any);
189+
const registry = new CircuitBreakerRegistry(context);
190+
const exporter = new DatabricksTelemetryExporter(context, 'host.example.com', registry, fakeAuthProvider);
191+
const sendRequestStub = sinon.stub(exporter as any, 'sendRequest').returns(makeOkResponse());
192+
193+
await exporter.export([makeConnectionMetric(900000)]);
194+
195+
expect(getConnectionParams(sendRequestStub).socket_timeout).to.equal(900);
196+
});
197+
198+
it('rounds sub-second socketTimeout values', async () => {
199+
const context = new ClientContextStub({ telemetryAuthenticatedExport: true } as any);
200+
const registry = new CircuitBreakerRegistry(context);
201+
const exporter = new DatabricksTelemetryExporter(context, 'host.example.com', registry, fakeAuthProvider);
202+
const sendRequestStub = sinon.stub(exporter as any, 'sendRequest').returns(makeOkResponse());
203+
204+
await exporter.export([makeConnectionMetric(1500)]);
205+
206+
expect(getConnectionParams(sendRequestStub).socket_timeout).to.equal(2);
207+
});
208+
});
209+
165210
describe('export() - retry logic', () => {
166211
it('should retry on retryable HTTP errors (503)', async () => {
167212
const context = new ClientContextStub({ telemetryMaxRetries: 2 } as any);

0 commit comments

Comments
 (0)