- 
                Notifications
    
You must be signed in to change notification settings  - Fork 161
 
feat(csharp/src/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection #3218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(csharp/src/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection #3218
Conversation
… to be set from TracingConnection
| 
           @jadewang-db @jeremytang-db  | 
    
| 
           So the idea is to be able to do something like this?  | 
    
          
 One option is to start a new Listeners for each connection - in which case, you would just use a new Guid for each connection/listener combo. Or, more complicated is that you hash the connection/token to produce a unique reproduceable value that represents the Id and only start a new Listener when that Id doesn't already exist. But this will be more complicated in the Dispose() code to ensure the last source won't send any more messages. this.SourceId = CalculateSourceId(properties);
...
ShouldListenTo = (activitySource) => 
    activitySource.Tags.Any(t => t.Key == "DatabricksSourceId" && t.Value?.Equals(this.SourceId) == true) | 
    
| 
           @birschick-bq 
  | 
    
          
 That is correct. This filters out any messages not generated by your connection. And ShouldListenTo is not called for every Activity, but two times for each ActivitySource (==TracingConnection). The ActivityListener can be instantiated inside the Connection and be disposed when the connection disposes.  | 
    
| /// <param name="activitySourceVersion">The version of the component publishing the tracing info.</param> | ||
| /// <param name="traceParent">The trace parent context, which is used to link the activity to a distributed trace.</param> | ||
| /// <param name="tags">The tags associated with the activity.</param> | ||
| public ActivityTrace(string? activitySourceName = default, string? activitySourceVersion = default, string? traceParent = default, IEnumerable<KeyValuePair<string, object?>>? tags = default) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a binary breaking change. Was this class already part of the last release? If we had "declared 1.0" and already included this class in a public release then we'd want to add an overload instead of changing the signature. (As we are not yet at 1.0, I think it's probably okay not to worry about binary compatibility in this case.)
Provides an virtual override for
GetActivitySourceTags(properties)to retrieve tags when creating theActivitySource.Also adds the ActivitySourceName property so an
ActivityListenercan create a useful filter.