ci: dispatch the SEA-via-kernel integration leg behind kernel labels + kernel CLOSE_STATEMENT telemetry & auth-rejection sentinel by mani-mathur-arch · Pull Request #403 · databricks/databricks-sql-go

mani-mathur-arch · 2026-07-14T16:47:15Z

Context for reviewers

Two changes for the SEA-via-kernel backend (the alternative backend that talks to Databricks SQL through the Rust databricks-sql-kernel static lib over a C ABI, behind the databricks_kernel build tag):

CI: extend the databricks-driver-test dispatch so the Go integration suite can run against the SEA-via-kernel backend, selected by label.
Kernel backend: record CLOSE_STATEMENT telemetry from kernelRows.Close(), and make the unsupported-authenticator rejection programmatically detectable via the ErrNotSupportedByKernel sentinel.

The two are unrelated in subject but small and both review-driven; they share this PR because the telemetry/sentinel commit was folded onto this branch. Reviewable independently — the CI change touches only .github/workflows/, the kernel change only internal/backend/kernel/ + kernel_config*.go.

Stack position (top of the stack):

#399  OAuth / namespace / metric-view / params / types / query-id   (base)
  └─ #400  richer TLS options
       └─ #401  kernel logging → driver log level
            └─ #402  nightly E2E workflow
                 └─ #403  CI dispatch label + CLOSE_STATEMENT telemetry   ← THIS PR

1. CI: kernel integration-test dispatch

Extend the databricks-driver-test dispatch so the Go integration suite can run against the SEA-via-kernel backend, selected by label — the analogue of the existing Thrift labels, not a copy.

Label	Backend	Proxy mode
`integration-test`	Thrift	replay
`integration-test-full`	Thrift	passthrough
`integration-test-kernel`	sea (kernel)	passthrough

The label resolves both proxy_mode and a new go_mode (thrift vs sea).
go_mode rides in the repository_dispatch client_payload. The receiver in driver-test reads it to decide whether to build the kernel static lib and run the databricks_kernel-tagged leg. Thrift labels send go_mode: thrift and are unchanged.
Only one kernel label, and it's passthrough: there are no committed Go-sea proxy recordings yet, so a sea+replay label would have nothing to replay and produce a red, misleading run. A sea replay label can be added later once recordings are captured.
The new label is added to the on-new-commit label-drop list and the skip-stub guidance.
The required merge-queue gate stays Thrift-only (go_mode pinned to thrift): the kernel leg is previewable on demand but not a required gate until the SEA backend ships in a released driver.

Two-repo change

This is the driver-side half. The receiver-side half (read go_mode, sea-gated kernel build prerequisites, thread DATABRICKS_GO_MODE, guard sea+replay) is a separate PR in databricks-driver-test. The runner + harness already handle DATABRICKS_GO_MODE=sea end-to-end. On the receiver side the review gate caught (and fixed before merge) two real defects worth knowing about: a backend-aware RECORDING_NAME (databricks-go-${go_mode}) so a sea run can't clobber/serve the committed thrift recordings, and a narrowed git-token insteadOf scope (exact databricks-sql-kernel repo, not the whole databricks/ org) so the kernel App-token isn't applied to unrelated org fetches.

2. Kernel backend: CLOSE_STATEMENT telemetry + auth-rejection sentinel

Two review-driven, Go-only fixes on the kernel backend.

CLOSE_STATEMENT telemetry

conn records CLOSE_STATEMENT / close latency / statement success-or-failure only when the rows' OnClose callback fires (the Thrift path fires it). kernelRows.Close() never called it, so kernel queries emitted no close telemetry — including failures — a production observability blind spot.

Close() now times the teardown and fires OnClose(latencyMs, chunkCount, iterationErr, nil). closeErr is nil because the kernel teardown (C stream/statement close) has no fallible close RPC to report, unlike Thrift's CloseOperation.
Next() is split into a thin wrapper over next() that records the first non-EOF error as iterationErr (io.EOF is normal drain, not a failure), which Close() passes to the callback — mirroring the Thrift path's iterationErr.
The callback is armed only after newKernelRows finishes constructing successfully, so the cleanup Close() on a schema-fetch/import failure does not record a falsely-successful CLOSE_STATEMENT (matching the Thrift NewRows, which sets the close callback just before returning).
Idempotent: the existing closed guard means a double-Close fires OnClose exactly once.

Auth-rejection sentinel

resolveKernelAuth's unsupported-authenticator branch (token-provider / external / federated) returned a bare errors.New, so fallback logic ("kernel can't honor this auth → use Thrift") could only fragile substring-match. It now wraps ErrNotSupportedByKernel via %w so callers can errors.Is it, matching the other kernel-config rejections.

Deliberately not wrapped: the "requires a personal access token" (empty-token) error — that is missing-required-config, not an unsupported feature; wrapping it would wrongly signal "fall back to Thrift" for a forgotten token.

Testing

Workflow YAML validated; label resolution dry-run: integration-test→thrift/replay, integration-test-full→thrift/passthrough, integration-test-kernel→sea/passthrough.
TestKernelRowsCloseFiresOnClose (tagged): success path (nil errs), iterationErr propagation, idempotent double-close, nil-callbacks-safe, construction-failure-no-success-close.
TestValidateKernelConfig: strengthened the non-PAT authenticator rejection to assert errors.Is(err, ErrNotSupportedByKernel).
Default CGO_ENABLED=0 + tagged databricks_kernel: build + vet + test — 24 packages ok, 0 fail each.

Stacking

Stacked on #402 (mani/sea-kernel-nightly-e2e).

This pull request and its description were written by Isaac.

github-actions · 2026-07-15T05:54:54Z

Go integration tests triggered (sea / passthrough). View workflow runs.

github-actions · 2026-07-15T07:48:51Z

Go integration tests triggered (sea / passthrough). View workflow runs.

github-actions · 2026-07-15T08:52:35Z

Integration test approval reset.

New commits were pushed to this PR. Label(s) integration-test-kernel were removed for security.

A maintainer must re-review and re-add a label to preview tests again. (The real gate runs in the merge queue.)

Latest commit: 2b09947

…rnel Follow-up to the review round: the unsupported-authenticator default case in resolveKernelAuth still returned a plain errors.New, while every other unsupported kernel option wraps ErrNotSupportedByKernel and doc.go advertises that errors.Is(err, ErrNotSupportedByKernel) detects any unsupported kernel feature. So this PR shipped a documented contract its own code broke for token-provider / external / federated auth. Wrap it with %w to honor the contract (same fix #403 makes one commit up the stack — matching its wording so the two converge cleanly on rebase). The empty-PAT case stays unwrapped: a missing token is misconfiguration to fix, not a feature the kernel can't honor. Tighten the "non-PAT/non-OAuth authenticator rejected" test to assert errors.Is(err, ErrNotSupportedByKernel) instead of only err != nil, so the contract is pinned rather than documented as an exception. Verified: default-build suite passes, go vet + gofmt clean. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

Extend the driver-test dispatch so the Go integration suite can run against the SEA-via-kernel backend, selected by label — the analogue of the existing Thrift labels, not a copy. Adds one kernel-namespaced label: - integration-test-kernel → sea backend, passthrough (real warehouse) The label resolves both proxy_mode and a new go_mode (thrift vs sea), and go_mode rides in the repository_dispatch client_payload. databricks-driver-test reads it to decide whether to build the kernel static lib and run the tagged (databricks_kernel) leg; the Thrift labels send go_mode=thrift and are unchanged. The new label is added to the on-new-commit label-drop list and the skip-stub guidance. The kernel label is passthrough (not replay): the sea leg has no committed recordings to replay yet, so there is no sea replay label until those are captured. The required merge-queue gate stays Thrift-only (go_mode pinned to thrift): the kernel leg is previewable on demand but not a required gate until the SEA backend ships in a released driver. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

…h rejection Two review-driven fixes on the kernel backend, both Go-only. CLOSE_STATEMENT telemetry: kernelRows.Close() now fires the OnClose callback, so kernel queries record close latency / statement success-or-failure the same way the Thrift path does (conn gates that recording on OnClose firing). Previously it never fired, so kernel traffic emitted no close telemetry — including failures — a production observability blind spot. Next() is split into a thin wrapper that records the first non-EOF error as iterationErr (io.EOF is normal drain), which Close() passes to the callback; closeErr is nil since the kernel teardown has no fallible close RPC. The callback is armed only after newKernelRows finishes constructing (mirroring the Thrift NewRows), so a schema-fetch/import failure's cleanup Close() does not record a falsely-successful CLOSE_STATEMENT. Auth rejection sentinel: the unsupported-authenticator branch of resolveKernelAuth wrapped a bare errors.New, so fallback logic could only substring-match. It now wraps ErrNotSupportedByKernel via %w so callers can errors.Is it, matching the other kernel config rejections. The missing-personal-access-token error stays a plain error (missing-required-config, not an unsupported feature — it must not signal "fall back to Thrift"). Tests: TestKernelRowsCloseFiresOnClose (success, iterationErr propagation, idempotent double-close, nil-callbacks, construction-failure-no-success-close); strengthened the non-PAT authenticator rejection to assert errors.Is(ErrNotSupportedByKernel). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

…x Thrift comment Addresses two review comments on the CLOSE_STATEMENT telemetry change: - The "construction-failure Close must not fire a success OnClose" subtest was a no-op: `fired` was never assignable to true, so it duplicated the nil-callbacks case and asserted nothing. Replaced with a test that drives the real newKernelRows cleanup path — a nil result stream makes kernel_result_stream_get_schema return a defined InvalidArgument error (the kernel null-checks the handle, never UB), so newKernelRows takes its r.Close() cleanup branch and returns an error, and the supplied OnClose must not fire. Verified by mutation: arming the callback before the schema import makes the test fail as intended. - Reworded the newKernelRows comment that claimed the deferred callback assignment matches Thrift's NewRows "just before returning" — Thrift actually assigns closeCallback during construction. Dropped the inaccurate comparison and state the invariant directly. Co-authored-by: Isaac

…rnel Follow-up to the review round: the unsupported-authenticator default case in resolveKernelAuth still returned a plain errors.New, while every other unsupported kernel option wraps ErrNotSupportedByKernel and doc.go advertises that errors.Is(err, ErrNotSupportedByKernel) detects any unsupported kernel feature. So this PR shipped a documented contract its own code broke for token-provider / external / federated auth. Wrap it with %w to honor the contract (same fix #403 makes one commit up the stack — matching its wording so the two converge cleanly on rebase). The empty-PAT case stays unwrapped: a missing token is misconfiguration to fix, not a feature the kernel can't honor. Tighten the "non-PAT/non-OAuth authenticator rejected" test to assert errors.Is(err, ErrNotSupportedByKernel) instead of only err != nil, so the contract is pinned rather than documented as an exception. Verified: default-build suite passes, go vet + gofmt clean. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

…tly workflows for e2e tests (#399) ## What Extends the SEA-via-kernel backend for the Go driver with the full DAIS gap-closure + PuPr feature set: authentication, session setup, type rendering, query execution, telemetry, richer TLS, kernel-log routing, and CI. This PR consolidates what was a 5-PR stack (#400 richer-TLS, #401 log-unify, #402 nightly-e2e, #403 c2-dispatch) into a single reviewable PR. The stack was linear, so the branches were fast-forwarded into this one — every change is preserved as a **distinct, logically-scoped commit** (23 commits), so it reads commit-by-commit rather than as one blob. #400–#403 now show as merged (their commits live here). ## Features **Authentication** - **OAuth M2M / U2M** — the kernel drives its own auth flow from `cfg.Authenticator` (mirroring pyo3/napi). Adds a `kernel.Auth` descriptor + `resolveKernelAuth`, wired through the `set_auth_pat` / `_m2m` / `_u2m` C-ABI setters. `cfg.Authenticator` is the single source of truth (last-writer-wins, matching Thrift). - **Unsupported auth is sentinel-wrapped** — token-provider / external / federated authenticators reject with `%w`-wrapped `ErrNotSupportedByKernel`, the same programmatic-fallback contract every other unsupported kernel option follows. **Session setup** - **Initial namespace** — post-connect `USE CATALOG` / `USE SCHEMA` with identifier quoting (the kernel C ABI has no namespace setter). - **Metric-view metadata** — `config.EffectiveSessionParams()` folds the server conf in backend-neutrally, so the kernel path sends the identical conf Thrift does. **Type rendering** - **INTERVAL** day-time & year-month rendering in `internal/arrowscan` (kernel returns native arrow values; formatted Go-side to the Thrift path's string form). Handles the `math.MinInt64` / `math.MinInt32` negation bound without overflow. - Parity coverage for **TIMESTAMP vs TIMESTAMP_NTZ** and **VARIANT / GEOMETRY** (live-probed on both backends). **Query execution & telemetry** - **Bound query parameters** via the kernel raw-param C ABI (`kernel_statement_bind_parameter`). The positional/named + SQL-NULL/empty-string decision is a pure `paramBindArg` seam, unit-tested under `CGO_ENABLED=0`. - **Kernel errors surfaced as `DBExecutionError`** carrying sqlstate + server query id, so `errors.As` → `SqlState()` / `QueryId()` works on the kernel path. The execute path deliberately reports **non-retryable** (a sent statement may have committed — no double-write), matching Thrift. - **Server query id** via a real `StatementID()` accessor (`kernel_executed_statement_query_id`), threaded into EXECUTE_STATEMENT telemetry, plus CLOSE_STATEMENT telemetry on the result-read path. - **Cancelled execute** evicts a session-fatal conn and preserves the kernel error metadata (multi-`%w`: `errors.Is(DeadlineExceeded)` AND `errors.As(*KernelError)` both hold). **Richer TLS (Go options)** - CA-bundle + independent hostname-skip setters mapped to the kernel's own TLS knobs (defensive PEM copy; the wholesale custom `WithTransport` stays rejected). **Kernel logging** - Kernel log output routed through the driver log level with connId/corrId/queryId correlation; allocation-free above Debug (guards the Arrow-batch hot path). - **scope note (please read before flagging):** - This unifies the *control knob*, not the *sink*, and that boundary is intentional. `DATABRICKS_LOG_LEVEL`/`SetLogLevel` now drives both the Go binding lines and the kernel's Rust lines from one level. But they stay two physical streams: Go lines go through `logger.Logger` (and honor `logger.SetLogOutput`), while the kernel's Rust lines go straight to stderr — the kernel C ABI exposes **no sink/callback hook**, so there is currently no way to route them through the Go logger. Consequences a reviewer will notice, all expected: - A custom `SetLogOutput`/file captures only the Go lines; Rust lines stay on stderr. - With the default sink both land on stderr but only *race* there — not a single ordered, merged stream. - The Rust subscriber's level is sampled once, at the first kernel connect. Fully merging both into one ordered sink requires a kernel log callback and is tracked separately (<TICKET>); it's out of scope here because it's a kernel-side ABI change, not a driver one. **CI** - A scheduled + dispatchable **nightly E2E** workflow that runs the credential-gated e2e + Thrift-vs-kernel parity suites against a real warehouse (the ordinary CI injects no warehouse secrets, so those tests otherwise skip). SEA-via-kernel integration leg gated behind a kernel label. ## Notable behavior - Bound query parameters are now supported (previously rejected at connect). - **Staging operations remain rejected** — the kernel path can't perform the local file transfer and the C ABI surfaces no staging signal. ## Testing - **Default pure-Go** (`CGO_ENABLED=0`): `go build` / `go vet` / `go test ./...` — 24 packages ok. - **Tagged kernel path** (`CGO_ENABLED=1 -tags databricks_kernel`, linked against a locally-built kernel `.a` at `KERNEL_REV`): 24 packages ok. - **Live e2e + parity** against a real warehouse: all pass — data types (incl. variant/geometry/decimal), NTZ, params-vs-Thrift (10/10), CloudFetch, cancellation, connection pool (40/40), StatementID, initial namespace, and **M2M** (OAuth client-credentials). - `gofmt` clean; Isaac Review clean (0 final comments). ## Merge gate - **Re-pin `KERNEL_REV` at merge time.** It currently points at the kernel dependency's PR-head SHA (GC-able once that kernel change merges); bump it to the resulting kernel `main` SHA, re-sync the cgo drift assertions in `cgo.go` if any signatures changed, and run `make test-kernel` against the new rev. This pull request and its description were written by Isaac. --------- Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

mani-mathur-arch added the ai-assisted label Jul 14, 2026

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from 5f940a0 to 90122c9 Compare July 14, 2026 17:01

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 2a43d24 to ebedfc7 Compare July 14, 2026 18:43

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from 90122c9 to 2a9715b Compare July 14, 2026 18:43

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from ebedfc7 to 23de162 Compare July 14, 2026 18:49

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from 2a9715b to 21a18d7 Compare July 14, 2026 18:49

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 23de162 to fb098a1 Compare July 14, 2026 18:53

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from 21a18d7 to c9ff45d Compare July 14, 2026 18:53

mani-mathur-arch changed the title ~~ci: dispatch the SEA-via-kernel integration leg behind kernel labels~~ ci: dispatch the SEA-via-kernel integration leg behind kernel labels + kernel CLOSE_STATEMENT telemetry & auth-rejection sentinel Jul 15, 2026

mani-mathur-arch commented Jul 15, 2026

View reviewed changes

Comment thread internal/backend/kernel/kernel_test.go Outdated

Comment thread internal/backend/kernel/rows.go Outdated

mani-mathur-arch marked this pull request as ready for review July 15, 2026 05:18

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from d49b653 to 1d357ba Compare July 15, 2026 05:37

mani-mathur-arch added the integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) label Jul 15, 2026

mani-mathur-arch added integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) and removed integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) labels Jul 15, 2026

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 6376422 to 4512646 Compare July 15, 2026 08:46

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from 1d357ba to 2b09947 Compare July 15, 2026 08:51

github-actions Bot removed the integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) label Jul 15, 2026

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 4512646 to 091accc Compare July 15, 2026 09:20

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from 2b09947 to a3d9c92 Compare July 15, 2026 09:22

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 091accc to ac112d4 Compare July 15, 2026 09:49

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from a3d9c92 to cf06659 Compare July 15, 2026 09:49

mani-mathur-arch added 3 commits July 15, 2026 11:45

mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from ac112d4 to 0c68dfb Compare July 15, 2026 12:40

mani-mathur-arch force-pushed the mani/sea-kernel-c2-dispatch branch from cf06659 to 94ced6a Compare July 15, 2026 12:41

Base automatically changed from mani/sea-kernel-nightly-e2e to mani/sea-kernel-consolidated July 15, 2026 13:48

mani-mathur-arch merged commit 94ced6a into mani/sea-kernel-consolidated Jul 15, 2026
5 checks passed

mani-mathur-arch deleted the mani/sea-kernel-c2-dispatch branch July 15, 2026 13:48

mani-mathur-arch mentioned this pull request Jul 15, 2026

feat(kernel): consolidated DAIS gap-closure + PuPr feature set + nightly workflows for e2e tests #399

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: dispatch the SEA-via-kernel integration leg behind kernel labels + kernel CLOSE_STATEMENT telemetry & auth-rejection sentinel#403

ci: dispatch the SEA-via-kernel integration leg behind kernel labels + kernel CLOSE_STATEMENT telemetry & auth-rejection sentinel#403
mani-mathur-arch merged 3 commits into
mani/sea-kernel-consolidatedfrom
mani/sea-kernel-c2-dispatch

mani-mathur-arch commented Jul 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mani-mathur-arch commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context for reviewers

1. CI: kernel integration-test dispatch

Two-repo change

2. Kernel backend: CLOSE_STATEMENT telemetry + auth-rejection sentinel

CLOSE_STATEMENT telemetry

Auth-rejection sentinel

Testing

Stacking

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mani-mathur-arch commented Jul 14, 2026 •

edited

Loading