feat(kernel): forward kernel logs into the driver logger via callback#411
Closed
mani-mathur-arch wants to merge 5 commits into
Closed
feat(kernel): forward kernel logs into the driver logger via callback#411mani-mathur-arch wants to merge 5 commits into
mani-mathur-arch wants to merge 5 commits into
Conversation
mani-mathur-arch
marked this pull request as ready for review
July 18, 2026 11:31
WithKernelClientCertificate marks the kernel experimental config as set, but an empty cert+key pair (e.g. from a failed PEM load) was indistinguishable from the option never being called: the old XOR validation accepted it and applyKernelTLS skipped the setter, so a caller who explicitly requested mTLS connected with no client identity. Add an explicit TLSClientCertConfigured marker (robust against nil-vs-empty), set it whenever the option is invoked, and tighten validateKernelConfig to reject any incomplete mTLS request (missing cert, missing key, or both empty). Covered by new option/validation/DeepCopy tests and the exhaustiveness guard.
The ABI-version handshake was only exercised on the happy path (TestABIVersionMatches, cgo build). The mismatch branch — the runtime hazard the check exists for, a driver header linked against a differently-built prebuilt .a — had no coverage because it lived behind the cgo symbols and the one-shot abiCheckOnce cache. Extract the pure got-vs-want verdict into compareABI (untagged) and have checkABIVersion delegate to it, then add TestCompareABI asserting the mismatch returns a non-nil error naming both versions and the remediation. Being untagged, the negative test runs under the default CGO_ENABLED=0 build with no kernel lib linked. Production behavior is unchanged.
Register a cgo-exported trampoline (kernelLogTrampoline) as the kernel's log sink (kernel_set_log_callback) so kernel-internal tracing events flow into the driver's logger — the unified Go+kernel stream, honouring logger.SetLogOutput — instead of only reaching stderr via kernel_init_logging. This completes the one-knob logging story: #399 unified the LEVEL across the Go binding lines and the kernel's Rust lines (PECOBLR-3650) against the stderr sink; this replaces that sink with the callback so the Rust lines land in the same writer as everything else. The reverse-call machinery: a runtime/cgo.Handle round-trips the *logSink through the C void* ctx (cgo pointer rules), and a defer recover() firewall converts any panic into a dropped line (a panic across the cgo boundary would abort the process) — the same shape a kernel->host token-provider callback would need. The driver maps its own log level (resolveKernelLogArg, unchanged) and passes it as the callback's level argument, so the kernel's Rust lines follow DATABRICKS_LOG_LEVEL, not just RUST_LOG; DBSQL_KERNEL_DEBUG still defers to RUST_LOG (NULL level). The forwarding sink logs through a SNAPSHOT of the driver logger taken at install and pinned to TraceLevel, for two reasons: (1) no double gating — the kernel already filtered events against the level we passed it, so re-gating through the live logger.Logger (at the driver level) would re-drop exactly the events the DBSQL_KERNEL_DEBUG override was meant to surface; (2) no data race — the kernel drain thread reads only its immutable snapshot rather than the mutable global logger a concurrent SetLogLevel/SetLogOutput would rewrite. A non-Success install is surfaced at Warn (visible at the default level), not the Debug-gated klog. The two C-ABI logging routes share the one process-global subscriber and are mutually exclusive; we install the callback, and a non-Success install (host already installed a subscriber) is logged, never fatal to connect. Level mapping and sink routing are pure Go (logforward.go, untagged) so they test under CGO_ENABLED=0 — including the per-level mapping assertions and the not-re-gated-by-driver-level property. The reverse-call round-trip, the recover firewall (a panicking sink must not crash the process), the wrong-type/nil-ctx guards, and delivery after a panic are exercised by tagged tests via a C invoke seam. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Remove ticket codes, review shorthand, and ops jargon so the PR reads cleanly for OSS reviewers.
Rely on resolveKernelLogArg returning an empty level for the RUST_LOG override instead of normalizing it twice.
mani-mathur-arch
force-pushed
the
mani/sea-kernel-tier2-features
branch
from
July 18, 2026 11:45
cf93eb3 to
990f9bf
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-log-callback
branch
from
July 18, 2026 11:46
bd3c0a8 to
8c0a8e2
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-tier2-features
branch
from
July 18, 2026 12:17
990f9bf to
7e11982
Compare
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wire the driver side of the kernel reverse-call log callback (behind the
databricks_kernelbuild tag), completing the one-knob logging story:kernelLogTrampoline) as the kernel's log sink viakernel_set_log_callback, so kernel-internaltracingevents flow into the driver's logger — the unified Go+kernel stream, honouringlogger.SetLogOutput— instead of only reaching stderr.runtime/cgo.Handleround-trips the*logSinkthrough the Cvoid* ctx(cgo pointer rules), and adefer recover()firewall converts any panic into a dropped line (a panic across the cgo boundary would abort the process). The handle is a deliberate process-lifetime pin (no detach in the v0 ABI).initKernelLoggingnow installs the callback instead of callingkernel_init_logging. It passes the driver's mapped log level (resolveKernelLogArg, unchanged) as the callback'slevel, so the kernel's Rust lines followDATABRICKS_LOG_LEVEL— the same knob PECOBLR-3650 established, now against the callback sink rather than stderr.DBSQL_KERNEL_DEBUGstill defers toRUST_LOG(NULL level).Successinstall (host already installed a subscriber) is logged, never fatal to connect.Closes PECOBLR-3654 (driver side).
Tests
CGO_ENABLED=0):logSink.forwardlevel mapping + nil-sink no-op (logforward_test.go), pure Go so it runs without the kernel linked.TestLogCallbackRoundTripdrives the C→Go trampoline via a C invoke seam and asserts delivery (handle unwrap + recover firewall + sink dispatch);TestLogCallbackTrampolineNilCtxSafeproves a bad handle can't crash the process. Live round-trip verified — a forwarded WARN line lands in the driver's zerolog sink.Co-authored-by: Isaac