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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
