feat(go/adbc): refactor logging instrumentation into OTel tracing - part 3/3 - #4661
Open
birschick-bq wants to merge 41 commits into
Open
Conversation
… dev/birschick-bq/flight-sql-log-to-trace-4-connection
… dev/birschick-bq/flight-sql-log-to-trace-5-statements
…into dev/birschick-bq/flight-sql-log-to-trace-5-statements
… dev/birschick-bq/flight-sql-log-to-trace-4-connection
…into dev/birschick-bq/flight-sql-log-to-trace-5-statements
…ight-sql-log-to-trace-4-connection
…into dev/birschick-bq/flight-sql-log-to-trace-5-statements
…into dev/birschick-bq/flight-sql-log-to-trace-5-statements
birschick-bq
marked this pull request as ready for review
August 25, 2026 03:40
Contributor
Author
|
@lidavidm - The last of the PRs for moving logging to OTel tracing. |
lidavidm
reviewed
Aug 26, 2026
| require.Contains(t, output, "FlightSQL.Statement.ExecuteQuery") | ||
| } | ||
|
|
||
| func TestFlightSQLTracingCleansUpAfterConstructionFailure(t *testing.T) { |
Member
There was a problem hiding this comment.
What exactly is this testing/how would this test fail before?
| // For queries expected to be executed repeatedly, Prepare should be | ||
| // called before execution. | ||
| func (s *statement) SetSqlQuery(query string) error { | ||
| func (s *statement) SetSqlQuery(query string) (err error) { |
Member
There was a problem hiding this comment.
Huh, is this worth tracing? Seems like as long as the underlying close is traced this should be fine
|
|
||
| type recordReaderCallerContextKey struct{} | ||
|
|
||
| func isRecordReaderSiblingCancellation(ctx context.Context) bool { |
Member
There was a problem hiding this comment.
I think we should document what this is checking/trying to do
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
3 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.
This pull request makes significant improvements to the FlightSQL driver's tracing and logging, replacing legacy logging with structured OpenTelemetry tracing, enhancing error recording, and ensuring better test coverage for tracing-related cleanup. The changes modernize how operation metadata is captured and reported, making tracing more consistent and useful for observability and debugging.
Tracing and Logging Modernization:
Replaced
slog-based logging with OpenTelemetry tracing throughoutflightsql_statement.goandflightsql_bulk_ingest.go, usingattribute.KeyValuefor structured event attributes and adding detailed span events for operation start and finish. This includes updating helper functions to return attributes in the new format. [1] [2] [3] [4] [5] [6] [7]Updated error handling in tracing: errors are now consistently recorded as span errors with stack traces, and operation-specific error wrapping is improved for clarity. [1] [2] [3] [4] [5] [6]
API and Internal Refactoring:
Refactored method signatures and internal calls to use the new tracing and attribute formats, including changes to
SetSqlQuery,ExecuteQuery,ExecuteUpdate, andPreparemethods instatement. [1] [2] [3] [4] [5]Changed the construction of the Flight client to add a unary interceptor for response metadata, improving consistency for both unary and stream calls.
Testing Improvements:
TestFlightSQLTracingCleansUpAfterConstructionFailureto ensure that tracing resources are properly cleaned up even when database construction fails, preventing resource leaks.Dependency and Import Updates:
Helper and Utility Updates:
These changes collectively ensure that the driver emits rich, structured telemetry for all major operations, facilitates easier debugging, and aligns with modern observability practices.
Extends: #4659
Part 3/3