Align kernel telemetry defaults with binding contract

Signed-off-by: Jay Xiao <jay.xiao@databricks.com>

Author: jay-xiao446

lib/kernel/KernelAuth.ts

@@ -662,6 +662,7 @@ export function buildKernelTelemetryOptions(
     | 'telemetryCircuitBreakerThreshold'
     | 'telemetryCircuitBreakerTimeout'
   >,
+  options: Pick<ConnectionOptions, 'telemetryEnabled'> = {},
 ) {
   const telemetry: KernelTelemetryOptions = {
     driverName: DRIVER_NAME,
@@ -676,16 +677,13 @@ export function buildKernelTelemetryOptions(
     localeName: getLocaleName(),
     charSetEncoding: 'UTF-8',
     processName: getProcessName(),
-    // The JS/Thrift telemetry path always runs with a per-host circuit breaker
-    // (`CircuitBreakerRegistry` creates one unconditionally). Enable the kernel's
-    // breaker too so the `telemetryCircuitBreakerThreshold` / `...TimeoutMs` knobs
-    // forwarded below actually take effect — the napi `.d.ts` documents those two
-    // as applying only when the breaker is enabled, so if the kernel defaults it
-    // off they would silently do nothing.
-    telemetryCircuitBreakerEnabled: true,
-    telemetryEnabled: (config.telemetryEnabled ?? true) && !isTelemetryDisabledByEnv(),
   };
 
+  const envDisabled = isTelemetryDisabledByEnv();
+  if (options.telemetryEnabled !== undefined || envDisabled) {
+    telemetry.telemetryEnabled = (options.telemetryEnabled ?? true) && !envDisabled;
+  }
+
   // `batchSize`, `flushIntervalMs`, and `closeFlushTimeoutMs` share the same napi
   // contract constraint as the breaker fields below (`Must be greater than zero when
   // supplied`), so a caller-supplied `0`/negative would forward verbatim and surface
@@ -711,11 +709,10 @@ export function buildKernelTelemetryOptions(
   if (Number.isFinite(config.telemetryCloseTimeoutMs) && config.telemetryCloseTimeoutMs! > 0) {
     telemetry.telemetryCloseFlushTimeoutMs = config.telemetryCloseTimeoutMs;
   }
-  // The breaker is forced on above, and the napi contract requires threshold/timeout
-  // to be strictly positive when it is enabled. A caller-supplied `0` (or negative)
-  // would otherwise be forwarded verbatim and surface as a hard kernel `openSession`
-  // rejection, so treat any non-positive value as a misconfiguration and fall back to
-  // the kernel defaults (5 / 60000) instead.
+  // The napi contract requires threshold/timeout to be strictly positive when
+  // supplied. A caller-supplied `0` (or negative) would otherwise be forwarded
+  // verbatim and surface as a hard kernel `openSession` rejection, so treat any
+  // non-positive value as a misconfiguration and delegate to the kernel default.
   if (Number.isFinite(config.telemetryCircuitBreakerThreshold) && config.telemetryCircuitBreakerThreshold! > 0) {
     telemetry.telemetryCircuitBreakerThreshold = config.telemetryCircuitBreakerThreshold;
   }

lib/kernel/KernelBackend.ts

@@ -98,7 +98,7 @@ export default class KernelBackend implements IBackend {
     this.nativeOptions = {
       ...buildKernelConnectionOptions(options),
       ...buildKernelRetryOptions(this.context.getConfig()),
-      ...buildKernelTelemetryOptions(this.context.getConfig()),
+      ...buildKernelTelemetryOptions(this.context.getConfig(), options),
     };
 
     // Bridge the Rust kernel's `tracing` logs into the SAME `DBSQLLogger` the

tests/unit/kernel/_helpers/nativeOptions.ts

@@ -54,8 +54,6 @@ export default function expectNativeConnectionOptions(actual: unknown, expectedR
     expect(rest.localeName, 'localeName').to.be.a('string').and.not.equal('');
     expect(rest.charSetEncoding, 'charSetEncoding').to.equal('UTF-8');
     expect(rest.processName, 'processName').to.be.a('string').and.not.equal('');
-    expect(rest.telemetryEnabled, 'telemetryEnabled').to.be.a('boolean');
-    expect(rest.telemetryCircuitBreakerEnabled, 'telemetryCircuitBreakerEnabled').to.equal(true);
   }
 
   for (const key of [

tests/unit/kernel/execution.test.ts

@@ -576,6 +576,7 @@ describe('KernelBackend', () => {
         host: 'workspace.example',
         path: '/sql/1.0/warehouses/xyz',
         token: 'dapi-token',
+        telemetryEnabled: false,
       } as ConnectionOptions);
 
       await backend.openSession({});
@@ -598,7 +599,7 @@ describe('KernelBackend', () => {
       expect(args.telemetryMaxRetries).to.equal(2);
       expect(args.telemetryRetryDelayMs).to.equal(50);
       expect(args.telemetryCloseFlushTimeoutMs).to.equal(2_500);
-      expect(args.telemetryCircuitBreakerEnabled).to.equal(true);
+      expect(args.telemetryCircuitBreakerEnabled).to.equal(undefined);
       expect(args.telemetryCircuitBreakerThreshold).to.equal(3);
       expect(args.telemetryCircuitBreakerTimeoutMs).to.equal(60_000);
     } finally {
@@ -640,6 +641,40 @@ describe('KernelBackend', () => {
     }
   });
 
+  it('openSession() omits telemetryEnabled when the caller did not explicitly configure telemetry', async () => {
+    const savedEnv = process.env.DATABRICKS_TELEMETRY_DISABLED;
+    delete process.env.DATABRICKS_TELEMETRY_DISABLED;
+
+    const connection = new FakeNativeConnection();
+    const binding = makeBinding(connection);
+    const backend = new KernelBackend({
+      context: makeContext(undefined, { telemetryEnabled: true }),
+      nativeBinding: binding,
+    });
+
+    try {
+      await backend.connect({
+        host: 'workspace.example',
+        path: '/sql/1.0/warehouses/xyz',
+        token: 'dapi-token',
+      } as ConnectionOptions);
+      await backend.openSession({});
+
+      const args = binding.openSessionStub.firstCall.args[0] as {
+        telemetryEnabled?: boolean;
+        telemetryCircuitBreakerEnabled?: boolean;
+      };
+      expect(args.telemetryEnabled).to.equal(undefined);
+      expect(args.telemetryCircuitBreakerEnabled).to.equal(undefined);
+    } finally {
+      if (savedEnv === undefined) {
+        delete process.env.DATABRICKS_TELEMETRY_DISABLED;
+      } else {
+        process.env.DATABRICKS_TELEMETRY_DISABLED = savedEnv;
+      }
+    }
+  });
+
   it('openSession() serializes session-level queryTags into sessionConf.QUERY_TAGS', async () => {
     const connection = new FakeNativeConnection();
     const binding = makeBinding(connection);