-
Notifications
You must be signed in to change notification settings - Fork 50
Forward kernel telemetry options #506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
0a8904e
8c661e6
4e64646
27e4f8e
4a7198d
9ece09b
90c154e
d88359b
7f512ca
c254d95
586df79
aa192f7
0240935
0a0586d
a7b7215
cd1573c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,11 +12,16 @@ | |
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| import os from 'os'; | ||
| import { ConnectionOptions } from '../contracts/IDBSQLClient'; | ||
| import { ClientConfig } from '../contracts/IClientContext'; | ||
| import { InternalConnectionOptions } from '../contracts/InternalConnectionOptions'; | ||
| import AuthenticationError from '../errors/AuthenticationError'; | ||
| import HiveDriverError from '../errors/HiveDriverError'; | ||
| import { buildUserAgentString, normalizePemBytes } from '../utils'; | ||
| import driverVersion from '../version'; | ||
| import { DRIVER_NAME } from '../telemetry/types'; | ||
| import { sanitizeProcessName } from '../telemetry/telemetryUtils'; | ||
|
|
||
| /** | ||
| * Default local listener port for the U2M authorization-code callback. | ||
|
|
@@ -131,6 +136,32 @@ export interface KernelSessionDefaults { | |
| retryOverallTimeoutSecs?: number; | ||
| } | ||
|
|
||
| export interface KernelTelemetryOptions { | ||
| /** Driver/runtime identity forwarded to kernel-owned telemetry. */ | ||
| driverName?: string; | ||
| driverVersion?: string; | ||
| runtimeName?: string; | ||
| runtimeVersion?: string; | ||
| runtimeVendor?: string; | ||
| osName?: string; | ||
| osVersion?: string; | ||
| osArch?: string; | ||
| clientAppName?: string; | ||
| localeName?: string; | ||
| charSetEncoding?: string; | ||
| processName?: string; | ||
| /** Kernel-owned telemetry switch and batching. */ | ||
| telemetryEnabled?: boolean; | ||
| telemetryBatchSize?: number; | ||
| telemetryFlushIntervalMs?: number; | ||
| telemetryMaxRetries?: number; | ||
| telemetryRetryDelayMs?: number; | ||
| telemetryCloseFlushTimeoutMs?: number; | ||
| telemetryCircuitBreakerEnabled?: boolean; | ||
| telemetryCircuitBreakerThreshold?: number; | ||
| telemetryCircuitBreakerTimeoutMs?: number; | ||
| } | ||
|
|
||
| /** | ||
| * TLS options shared across all auth-mode variants. Mirror the napi | ||
| * binding's `ConnectionOptions.checkServerCertificate` / `.customCaCert` | ||
|
|
@@ -227,6 +258,7 @@ export interface KernelFederationOptions { | |
| export type KernelNativeConnectionOptions = KernelSessionDefaults & | ||
| KernelTlsOptions & | ||
| KernelHttpOptions & | ||
| KernelTelemetryOptions & | ||
| KernelProxyOptions & | ||
| KernelFederationOptions & | ||
| ( | ||
|
|
@@ -588,6 +620,91 @@ export function buildKernelRetryOptions(config: { | |
| return out; | ||
| } | ||
|
|
||
| function getLocaleName(env: NodeJS.ProcessEnv = process.env): string { | ||
| try { | ||
|
peco-review-bot[bot] marked this conversation as resolved.
|
||
| const lang = env.LANG || env.LC_ALL || env.LC_MESSAGES || ''; | ||
| const match = lang.match(/^([a-z]{2}_[A-Z]{2})/); | ||
| return match?.[1] ?? 'en_US'; | ||
| } catch { | ||
| return 'en_US'; | ||
| } | ||
| } | ||
|
|
||
| function getProcessName(): string { | ||
|
peco-review-bot[bot] marked this conversation as resolved.
|
||
| try { | ||
| if (process.title && process.title !== 'node') { | ||
| return sanitizeProcessName(process.title) || 'node'; | ||
| } | ||
| const scriptPath = process.argv?.[1]; | ||
| if (scriptPath) { | ||
| return sanitizeProcessName(scriptPath).replace(/\.[^.]*$/, '') || 'node'; | ||
| } | ||
| return 'node'; | ||
|
peco-review-bot[bot] marked this conversation as resolved.
|
||
| } catch { | ||
| return 'node'; | ||
| } | ||
| } | ||
|
|
||
| export function isTelemetryDisabledByEnv(env: NodeJS.ProcessEnv = process.env): boolean { | ||
| const raw = env.DATABRICKS_TELEMETRY_DISABLED; | ||
| const trimmed = typeof raw === 'string' ? raw.trim() : ''; | ||
| return trimmed.length > 0 && /^(1|true|yes|on)$/i.test(trimmed); | ||
| } | ||
|
|
||
| export function buildKernelTelemetryOptions( | ||
| config: Pick< | ||
| ClientConfig, | ||
| | 'telemetryEnabled' | ||
| | 'telemetryBatchSize' | ||
| | 'telemetryFlushIntervalMs' | ||
| | 'telemetryMaxRetries' | ||
| | 'telemetryBackoffBaseMs' | ||
| | 'telemetryCloseTimeoutMs' | ||
| | 'telemetryCircuitBreakerThreshold' | ||
| | 'telemetryCircuitBreakerTimeout' | ||
| >, | ||
| ) { | ||
| const telemetry: KernelTelemetryOptions = { | ||
| driverName: DRIVER_NAME, | ||
| driverVersion, | ||
| runtimeName: 'Node.js', | ||
| runtimeVersion: process.version, | ||
| runtimeVendor: 'Node.js Foundation', | ||
| osName: process.platform, | ||
| osVersion: os.release(), | ||
| osArch: os.arch(), | ||
| clientAppName: undefined, | ||
| localeName: getLocaleName(), | ||
|
peco-review-bot[bot] marked this conversation as resolved.
|
||
| charSetEncoding: 'UTF-8', | ||
| processName: getProcessName(), | ||
| telemetryEnabled: (config.telemetryEnabled ?? true) && !isTelemetryDisabledByEnv(), | ||
| }; | ||
|
|
||
| if (Number.isFinite(config.telemetryBatchSize)) { | ||
|
peco-review-bot[bot] marked this conversation as resolved.
Outdated
|
||
| telemetry.telemetryBatchSize = config.telemetryBatchSize; | ||
| } | ||
| if (Number.isFinite(config.telemetryFlushIntervalMs)) { | ||
| telemetry.telemetryFlushIntervalMs = config.telemetryFlushIntervalMs; | ||
| } | ||
| if (Number.isFinite(config.telemetryMaxRetries)) { | ||
|
peco-review-bot[bot] marked this conversation as resolved.
Outdated
|
||
| telemetry.telemetryMaxRetries = config.telemetryMaxRetries; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium — The Those defaults were chosen for the JS HTTP exporter's batching/backoff, not the kernel's Rust telemetry pipeline. If that override is intended, the "Omitted ⇒ kernel default" wording and the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Confirmed the reviewer is correct: DBSQLClient.getDefaultConfig() (lib/DBSQLClient.ts:170-185) always populates every telemetry field from DEFAULT_TELEMETRY_CONFIG with finite values, so the Number.isFinite guards in buildKernelTelemetryOptions always pass in the normal client flow and the kernel never uses its own tuned defaults (the omit path is only reachable from hand-built configs like the unit tests). Resolving this is a design decision with two opposite outcomes — either (a) the override is intended and the "Omitted ⇒ kernel default" docs + isFinite guards should be reworded/removed, or (b) it's unintended and getDefaultConfig should stop populating these so the kernel keeps its Rust-pipeline tuning (a behavioral change to a widely-consumed connector). Which telemetry defaults should win (JS connector vs Rust kernel) is a product/design judgment I can't make from the code and can't verify here; needs a human to decide intent before either the docs/guards or getDefaultConfig are changed. |
||
| } | ||
| if (Number.isFinite(config.telemetryBackoffBaseMs)) { | ||
| telemetry.telemetryRetryDelayMs = config.telemetryBackoffBaseMs; | ||
| } | ||
| if (Number.isFinite(config.telemetryCloseTimeoutMs)) { | ||
| telemetry.telemetryCloseFlushTimeoutMs = config.telemetryCloseTimeoutMs; | ||
| } | ||
| if (Number.isFinite(config.telemetryCircuitBreakerThreshold)) { | ||
|
peco-review-bot[bot] marked this conversation as resolved.
Outdated
|
||
| telemetry.telemetryCircuitBreakerThreshold = config.telemetryCircuitBreakerThreshold; | ||
| } | ||
| if (Number.isFinite(config.telemetryCircuitBreakerTimeout)) { | ||
| telemetry.telemetryCircuitBreakerTimeoutMs = config.telemetryCircuitBreakerTimeout; | ||
| } | ||
|
|
||
| return telemetry; | ||
| } | ||
|
|
||
| /** | ||
| * Map the public `ConnectionOptions.proxy` (`{protocol, host, port, auth}` — | ||
| * the same shape the Thrift backend accepts) onto the kernel's structured napi | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.