feat(csharp/src/Drivers/Databricks): Add comprehensive connection logging to Databricks ADBC driver #3577
+120
−26
Conversation
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
This commit adds detailed activity tracing for the Databricks connection phase to improve observability and debugging. Changes: - Added driver information logging (name, version, assembly) at connection start - Added connection properties logging with exact ADBC parameter names - Implemented property sanitization for sensitive values (passwords, tokens, secrets) - Added OpenSession request logging (protocol version, namespace, configuration) - Added OpenSession response logging with protocol capabilities and feature flags - Added separate logging for protocol-dependent features vs user settings - Added feature downgrade logging when protocol doesn't support requested features - Added namespace handling and fallback logging The logging follows the pattern used by commercial ODBC drivers like Simba, providing comprehensive visibility into the connection establishment process. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove connection.client_protocol_version (duplicate of connection.client_protocol) - Remove connection.server_protocol_version_value (duplicate of connection.server_protocol_version) - Remove total_properties from connection.properties.end event 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The previous implementation created nested OpenAsync activities which caused HandleOpenSessionResponse logging to be lost. Now all connection logging (driver info, properties, request, response, protocol capabilities, and feature flags) will appear in a single OpenAsync activity. Changes: - Removed duplicate OpenAsync override that created nested activity - Moved driver info and property logging to CreateSessionRequest - All logging now uses the base class's traced OpenAsync activity 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Events added late in the activity lifecycle may not be captured by the activity listener. Changed protocol capabilities and feature flags from AddEvent to SetTag so they appear reliably in the log output. Changes: - Protocol capabilities now logged as tags (connection.protocol.*) - Feature flags now logged as tags (connection.feature.*) - Feature downgrades now logged as tags when they occur 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
eric-wang-1990
commented
Oct 15, 2025
|
|
||
| protected override async Task HandleOpenSessionResponse(TOpenSessionResp? session, Activity? activity = default) | ||
| { | ||
| activity?.AddEvent("connection.open_session_response.received"); |
Contributor
Author
There was a problem hiding this comment.
It seems that since this HandleOpenSessionresponse is called from after this OpenSession
, and operation to the activity after that call will somehow not be serialized to the log. Do you have any idea why? @birschick-bq
CurtHagenlocher
changed the title
Contributor
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Overall these changes look fine to me, but I haven't really developed an internalized sense for System.Diagnostics.Activity so I'm hoping that @birschick-bq can provide feedback too.
…databricks-connection-logging
…y.Current
Changed CreateSessionRequest to wrap its logic in this.TraceActivity(activity => {...})
instead of relying on Activity.Current. This creates a proper child Activity span
for the session creation operation and improves tracing isolation.
All Activity.Current references have been replaced with the explicit activity parameter.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
@CurtHagenlocher Are you able to merge this? |
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.
Summary
This PR adds detailed activity tracing for the Databricks connection phase to improve observability and debugging. The logging follows the pattern used by commercial ODBC drivers like Simba.
Key improvements:
adbc.databricks.xxx,adbc.spark.xxx)***Changes
DatabricksConnection.cs:OpenAsync()override to log driver info and connection properties within activity contextLogConnectionProperties()method to iterate and log all properties with sanitizationCreateSessionRequest()to log request detailsHandleOpenSessionResponse()to log protocol negotiation and feature flagsExample Log Output
Connection properties are now logged in the activity trace with exact names:
How is this tested?
dotnet build🤖 Generated with Claude Code