Skip to content

Commit 449a4f2

Browse files
committed
Address PR #321 review comments: Fix ExecContext telemetry timing and CloseOperation error tracking
This commit addresses two key review comments from @vikrantpuppala on PR #321: 1. **ExecContext timing fix**: Capture execution start time BEFORE running query - Now captures `executeStart := time.Now()` before `runQuery()` call - Uses `BeforeExecuteWithTime()` with pre-captured timestamp - Matches the pattern already implemented in QueryContext - Ensures telemetry accurately measures actual query execution time 2. **CloseOperation error tracking**: Capture cleanup errors in telemetry - Added `closeOpErr` variable to track CloseOperation failures - Includes CloseOperation errors in telemetry's deferred function - Provides observability for resource cleanup issues - Operation still returns success to caller (cleanup is best-effort) These changes ensure telemetry captures the complete statement lifecycle, including both execution timing and cleanup operations, without impacting the caller's error handling semantics. Co-authored-by: Isaac
1 parent d24f896 commit 449a4f2

File tree

1 file changed

+5
-1
lines changed

1 file changed

+5
-1
lines changed

connection.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
125125

126126
corrId := driverctx.CorrelationIdFromContext(ctx)
127127

128+
// Capture execution start time for telemetry before running the query
129+
executeStart := time.Now()
128130
var pollCount int
129131
exStmtResp, opStatusResp, err := c.runQuery(ctx, query, args, &pollCount)
130132
log, ctx = client.LoggerAndContext(ctx, exStmtResp)
@@ -135,7 +137,8 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
135137
var closeOpErr error // Track CloseOperation errors for telemetry
136138
if c.telemetry != nil && exStmtResp != nil && exStmtResp.OperationHandle != nil && exStmtResp.OperationHandle.OperationId != nil {
137139
statementID = client.SprintGuid(exStmtResp.OperationHandle.OperationId.GUID)
138-
ctx = c.telemetry.BeforeExecute(ctx, c.id, statementID)
140+
// Use BeforeExecuteWithTime to set the correct start time (before execution)
141+
ctx = c.telemetry.BeforeExecuteWithTime(ctx, c.id, statementID, executeStart)
139142
defer func() {
140143
finalErr := err
141144
if stagingErr != nil {
@@ -149,6 +152,7 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
149152
c.telemetry.CompleteStatement(ctx, statementID, finalErr != nil)
150153
}()
151154
c.telemetry.AddTag(ctx, "poll_count", pollCount)
155+
c.telemetry.AddTag(ctx, "operation_type", telemetry.OperationTypeExecuteStatement)
152156
}
153157

154158
if exStmtResp != nil && exStmtResp.OperationHandle != nil {

0 commit comments

Comments
 (0)