Skip to content

Commit 746974e

Browse files
committed
Address PR review comment: Track CloseOperation errors in telemetry
This addresses @vikrantpuppala's review comment: "if the CloseOperation call fails below, that error is logged but never reflected in telemetry so we're missing capturing some errors in telemetry" Changes: - Added closeOpErr variable to capture CloseOperation failures - Include CloseOperation errors in telemetry's deferred function - Provides observability for resource cleanup issues - Operation still returns success to caller (cleanup is best-effort) Note: The timing fix ("shouldn't this be before runQuery?") will be addressed in the follow-up PR once BeforeExecuteWithTime infrastructure is available.
1 parent 4e38383 commit 746974e

File tree

1 file changed

+6
-0
lines changed

1 file changed

+6
-0
lines changed

connection.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
129129

130130
// Telemetry: track statement execution
131131
var statementID string
132+
var closeOpErr error // Track CloseOperation errors for telemetry
132133
if c.telemetry != nil && exStmtResp != nil && exStmtResp.OperationHandle != nil && exStmtResp.OperationHandle.OperationId != nil {
133134
statementID = client.SprintGuid(exStmtResp.OperationHandle.OperationId.GUID)
134135
ctx = c.telemetry.BeforeExecute(ctx, statementID)
@@ -137,6 +138,10 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
137138
if stagingErr != nil {
138139
finalErr = stagingErr
139140
}
141+
// Include CloseOperation error in telemetry if it occurred
142+
if closeOpErr != nil && finalErr == nil {
143+
finalErr = closeOpErr
144+
}
140145
c.telemetry.AfterExecute(ctx, finalErr)
141146
c.telemetry.CompleteStatement(ctx, statementID, finalErr != nil)
142147
}()
@@ -152,6 +157,7 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
152157
})
153158
if err1 != nil {
154159
log.Err(err1).Msg("databricks: failed to close operation after executing statement")
160+
closeOpErr = err1 // Capture for telemetry
155161
}
156162
}
157163
}

0 commit comments

Comments
 (0)