From a404eec60db2f7151aa45bf80fef48764404a22c Mon Sep 17 00:00:00 2001 From: Jay Xiao Date: Fri, 28 Aug 2026 01:12:25 +0000 Subject: [PATCH] Leave kernel telemetry default unset Signed-off-by: Jay Xiao --- CONNECTION_PARAMETERS.md | 14 ++++++++------ README.md | 5 +++-- doc.go | 3 ++- internal/backend/kernel/config.go | 5 +++-- kernel_config.go | 10 +++++----- kernel_config_test.go | 16 ++++++---------- 6 files changed, 27 insertions(+), 26 deletions(-) diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index a2008c0f..4d7f1896 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -213,21 +213,23 @@ left unset (the default), a server-side feature flag decides whether wrapper tel active; setting it explicitly overrides the flag. On the kernel backend, the Go wrapper skips its telemetry interceptor so it does not duplicate kernel-owned telemetry for the same connection and statements, and forwards the kernel-owned telemetry knobs into the -kernel config. +kernel config only when `enableTelemetry` is explicitly supplied. | DSN parameter | Thrift | Kernel | Default | Notes | |---|:---:|:---:|---|---| -| `enableTelemetry` | ✅ | ✅ | unset (server flag decides wrapper telemetry; kernel telemetry defaults on) | Force Go wrapper telemetry on/off on the Thrift path, overriding the server feature flag. On the kernel path, forwarded to kernel-owned telemetry; unset forwards enabled. | -| `telemetry_batch_size` | ✅ | ✅ | `200` wrapper default; kernel default when unset | Events per batch. Forwarded to the kernel only when explicitly set. | -| `telemetry_flush_interval` | ✅ | ✅ | `30s` wrapper default; kernel default when unset | Flush interval. Forwarded to the kernel only when explicitly set. | +| `enableTelemetry` | ✅ | ✅ | unset (server flag decides wrapper telemetry; kernel default decides kernel telemetry) | Force Go wrapper telemetry on/off on the Thrift path, overriding the server feature flag. On the kernel path, forwarded to kernel-owned telemetry only when explicitly set. | +| `telemetry_batch_size` | ✅ | ✅ | `200` wrapper default; kernel default when `enableTelemetry` is unset | Events per batch. Forwarded to the kernel only when `enableTelemetry` is explicitly set. | +| `telemetry_flush_interval` | ✅ | ✅ | `30s` wrapper default; kernel default when `enableTelemetry` is unset | Flush interval. Forwarded to the kernel only when `enableTelemetry` is explicitly set. | | `telemetry_retry_count` | ⚠️ | ⚠️ | — | **Deprecated and ignored** (retries are owned by the HTTP client + circuit breaker); logs a one-time warning. | | `telemetry_retry_delay` | ⚠️ | ⚠️ | — | **Deprecated and ignored** (see above). | These telemetry knobs are **DSN-only** — there are no `WithX` connector options for them. An app assembled with `NewConnector(...)` options rather than a DSN cannot tune telemetry: wrapper telemetry falls back to the server feature flag (since `enableTelemetry` is -unset), kernel telemetry defaults on, and `telemetry_batch_size` / -`telemetry_flush_interval` use their backend defaults. +unset), and kernel telemetry uses the kernel defaults. On the kernel path, +`telemetry_batch_size` and `telemetry_flush_interval` are applied only with an explicit +`enableTelemetry` value because the kernel C ABI does not accept an unset enabled flag +alongside tuning fields. The Go wrapper telemetry interceptor is skipped on the kernel path so it does not duplicate kernel-owned telemetry for the same connection and statements. See diff --git a/README.md b/README.md index 7458c0e2..243e22ff 100644 --- a/README.md +++ b/README.md @@ -362,8 +362,9 @@ left unset (the default), a **server-side feature flag** decides whether wrapper telemetry is active — so it may be enabled without an explicit opt-in. Setting `enableTelemetry` explicitly overrides the flag. On the kernel backend, the Go wrapper skips its telemetry interceptor entirely so it does not duplicate kernel-owned -telemetry; `enableTelemetry` and `telemetry_batch_size` are forwarded into the kernel -telemetry config instead. +telemetry; when `enableTelemetry` is explicitly supplied, it and any telemetry tuning +knobs are forwarded into the kernel telemetry config. When `enableTelemetry` is unset, +telemetry config is left for the kernel default. ``` # force on (regardless of the server flag): diff --git a/doc.go b/doc.go index ec4a2498..e323306f 100644 --- a/doc.go +++ b/doc.go @@ -282,7 +282,8 @@ Features above the backend seam are inherited unchanged: the database/sql connec pool and connection lifecycle. The Go wrapper telemetry interceptor is skipped on the kernel path so it does not duplicate kernel-owned telemetry for the same connection and statements; `enableTelemetry`, `telemetry_batch_size`, and -`telemetry_flush_interval` are forwarded to kernel-owned telemetry config. Result types render byte-for-byte identical to the +`telemetry_flush_interval` are forwarded to kernel-owned telemetry config only when +`enableTelemetry` is explicitly supplied. Result types render byte-for-byte identical to the Thrift backend: scalars, DECIMAL (exact string), TIMESTAMP / TIMESTAMP_NTZ (shifted into the session time zone), INTERVAL, nested Array/Map/Struct and VARIANT (as JSON), and GEOMETRY / GEOGRAPHY (WKT). The server query id is surfaced on the success path, so a diff --git a/internal/backend/kernel/config.go b/internal/backend/kernel/config.go index c8ea1871..dccd12dd 100644 --- a/internal/backend/kernel/config.go +++ b/internal/backend/kernel/config.go @@ -134,8 +134,9 @@ func requestTimeoutMilliseconds(timeout time.Duration) int64 { // TelemetryConfig is the kernel telemetry subset exposed by the Go driver. It // mirrors the kernel telemetry C ABI: Enabled follows the user-supplied -// enableTelemetry value, defaulting to true when unset. Zero-valued tuning fields -// keep the kernel defaults until applyTelemetry fills them for the setter. +// enableTelemetry value. A nil Config.Telemetry means enableTelemetry was unset, +// so the kernel default is left untouched. Zero-valued tuning fields keep the +// kernel defaults until applyTelemetry fills them for the setter. type TelemetryConfig struct { Enabled bool BatchSize int diff --git a/kernel_config.go b/kernel_config.go index 1e7f0489..4d4f4a59 100644 --- a/kernel_config.go +++ b/kernel_config.go @@ -152,8 +152,8 @@ func buildKernelConfig(cfg *config.Config, kauth kernel.Auth) kernel.Config { // server identically with no per-backend translation. SessionConf: cfg.EffectiveSessionParams(), // Kernel-owned telemetry. The Go wrapper interceptor is skipped on the - // kernel path, but the kernel still needs the user's telemetry knobs and - // this binding's system identity for its own runtime. + // kernel path. Leave Telemetry nil when enableTelemetry is unset so the + // kernel's own default policy remains authoritative. Telemetry: kernelTelemetryConfig(cfg), DriverSystemConfiguration: kernelDriverSystemConfiguration(cfg), } @@ -192,9 +192,9 @@ func buildKernelConfig(cfg *config.Config, kauth kernel.Auth) kernel.Config { } func kernelTelemetryConfig(cfg *config.Config) *kernel.TelemetryConfig { - enabled := true - if val, isSet := cfg.EnableTelemetry.Get(); isSet { - enabled = val + enabled, enableSet := cfg.EnableTelemetry.Get() + if !enableSet { + return nil } return &kernel.TelemetryConfig{ Enabled: enabled, diff --git a/kernel_config_test.go b/kernel_config_test.go index a2b71013..825789b6 100644 --- a/kernel_config_test.go +++ b/kernel_config_test.go @@ -566,21 +566,17 @@ func TestBuildKernelConfig(t *testing.T) { } }) - t.Run("kernel telemetry config defaults enabled and omits unset batch size", func(t *testing.T) { + t.Run("kernel telemetry config is nil when enableTelemetry is unset", func(t *testing.T) { c := baseKernelConfig() + c.TelemetryBatchSize = 17 + c.TelemetryFlushInterval = 9 * time.Second kc := buildKernelConfig(c, kernel.Auth{Mode: kernel.AuthPAT, Token: "dapi-x"}) - if kc.Telemetry == nil { - t.Fatal("Telemetry not forwarded") - } - if !kc.Telemetry.Enabled { - t.Error("Telemetry.Enabled = false, want true when enableTelemetry is unset") - } - if kc.Telemetry.BatchSize != 0 { - t.Errorf("Telemetry.BatchSize = %d, want 0 when telemetry_batch_size is unset", kc.Telemetry.BatchSize) + if kc.Telemetry != nil { + t.Fatalf("Telemetry = %+v, want nil when enableTelemetry is unset", kc.Telemetry) } }) - t.Run("kernel telemetry config follows explicit enableTelemetry and batch size", func(t *testing.T) { + t.Run("kernel telemetry config follows explicit enableTelemetry and tuning", func(t *testing.T) { c := baseKernelConfig() c.EnableTelemetry = config.NewConfigValue(false) c.TelemetryBatchSize = 17