You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "msrathore-db (via GitHub)" <gi...@apache.org> on 2025/10/25 13:34:03 UTC
[PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
msrathore-db opened a new pull request, #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617
## Summary
Adds comprehensive Activity-based logging to `DatabricksStatement.cs` for improved observability and debugging of Databricks ADBC operations.
## Changes
### Methods with Logging Added
- **`SetStatementProperties`**: Logs configuration (Arrow native types, CloudFetch settings, async mode)
- **`GetSchemaFromMetadata`**: Logs schema parsing decisions (Arrow vs Thrift, column count)
- **`GetCatalogsAsync`**: Logs catalog queries with feature flags and row counts
- **`GetSchemasAsync`**: Logs schema queries with catalog filtering
- **`GetTablesAsync`**: Logs table queries with SPARK catalog handling
- **`GetColumnsAsync`**: Logs column queries with search criteria
- **`GetPrimaryKeysAsync`**: Logs primary key queries
- **`GetCrossReferenceAsync`**: Logs foreign key queries
- **`GetColumnsExtendedAsync`**: Logs DESC TABLE EXTENDED queries
- **`SetOption`**: Logs configuration changes
### What Gets Logged
**Tags:**
- Statement configuration (CloudFetch settings, Arrow native types, batch size)
- Feature flags (`enable_multiple_catalog_support`, `pk_fk_enabled`)
- Query parameters (catalog, schema, table, column names)
- Results (`db.response.returned_rows`)
**Events:**
- `statement.<operation>.start` / `statement.<operation>.complete`
- Decision points (`calling_base_implementation`, `returning_empty_result`, `fallback_to_base`)
- Schema handling (`using_arrow_schema`, `fallback_to_thrift`)
## Example Log Output
```json
{
"OperationName": "GetCatalogs",
"Duration": "00:00:00.582",
"TagObjects": {
"statement.feature.enable_multiple_catalog_support": true,
"db.response.returned_rows": 28
},
"Events": [
{ "Name": "statement.get_catalogs.start" },
{ "Name": "statement.get_catalogs.calling_base_implementation" },
{ "Name": "statement.get_catalogs.complete" }
]
}
```
## Testing
Tested locally by enabling logging with:
```csharp
properties["adbc.traces.exporter"] = "adbcfile";
```
Verified that all tags, events, and distributed tracing context are captured correctly in trace files for all implemented methods (GetCatalogs, GetSchemas, GetTables, GetColumns, and statement configuration).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
Posted by "msrathore-db (via GitHub)" <gi...@apache.org>.
msrathore-db commented on code in PR #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617#discussion_r2492381283
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -173,6 +210,8 @@ public override void SetOption(string key, string value)
{
long maxBytesPerFileValue = DatabricksConnection.ParseBytesWithUnits(value);
this.maxBytesPerFile = maxBytesPerFileValue;
+ Activity.Current?.SetTag("statement.cloudfetch.max_bytes_per_file", this.maxBytesPerFile);
Review Comment:
Made the change. Thanks
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
Posted by "eric-wang-1990 (via GitHub)" <gi...@apache.org>.
eric-wang-1990 commented on code in PR #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617#discussion_r2480184115
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -88,22 +89,34 @@ public DatabricksStatement(DatabricksConnection connection)
/// <returns>The Arrow schema</returns>
protected override Schema GetSchemaFromMetadata(TGetResultSetMetadataResp metadata)
{
+ // Log schema parsing decision
+ Activity.Current?.SetTag("statement.schema.has_arrow_schema", metadata.__isset.arrowSchema);
+
// For Protocol V5+, prefer Arrow schema if available
if (metadata.__isset.arrowSchema)
{
Schema? arrowSchema = ((DatabricksSchemaParser)Connection.SchemaParser).ParseArrowSchema(metadata.ArrowSchema);
if (arrowSchema != null)
{
+ Activity.Current?.SetTag("statement.schema.source", "arrow");
+ Activity.Current?.SetTag("statement.schema.column_count", arrowSchema.FieldsList.Count);
+ Activity.Current?.AddEvent("statement.schema.using_arrow_schema");
Review Comment:
Let's remove this event, you already have tag as arrow
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -88,22 +89,34 @@ public DatabricksStatement(DatabricksConnection connection)
/// <returns>The Arrow schema</returns>
protected override Schema GetSchemaFromMetadata(TGetResultSetMetadataResp metadata)
{
+ // Log schema parsing decision
+ Activity.Current?.SetTag("statement.schema.has_arrow_schema", metadata.__isset.arrowSchema);
+
// For Protocol V5+, prefer Arrow schema if available
if (metadata.__isset.arrowSchema)
{
Schema? arrowSchema = ((DatabricksSchemaParser)Connection.SchemaParser).ParseArrowSchema(metadata.ArrowSchema);
if (arrowSchema != null)
{
+ Activity.Current?.SetTag("statement.schema.source", "arrow");
+ Activity.Current?.SetTag("statement.schema.column_count", arrowSchema.FieldsList.Count);
+ Activity.Current?.AddEvent("statement.schema.using_arrow_schema");
return arrowSchema;
}
}
// Fallback to traditional Thrift schema
- return Connection.SchemaParser.GetArrowSchema(metadata.Schema, Connection.DataTypeConversion);
+ Activity.Current?.SetTag("statement.schema.source", "thrift");
+ Activity.Current?.AddEvent("statement.schema.fallback_to_thrift");
Review Comment:
Remove this event
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -173,6 +210,8 @@ public override void SetOption(string key, string value)
{
long maxBytesPerFileValue = DatabricksConnection.ParseBytesWithUnits(value);
this.maxBytesPerFile = maxBytesPerFileValue;
+ Activity.Current?.SetTag("statement.cloudfetch.max_bytes_per_file", this.maxBytesPerFile);
Review Comment:
These are duplicates, you already logged these in SetStatementProperties
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -173,6 +210,8 @@ public override void SetOption(string key, string value)
{
long maxBytesPerFileValue = DatabricksConnection.ParseBytesWithUnits(value);
this.maxBytesPerFile = maxBytesPerFileValue;
+ Activity.Current?.SetTag("statement.cloudfetch.max_bytes_per_file", this.maxBytesPerFile);
Review Comment:
And remove all the other logs in this function.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
Posted by "eric-wang-1990 (via GitHub)" <gi...@apache.org>.
eric-wang-1990 commented on code in PR #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617#discussion_r2466269869
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -88,22 +89,34 @@ public DatabricksStatement(DatabricksConnection connection)
/// <returns>The Arrow schema</returns>
protected override Schema GetSchemaFromMetadata(TGetResultSetMetadataResp metadata)
{
+ // Log schema parsing decision
+ Activity.Current?.SetTag("statement.schema.has_arrow_schema", metadata.__isset.arrowSchema);
Review Comment:
Does this output logs correctly? Why not using this.TraceActivity?
@birschick-bq What should be the recommended way for trace?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
Posted by "msrathore-db (via GitHub)" <gi...@apache.org>.
msrathore-db commented on code in PR #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617#discussion_r2492381794
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -88,22 +89,34 @@ public DatabricksStatement(DatabricksConnection connection)
/// <returns>The Arrow schema</returns>
protected override Schema GetSchemaFromMetadata(TGetResultSetMetadataResp metadata)
{
+ // Log schema parsing decision
+ Activity.Current?.SetTag("statement.schema.has_arrow_schema", metadata.__isset.arrowSchema);
+
// For Protocol V5+, prefer Arrow schema if available
if (metadata.__isset.arrowSchema)
{
Schema? arrowSchema = ((DatabricksSchemaParser)Connection.SchemaParser).ParseArrowSchema(metadata.ArrowSchema);
if (arrowSchema != null)
{
+ Activity.Current?.SetTag("statement.schema.source", "arrow");
+ Activity.Current?.SetTag("statement.schema.column_count", arrowSchema.FieldsList.Count);
+ Activity.Current?.AddEvent("statement.schema.using_arrow_schema");
return arrowSchema;
}
}
// Fallback to traditional Thrift schema
- return Connection.SchemaParser.GetArrowSchema(metadata.Schema, Connection.DataTypeConversion);
+ Activity.Current?.SetTag("statement.schema.source", "thrift");
+ Activity.Current?.AddEvent("statement.schema.fallback_to_thrift");
Review Comment:
Updated. Thanks
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617#discussion_r2466462783
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -88,22 +89,34 @@ public DatabricksStatement(DatabricksConnection connection)
/// <returns>The Arrow schema</returns>
protected override Schema GetSchemaFromMetadata(TGetResultSetMetadataResp metadata)
{
+ // Log schema parsing decision
+ Activity.Current?.SetTag("statement.schema.has_arrow_schema", metadata.__isset.arrowSchema);
Review Comment:
I was concerned that `Activity.Current` might not be set to the activity in your current block - i.e., it might have been set for a child call. But in thinking about it, I don't think that would be the case, as long as the child call disposed the Activity in their scope.
However, I think the best usage pattern for `Activity.Current` is where you don't start a new activity and you don't pass an existing activity as a parameter. That is, a situation in which you might be called from a method that may or may not have a current activity started.
So I think `Activity.Current` is used appropriately in the case shown in line 93, above.
Use `this.TraceActivity` when you want a new activity (line) with its own events and tags. Typically something "major" or structural in your code. If the new activity (`this.TraceActivity`) is a child call, the `ParentId` will be set to indicate it is a child activity of the parent activity.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Add Activity-based logging to DatabricksStatement [arrow-adbc]
Posted by "msrathore-db (via GitHub)" <gi...@apache.org>.
msrathore-db commented on code in PR #3617:
URL: https://github.com/apache/arrow-adbc/pull/3617#discussion_r2492380782
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -88,22 +89,34 @@ public DatabricksStatement(DatabricksConnection connection)
/// <returns>The Arrow schema</returns>
protected override Schema GetSchemaFromMetadata(TGetResultSetMetadataResp metadata)
{
+ // Log schema parsing decision
+ Activity.Current?.SetTag("statement.schema.has_arrow_schema", metadata.__isset.arrowSchema);
+
// For Protocol V5+, prefer Arrow schema if available
if (metadata.__isset.arrowSchema)
{
Schema? arrowSchema = ((DatabricksSchemaParser)Connection.SchemaParser).ParseArrowSchema(metadata.ArrowSchema);
if (arrowSchema != null)
{
+ Activity.Current?.SetTag("statement.schema.source", "arrow");
+ Activity.Current?.SetTag("statement.schema.column_count", arrowSchema.FieldsList.Count);
+ Activity.Current?.AddEvent("statement.schema.using_arrow_schema");
Review Comment:
Makes sense. Updated.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org