feat(kernel): classify telemetry errors by kernel status code#420
Open
mani-mathur-arch wants to merge 3 commits into
Open
feat(kernel): classify telemetry errors by kernel status code#420mani-mathur-arch wants to merge 3 commits into
mani-mathur-arch wants to merge 3 commits into
Conversation
Map the kernel's structured status code to a telemetry ErrorCategory via (*KernelError).Category(), read by classifyError through CategoryFromError, so a kernel failure reports its authoritative code-derived category instead of one guessed from the message text. Mirrors the Python driver's _CODE_TO_EXCEPTION and Node's mapKernelErrorToJsError. Unmapped codes fall back to message matching. cgo.go pins the new status constants to the C enum. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Map Internal and InvalidStatementHandle so every kernel status code has a telemetry category (generic and statement_closed respectively, matching how Python/Node bucket them). A *KernelError now never falls through to the message-substring classifier. cgo.go pins the two new constants to the C enum. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Add TestKernelErrorCategoryExhaustive: every kernel status code must map to a non-empty category and the map size must match the code list, so a future code added without a mapping fails CI instead of silently falling back to message matching (mutation-verified). Correct the codeToCategory doc — only Internal goes to the generic bucket; InvalidStatementHandle maps to statement_closed. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
added
the
integration-test-kernel
|
Go integration tests triggered ( |
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
2 participants
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
The kernel backend returns a structured
KernelErrorcarrying an authoritative statusCode, but telemetry'sclassifyErrorwas re-deriving the error category by substring-matching the error message. This adds(*KernelError).Category(), which maps the kernel status code to a telemetryErrorCategory;classifyErrorreads it viaCategoryFromError(added in #415) and only falls back to message matching for non-kernel errors.cgo.gopins the new status constants to the C enum at compile time.This mirrors how the other kernel-backed drivers derive the error identity from the kernel code — Python's
_CODE_TO_EXCEPTIONand Node'smapKernelErrorToJsError. Every kernel status code is mapped (includingInternal→ generic andInvalidStatementHandle→statement_closed, the same generic buckets Python/Node use), so a*KernelErrornever depends on the message-substring fallback.Telemetry-only: the error a caller's
Exec/Queryreturns (full message, sqlstate, queryId viaExecutionError) is unchanged.Testing
TestKernelErrorCategory(all 13 kernel codes + a value outside the enum → "") andTestKernelErrorCategoryThroughWrap(category survives%wwrapping) — pure Go, run underCGO_ENABLED=0.kernel_error_telemetry_e2e_test.go(taggedcgo && databricks_kernel): captures the outbound/telemetry-extpayload on a live warehouse and asserts a realSqlErrorlands asexecute_statement_failed.SqlError → execute_statement_failedandCancelled → cancelledobserved both on the wire and in the backingfrontend_log_sql_driver_logtable.Follow-up (not in this PR)
This PR makes the mapping total over kernel status codes, so any
*KernelErroris code-classified. Errors that originate in the Go binding itself and carry no kernel code — Arrow schema/batch import failures and result-scan failures — can still reach the message fallback. Attaching a source-declared category at those sites is deferred, because most of them flow through the sharedinternal/rows/arrowbasedlayer that #417 / #419 (PECOBLR-3537, Thrift side) are already categorizing; doing it here would overlap those PRs and their newarrowbased/errors.go. Once they land, the residual is a small kernel-binding-only set (the cgo-boundary import failures + a couple ofoperation.goguards), which can be tagged in a focused follow-up with no conflict.This pull request and its description were written by Isaac.