Forward telemetry parameters to the kernel binding

[jay-xiao446]

Summary

• Forward kernel-owned telemetry configuration into databricks_sql_kernel.Session: driver/runtime identity, telemetry_enabled, and telemetry_batch_size.
• Set kernel telemetry_enabled from enable_telemetry; keep force_enable_telemetry as a Python wrapper-only feature-flag bypass concept.
• Disable Python-side connector telemetry for use_kernel=True connections so wrapper telemetry does not duplicate kernel-owned telemetry.
• Add coverage that Python wrapper telemetry is disabled on the kernel path while the kernel-bound telemetry parameter follows enable_telemetry.

Tests

• .venv/bin/python -m pytest tests/unit/test_telemetry.py -q
• .venv/bin/python -m pytest tests/unit/test_session.py -q
• .venv/bin/python -m pytest tests/unit/test_kernel_client.py -q

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

Verdict: 1 Low

Looks good — the core gate (is_telemetry_enabled returns False for kernel connections, via a safe getattr) is correct and well-tested, and session.use_kernel is guaranteed set before it's read. One low-severity gap: the connection-failure telemetry path (connection_failure_log) is not gated on use_kernel, so a failed kernel connect still emits a Python-side failure log.

Other findings

• 🔵 Low — This PR disables Python-side telemetry for kernel connections by short-circuiting is_telemetry_enabled (telemetry_client.py:125). However, the connection-failure telemetry path is a separate code path that bypasses is_telemetry_enabled entirely: when session.open() raises, connection_failure_log is invoked and gated only on enable_telemetry (defaulting to True), never on use_kernel. So a kernel connection that fails to open will still emit a Python-side failure log — inconsistent with the PR's stated goal of disabling Python telemetry for kernel connections (and potentially duplicating what the kernel reports). If failure-before-kernel-open telemetry is intentionally still desired on the Python side, this is fine as-is; otherwise consider passing use_kernel through and skipping connection_failure_log on that path.

[peco-review-bot]

Verdict: 2 Low

Looks good overall — the change cleanly disables Python-side telemetry on kernel connections and threads phase-7 identity/telemetry kwargs into the kernel Session, with solid regression tests (including the force-enabled case). Two low-severity notes: an identity-vs-truthiness mismatch between the telemetry bypass and backend routing, and a potential wheel-version compat gap since the new kwargs are passed unconditionally against a ^0.2.0 pin.

[peco-review-bot]

🔵 Low — The kernel bypass uses an identity check (is True), but backend routing in _create_backend uses truthiness (if self.use_kernel: where self.use_kernel = kwargs.get("use_kernel", False)). These disagree for any truthy-but-non-True value: e.g. use_kernel=1 or use_kernel="true" would still route the connection through KernelDatabricksClient (truthy), yet 1 is True / "true" is True evaluate to False, so Python-side telemetry would NOT be disabled — defeating the intent of this PR for those inputs.

Recommend matching the routing semantics with a plain truthiness check so the two code paths can't diverge:

if getattr(connection.session, "use_kernel", False):
    return False

Minor, since use_kernel is documented/expected to be a bool, but the mismatch is a latent inconsistency.

[peco-review-bot]

🔵 Low — _kernel_telemetry_kwargs builds the phase-7 identity/telemetry kwargs (driver_name, telemetry_enabled, process_name, etc.) and they are spread unconditionally into _kernel.Session(**telemetry_kwargs) at open_session. The kernel wheel constraint is still ^0.2.0 (>=0.2.0,<0.3.0). If these kwargs were introduced in a later 0.2.x than 0.2.0, a user with an older-but-constraint-satisfying wheel installed would hit a TypeError: Session() got an unexpected keyword argument ... at connect time. If phase-7 requires a minimum kernel version, consider bumping the lower bound of the databricks-sql-kernel pin so the wheel and connector stay in lockstep. (Flagged Low — I can't verify the kernel Session signature from this repo.)

[peco-review-bot]

Verdict: 2 Low

Looks good overall — the kernel-path telemetry forwarding and the is_telemetry_enabled short-circuit are correct, defaults line up between connect()/session.py/_kernel_telemetry_kwargs, and the new tests are meaningful. Two low-severity notes: (1) the connection-failure telemetry path isn't gated on use_kernel, so the Python wrapper still emits failure telemetry on the kernel path; (2) the phase-7 Session kwargs are forwarded unconditionally while the kernel dependency floor stays ^0.2.0, a potential connect-time break if an older wheel is installed.

Other findings

• 🔵 Low — The kernel-path guard in is_telemetry_enabled disables Python-side telemetry only for the successful connection path. When a use_kernel=True connection fails during session.open(), this connection_failure_log(...) call still fires with enable_telemetry=kwargs.get("enable_telemetry", True) — it is not gated on use_kernel. So the Python wrapper still emits a connection-failure telemetry event on the kernel path, which is the kind of wrapper-owned telemetry this PR set out to suppress for use_kernel. It may be intentional (the kernel session never opened, so it couldn't emit its own failure telemetry), but if the goal is "no Python-side connector telemetry on the kernel path," this branch is an unguarded exception worth confirming.

[peco-review-bot]

🔵 Low — Unlike retry_kwargs/http_headers_kwargs (which are conditionally omitted when empty), _kernel_telemetry_kwargs always returns the phase-7 identity fields (driver_name, runtime_*, os_*, process_name, telemetry_enabled), so these kwargs are passed to _kernel.Session(...) on every use_kernel open. If the installed kernel wheel predates phase-7 support for these Session kwargs, construction raises TypeError and every use_kernel=True connection breaks. The dependency floor is still ^0.2.0 and isn't bumped in this PR. If phase-7 requires a newer kernel wheel, consider raising the minimum version so incompatible wheels fail at install time rather than at connect time. (Low because I can't verify the 0.2.0 Session signature from this repo — the kernel is a compiled extension.)