eric-wang-1990 commented on code in PR #3577:
URL: https://github.com/apache/arrow-adbc/pull/3577#discussion_r2466433033
##########
csharp/src/Drivers/Databricks/DatabricksConnection.cs:
##########
@@ -668,17 +695,34 @@ public override AdbcStatement CreateStatement()
protected override TOpenSessionReq CreateSessionRequest()
{
+ // Log driver information at the beginning of the connection
+ Activity.Current?.AddEvent("connection.driver.info", [
+ new("driver.name", "Apache Arrow ADBC Databricks Driver"),
+ new("driver.version", s_assemblyVersion),
+ new("driver.assembly", s_assemblyName)
+ ]);
+
+ // Log connection properties (sanitize sensitive values)
+ LogConnectionProperties(Activity.Current);
+
var req = new TOpenSessionReq
{
Client_protocol =
TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V7,
Client_protocol_i64 =
(long)TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V7,
CanUseMultipleCatalogs = _enableMultipleCatalogSupport,
};
+ // Log OpenSession request details
+
Activity.Current?.AddEvent("connection.open_session_request.creating");
+ Activity.Current?.SetTag("connection.client_protocol",
req.Client_protocol.ToString());
+ Activity.Current?.SetTag("connection.can_use_multiple_catalogs",
_enableMultipleCatalogSupport);
+
// Set default namespace if available
if (_defaultNamespace != null)
{
req.InitialNamespace = _defaultNamespace;
+
Activity.Current?.SetTag("connection.initial_namespace.catalog",
_defaultNamespace.CatalogName ?? "(none)");
+
Activity.Current?.SetTag("connection.initial_namespace.schema",
_defaultNamespace.SchemaName ?? "(none)");
Review Comment:
So this is the initial namespace catalog and schema, which can be passed
from user-passed in connection property or from dbx workspace setting. Is this
considered as PII? We log user passed-in attributes already in simba drivers.
##########
csharp/src/Drivers/Databricks/DatabricksConnection.cs:
##########
@@ -100,6 +101,32 @@ public DatabricksConnection(IReadOnlyDictionary<string,
string> properties) : ba
ValidateProperties();
}
+ private void LogConnectionProperties(Activity? activity)
+ {
+ if (activity == null) return;
+
+ activity.AddEvent("connection.properties.start");
+
+ foreach (var kvp in Properties)
+ {
+ string key = kvp.Key;
+ string value = kvp.Value;
+
+ // Sanitize sensitive properties - only mask actual
credentials/tokens, not configuration
+ bool isSensitive = key.IndexOf("password",
StringComparison.OrdinalIgnoreCase) >= 0 ||
+ key.IndexOf("secret",
StringComparison.OrdinalIgnoreCase) >= 0 ||
+ key.Equals(AdbcOptions.Password,
StringComparison.OrdinalIgnoreCase) ||
+ key.Equals(SparkParameters.AccessToken,
StringComparison.OrdinalIgnoreCase) ||
+
key.Equals(DatabricksParameters.OAuthClientSecret,
StringComparison.OrdinalIgnoreCase);
+
+ string logValue = isSensitive ? "***" : value;
+
+ activity.SetTag(key, logValue);
+ }
+
+ activity.AddEvent("connection.properties.end");
Review Comment:
I am gonna remove this start/end pattern, since I am setting the tags to the
whole activity span using activity.SetTag, adding the event does not quite
makes sense, since those tags are not attached to the event.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
