From b5789ef11d78891682ebcbcc51bbc40d94d5d4f8 Mon Sep 17 00:00:00 2001 From: Mani Kaustubh Mathur Date: Wed, 22 Jul 2026 17:20:38 +0000 Subject: [PATCH 1/3] feat(kernel): classify telemetry errors by kernel status code 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 --- internal/backend/kernel/cgo.go | 5 + internal/backend/kernel/errors_classify.go | 47 ++++++- .../backend/kernel/errors_classify_test.go | 40 ++++++ kernel_error_telemetry_e2e_test.go | 119 ++++++++++++++++++ 4 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 kernel_error_telemetry_e2e_test.go diff --git a/internal/backend/kernel/cgo.go b/internal/backend/kernel/cgo.go index cd604121..bd5f6c2f 100644 --- a/internal/backend/kernel/cgo.go +++ b/internal/backend/kernel/cgo.go @@ -235,8 +235,13 @@ func lastError(code C.KernelStatusCode) *KernelError { const ( _ = uint(statusInvalidArgument-int(C.KernelStatusCode_InvalidArgument)) | uint(int(C.KernelStatusCode_InvalidArgument)-statusInvalidArgument) _ = uint(statusUnauthenticated-int(C.KernelStatusCode_Unauthenticated)) | uint(int(C.KernelStatusCode_Unauthenticated)-statusUnauthenticated) + _ = uint(statusPermissionDenied-int(C.KernelStatusCode_PermissionDenied)) | uint(int(C.KernelStatusCode_PermissionDenied)-statusPermissionDenied) + _ = uint(statusNotFound-int(C.KernelStatusCode_NotFound)) | uint(int(C.KernelStatusCode_NotFound)-statusNotFound) + _ = uint(statusResourceExhausted-int(C.KernelStatusCode_ResourceExhausted)) | uint(int(C.KernelStatusCode_ResourceExhausted)-statusResourceExhausted) _ = uint(statusUnavailable-int(C.KernelStatusCode_Unavailable)) | uint(int(C.KernelStatusCode_Unavailable)-statusUnavailable) _ = uint(statusTimeout-int(C.KernelStatusCode_Timeout)) | uint(int(C.KernelStatusCode_Timeout)-statusTimeout) + _ = uint(statusCancelled-int(C.KernelStatusCode_Cancelled)) | uint(int(C.KernelStatusCode_Cancelled)-statusCancelled) + _ = uint(statusDataLoss-int(C.KernelStatusCode_DataLoss)) | uint(int(C.KernelStatusCode_DataLoss)-statusDataLoss) _ = uint(statusNetworkError-int(C.KernelStatusCode_NetworkError)) | uint(int(C.KernelStatusCode_NetworkError)-statusNetworkError) _ = uint(statusSqlError-int(C.KernelStatusCode_SqlError)) | uint(int(C.KernelStatusCode_SqlError)-statusSqlError) ) diff --git a/internal/backend/kernel/errors_classify.go b/internal/backend/kernel/errors_classify.go index c28a859d..70cbd477 100644 --- a/internal/backend/kernel/errors_classify.go +++ b/internal/backend/kernel/errors_classify.go @@ -23,12 +23,17 @@ import ( // without the C import. cgo.go asserts each equals its C.KernelStatusCode_* value // at compile time, so drift from the header is a build error, not a latent bug. const ( - statusInvalidArgument = 1 - statusUnauthenticated = 2 - statusUnavailable = 6 - statusTimeout = 7 - statusNetworkError = 12 - statusSqlError = 13 + statusInvalidArgument = 1 + statusUnauthenticated = 2 + statusPermissionDenied = 3 + statusNotFound = 4 + statusResourceExhausted = 5 + statusUnavailable = 6 + statusTimeout = 7 + statusCancelled = 8 + statusDataLoss = 9 + statusNetworkError = 12 + statusSqlError = 13 ) // KernelError is the Go-side structured error mapped from the kernel's KernelError @@ -58,6 +63,36 @@ func (e *KernelError) Error() string { return fmt.Sprintf("kernel: %s (code=%d%s)", e.Message, e.Code, q) } +// codeToCategory maps the kernel status code to a telemetry ErrorCategory. This is +// the Go analog of the Python driver's _CODE_TO_EXCEPTION and Node's +// mapKernelErrorToJsError: the kernel already hands us an authoritative code, so +// telemetry keys off it instead of re-guessing from the message text. A code with +// no precise category is omitted so classifyError falls back to message matching. +var codeToCategory = map[int]dbsqlerrint.ErrorCategory{ + statusInvalidArgument: dbsqlerrint.CategoryInvalidRequest, + statusUnauthenticated: dbsqlerrint.CategoryAuthError, + statusPermissionDenied: dbsqlerrint.CategoryPermissionError, + statusNotFound: dbsqlerrint.CategoryNotFound, + statusResourceExhausted: dbsqlerrint.CategoryRateLimitExceeded, + statusUnavailable: dbsqlerrint.CategoryConnectionError, + statusTimeout: dbsqlerrint.CategoryTimeout, + // Cancelled is generic (caller cancel, dropped future, or server-side cancel) + // and classifyError also runs for session-lifecycle ops, so use the neutral + // cancelled category rather than the statement-specific one. + statusCancelled: dbsqlerrint.CategoryCancelled, + statusDataLoss: dbsqlerrint.CategoryResultSet, + statusNetworkError: dbsqlerrint.CategoryConnectionError, + statusSqlError: dbsqlerrint.CategoryExecuteStatement, +} + +// Category satisfies the telemetry categorizer interface (read via +// CategoryFromError), so a kernel failure reports its code-derived category rather +// than one inferred from the message. Returns "" for an unmapped code, leaving the +// message-substring fallback in place. +func (e *KernelError) Category() dbsqlerrint.ErrorCategory { + return codeToCategory[e.Code] +} + // statementIDFromError returns the server query id carried on a KernelError, or "" // for a nil / non-KernelError error or one with no id. The execute-error path uses // it to set kernelOp.statementID (there is no exec handle to read it from), so the diff --git a/internal/backend/kernel/errors_classify_test.go b/internal/backend/kernel/errors_classify_test.go index 2140d3fa..d07cd2db 100644 --- a/internal/backend/kernel/errors_classify_test.go +++ b/internal/backend/kernel/errors_classify_test.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "testing" + + dbsqlerrint "github.com/databricks/databricks-sql-go/internal/errors" ) // These guard the safety contract (which errors may trigger database/sql's @@ -149,6 +151,44 @@ func TestStatementIDFromError(t *testing.T) { } } +// Category maps the kernel code to a telemetry category (read via +// CategoryFromError), so a kernel failure reports its authoritative code instead of +// a message-inferred guess. An unmapped code returns "" to keep the message fallback. +func TestKernelErrorCategory(t *testing.T) { + cases := []struct { + code int + want dbsqlerrint.ErrorCategory + }{ + {statusInvalidArgument, dbsqlerrint.CategoryInvalidRequest}, + {statusUnauthenticated, dbsqlerrint.CategoryAuthError}, + {statusPermissionDenied, dbsqlerrint.CategoryPermissionError}, + {statusNotFound, dbsqlerrint.CategoryNotFound}, + {statusResourceExhausted, dbsqlerrint.CategoryRateLimitExceeded}, + {statusUnavailable, dbsqlerrint.CategoryConnectionError}, + {statusTimeout, dbsqlerrint.CategoryTimeout}, + {statusCancelled, dbsqlerrint.CategoryCancelled}, + {statusDataLoss, dbsqlerrint.CategoryResultSet}, + {statusNetworkError, dbsqlerrint.CategoryConnectionError}, + {statusSqlError, dbsqlerrint.CategoryExecuteStatement}, + {9999, ""}, // unmapped → "" so classifyError falls back to the message + } + for _, c := range cases { + if got := (&KernelError{Code: c.code}).Category(); got != c.want { + t.Errorf("code %d: Category() = %q, want %q", c.code, got, c.want) + } + } +} + +// The category must survive the fmt.Errorf("%w") wrapping the execute/read paths +// apply, since CategoryFromError walks the chain to reach the *KernelError. This is +// the property the telemetry classifier depends on end to end. +func TestKernelErrorCategoryThroughWrap(t *testing.T) { + wrapped := fmt.Errorf("kernel: execute: %w", &KernelError{Code: statusSqlError, Message: "boom"}) + if got := dbsqlerrint.CategoryFromError(wrapped); got != dbsqlerrint.CategoryExecuteStatement { + t.Errorf("wrapped kernel error: CategoryFromError = %q, want %q", got, dbsqlerrint.CategoryExecuteStatement) + } +} + func contains(s, sub string) bool { return len(s) >= len(sub) && (s == sub || indexOf(s, sub) >= 0) } func indexOf(s, sub string) int { diff --git a/kernel_error_telemetry_e2e_test.go b/kernel_error_telemetry_e2e_test.go new file mode 100644 index 00000000..6c6dedee --- /dev/null +++ b/kernel_error_telemetry_e2e_test.go @@ -0,0 +1,119 @@ +//go:build cgo && databricks_kernel + +package dbsql + +import ( + "context" + "database/sql" + "encoding/json" + "io" + "net/http" + "strings" + "sync" + "testing" + + "github.com/databricks/databricks-sql-go/internal/config" + dbsqlerrint "github.com/databricks/databricks-sql-go/internal/errors" + "github.com/databricks/databricks-sql-go/telemetry" +) + +// captureTransport records the JSON body of every /telemetry-ext POST so a test can +// inspect what the driver actually put on the wire, while still hitting the real +// server for everything else. +type captureTransport struct { + base http.RoundTripper + mu sync.Mutex + logs []*telemetry.TelemetryEvent +} + +func (t *captureTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if strings.Contains(req.URL.Path, "/telemetry-ext") && req.Body != nil { + body, _ := io.ReadAll(req.Body) + req.Body = io.NopCloser(strings.NewReader(string(body))) + t.mu.Lock() + t.record(body) + t.mu.Unlock() + } + return t.base.RoundTrip(req) +} + +func (t *captureTransport) record(body []byte) { + var r telemetry.TelemetryRequest + if json.Unmarshal(body, &r) != nil { + return + } + for _, pl := range r.ProtoLogs { + var fl telemetry.TelemetryFrontendLog + if json.Unmarshal([]byte(pl), &fl) == nil && fl.Entry != nil && fl.Entry.SQLDriverLog != nil { + t.logs = append(t.logs, fl.Entry.SQLDriverLog) + } + } +} + +func (t *captureTransport) errorNames() []string { + t.mu.Lock() + defer t.mu.Unlock() + var names []string + for _, ev := range t.logs { + if ev.ErrorInfo != nil && ev.ErrorInfo.ErrorName != "" { + names = append(names, ev.ErrorInfo.ErrorName) + } + } + return names +} + +// TestKernelE2EErrorTelemetryCategory is the live proof of the change: a kernel SQL +// failure must report its code-derived category (execute_statement_failed for a +// SqlError) rather than the "error" message-fallback the classifier used before the +// KernelError.Category() mapping existed. +func TestKernelE2EErrorTelemetryCategory(t *testing.T) { + host, httpPath, token := pecoTestingCreds(t) + + cap := &captureTransport{base: http.DefaultTransport} + // Explicit telemetry opt-in + the capture transport, otherwise the same kernel + // connector a real consumer uses. + conn, err := NewConnector( + WithServerHostname(host), + WithHTTPPath(httpPath), + WithAccessToken(token), + WithUseKernel(true), + func(cfg *config.Config) { + cfg.EnableTelemetry = config.NewConfigValue(true) + cfg.TelemetryBatchSize = 1 + }, + ) + if err != nil { + t.Fatalf("NewConnector: %v", err) + } + c := conn.(*connector) + base := c.client.Transport + if base == nil { + base = http.DefaultTransport + } + cap.base = base + c.client.Transport = cap + + db := sql.OpenDB(conn) + defer db.Close() + + // A reference to a table that does not exist is a server-side SqlError: it + // carries a statement id (so the failure metric fires) and maps to + // CategoryExecuteStatement. + _, qErr := db.ExecContext(context.Background(), + "INSERT INTO does_not_exist_"+strings.Repeat("x", 8)+" VALUES (1)") + if qErr == nil { + t.Fatal("expected the bad-table statement to fail") + } + + // Flush pending telemetry synchronously. + db.Close() + + names := cap.errorNames() + want := string(dbsqlerrint.CategoryExecuteStatement) + for _, n := range names { + if n == want { + return + } + } + t.Fatalf("no error telemetry with error_name=%q; captured error names=%v", want, names) +} From 95be20bb6316f67da6204f0efa1606e75b3eeba0 Mon Sep 17 00:00:00 2001 From: Mani Kaustubh Mathur Date: Wed, 22 Jul 2026 19:21:39 +0000 Subject: [PATCH 2/3] feat(kernel): map remaining kernel status codes for telemetry 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 --- internal/backend/kernel/cgo.go | 2 ++ internal/backend/kernel/errors_classify.go | 22 ++++++++++++------- .../backend/kernel/errors_classify_test.go | 4 +++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/internal/backend/kernel/cgo.go b/internal/backend/kernel/cgo.go index bd5f6c2f..3fc26289 100644 --- a/internal/backend/kernel/cgo.go +++ b/internal/backend/kernel/cgo.go @@ -242,6 +242,8 @@ const ( _ = uint(statusTimeout-int(C.KernelStatusCode_Timeout)) | uint(int(C.KernelStatusCode_Timeout)-statusTimeout) _ = uint(statusCancelled-int(C.KernelStatusCode_Cancelled)) | uint(int(C.KernelStatusCode_Cancelled)-statusCancelled) _ = uint(statusDataLoss-int(C.KernelStatusCode_DataLoss)) | uint(int(C.KernelStatusCode_DataLoss)-statusDataLoss) + _ = uint(statusInternal-int(C.KernelStatusCode_Internal)) | uint(int(C.KernelStatusCode_Internal)-statusInternal) + _ = uint(statusInvalidStmtHandle-int(C.KernelStatusCode_InvalidStatementHandle)) | uint(int(C.KernelStatusCode_InvalidStatementHandle)-statusInvalidStmtHandle) _ = uint(statusNetworkError-int(C.KernelStatusCode_NetworkError)) | uint(int(C.KernelStatusCode_NetworkError)-statusNetworkError) _ = uint(statusSqlError-int(C.KernelStatusCode_SqlError)) | uint(int(C.KernelStatusCode_SqlError)-statusSqlError) ) diff --git a/internal/backend/kernel/errors_classify.go b/internal/backend/kernel/errors_classify.go index 70cbd477..a7594f29 100644 --- a/internal/backend/kernel/errors_classify.go +++ b/internal/backend/kernel/errors_classify.go @@ -32,6 +32,8 @@ const ( statusTimeout = 7 statusCancelled = 8 statusDataLoss = 9 + statusInternal = 10 + statusInvalidStmtHandle = 11 statusNetworkError = 12 statusSqlError = 13 ) @@ -66,8 +68,9 @@ func (e *KernelError) Error() string { // codeToCategory maps the kernel status code to a telemetry ErrorCategory. This is // the Go analog of the Python driver's _CODE_TO_EXCEPTION and Node's // mapKernelErrorToJsError: the kernel already hands us an authoritative code, so -// telemetry keys off it instead of re-guessing from the message text. A code with -// no precise category is omitted so classifyError falls back to message matching. +// telemetry keys off it instead of re-guessing from the message text. Every kernel +// status code is mapped (Internal / InvalidStatementHandle to generic buckets, as +// Python/Node do) so a *KernelError never depends on the message-substring fallback. var codeToCategory = map[int]dbsqlerrint.ErrorCategory{ statusInvalidArgument: dbsqlerrint.CategoryInvalidRequest, statusUnauthenticated: dbsqlerrint.CategoryAuthError, @@ -79,16 +82,19 @@ var codeToCategory = map[int]dbsqlerrint.ErrorCategory{ // Cancelled is generic (caller cancel, dropped future, or server-side cancel) // and classifyError also runs for session-lifecycle ops, so use the neutral // cancelled category rather than the statement-specific one. - statusCancelled: dbsqlerrint.CategoryCancelled, - statusDataLoss: dbsqlerrint.CategoryResultSet, - statusNetworkError: dbsqlerrint.CategoryConnectionError, - statusSqlError: dbsqlerrint.CategoryExecuteStatement, + statusCancelled: dbsqlerrint.CategoryCancelled, + statusDataLoss: dbsqlerrint.CategoryResultSet, + statusInternal: dbsqlerrint.CategoryGeneric, + // A closed/invalid statement handle is a use-after-close on the result path. + statusInvalidStmtHandle: dbsqlerrint.CategoryStatementClosed, + statusNetworkError: dbsqlerrint.CategoryConnectionError, + statusSqlError: dbsqlerrint.CategoryExecuteStatement, } // Category satisfies the telemetry categorizer interface (read via // CategoryFromError), so a kernel failure reports its code-derived category rather -// than one inferred from the message. Returns "" for an unmapped code, leaving the -// message-substring fallback in place. +// than one inferred from the message. Every kernel code is mapped; a value outside +// the enum returns "" and only then does classifyError fall back to the message. func (e *KernelError) Category() dbsqlerrint.ErrorCategory { return codeToCategory[e.Code] } diff --git a/internal/backend/kernel/errors_classify_test.go b/internal/backend/kernel/errors_classify_test.go index d07cd2db..9f72d261 100644 --- a/internal/backend/kernel/errors_classify_test.go +++ b/internal/backend/kernel/errors_classify_test.go @@ -168,9 +168,11 @@ func TestKernelErrorCategory(t *testing.T) { {statusTimeout, dbsqlerrint.CategoryTimeout}, {statusCancelled, dbsqlerrint.CategoryCancelled}, {statusDataLoss, dbsqlerrint.CategoryResultSet}, + {statusInternal, dbsqlerrint.CategoryGeneric}, + {statusInvalidStmtHandle, dbsqlerrint.CategoryStatementClosed}, {statusNetworkError, dbsqlerrint.CategoryConnectionError}, {statusSqlError, dbsqlerrint.CategoryExecuteStatement}, - {9999, ""}, // unmapped → "" so classifyError falls back to the message + {9999, ""}, // outside the enum → "" so classifyError falls back to the message } for _, c := range cases { if got := (&KernelError{Code: c.code}).Category(); got != c.want { From 7bd31b8286018568de9ba4d97513ef87719c8f49 Mon Sep 17 00:00:00 2001 From: Mani Kaustubh Mathur Date: Thu, 23 Jul 2026 04:51:19 +0000 Subject: [PATCH 3/3] test(kernel): guard codeToCategory exhaustiveness; fix doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/backend/kernel/errors_classify.go | 4 +-- .../backend/kernel/errors_classify_test.go | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/backend/kernel/errors_classify.go b/internal/backend/kernel/errors_classify.go index a7594f29..73b7c39c 100644 --- a/internal/backend/kernel/errors_classify.go +++ b/internal/backend/kernel/errors_classify.go @@ -69,8 +69,8 @@ func (e *KernelError) Error() string { // the Go analog of the Python driver's _CODE_TO_EXCEPTION and Node's // mapKernelErrorToJsError: the kernel already hands us an authoritative code, so // telemetry keys off it instead of re-guessing from the message text. Every kernel -// status code is mapped (Internal / InvalidStatementHandle to generic buckets, as -// Python/Node do) so a *KernelError never depends on the message-substring fallback. +// status code is mapped (Internal to the generic bucket, matching how Python/Node +// bucket it) so a *KernelError never depends on the message-substring fallback. var codeToCategory = map[int]dbsqlerrint.ErrorCategory{ statusInvalidArgument: dbsqlerrint.CategoryInvalidRequest, statusUnauthenticated: dbsqlerrint.CategoryAuthError, diff --git a/internal/backend/kernel/errors_classify_test.go b/internal/backend/kernel/errors_classify_test.go index 9f72d261..32eeec09 100644 --- a/internal/backend/kernel/errors_classify_test.go +++ b/internal/backend/kernel/errors_classify_test.go @@ -181,6 +181,32 @@ func TestKernelErrorCategory(t *testing.T) { } } +// allKernelStatusCodes is the full C enum (KernelStatusCode 1..13). It is the +// tripwire for the "every code is mapped" invariant: adding a status constant here +// makes TestKernelErrorCategoryExhaustive fail until codeToCategory maps it, so a +// new code can't silently fall through to the message classifier. +var allKernelStatusCodes = []int{ + statusInvalidArgument, statusUnauthenticated, statusPermissionDenied, + statusNotFound, statusResourceExhausted, statusUnavailable, statusTimeout, + statusCancelled, statusDataLoss, statusInternal, statusInvalidStmtHandle, + statusNetworkError, statusSqlError, +} + +// Every kernel status code resolves to a non-empty category, and the map holds +// exactly those codes — so a code added without a mapping (or a stray extra entry) +// fails here rather than silently regressing to message matching. +func TestKernelErrorCategoryExhaustive(t *testing.T) { + for _, code := range allKernelStatusCodes { + if (&KernelError{Code: code}).Category() == "" { + t.Errorf("code %d has no category — add it to codeToCategory", code) + } + } + if len(codeToCategory) != len(allKernelStatusCodes) { + t.Errorf("codeToCategory has %d entries, want %d (map and status-code list drifted)", + len(codeToCategory), len(allKernelStatusCodes)) + } +} + // The category must survive the fmt.Errorf("%w") wrapping the execute/read paths // apply, since CategoryFromError walks the chain to reach the *KernelError. This is // the property the telemetry classifier depends on end to end.