You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "birschick-bq (via GitHub)" <gi...@apache.org> on 2025/07/29 23:46:38 UTC
[PR] feat(csharp/src/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
birschick-bq opened a new pull request, #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218
Provides an virtual override for `GetActivitySourceTags(properties)` to retrieve tags when creating the `ActivitySource`.
Also adds the ActivitySourceName property so an `ActivityListener` can create a useful filter.
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#issuecomment-3134408346
@jadewang-db @jeremytang-db
Please review when you have a chance. 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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "jeremytang-db (via GitHub)" <gi...@apache.org>.
jeremytang-db commented on PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#issuecomment-3134464943
So the idea is to be able to do something like this?
`ShouldListenTo = (activitySource) => activitySource.Tags["Hosturl"] = "abc.databricks.com"`
If there are mulitple instances of DatabricksConnection with the same combination of tags, wouldn't all the listeners pick up on all the activities?
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#issuecomment-3137916645
> @birschick-bq So if I understand correctly, for the first approach:
>
> 1. Each connection generates a GUID
> 2. Add the GUID to the ActivitySource tags
> 3. Each connection creates a Listener and passes their own GUID to it
> 4. Each listener checks for the GUID in the activitySource tags to listen to
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.
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#issuecomment-3137449634
> So the idea is to be able to do something like this? `ShouldListenTo = (activitySource) => activitySource.Tags["Hosturl"] = "abc.databricks.com"` If there are mulitple instances of DatabricksConnection with the same combination of tags, wouldn't all the listeners pick up on all the activities?
@jeremytang-db
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.
```csharp
this.SourceId = CalculateSourceId(properties);
...
ShouldListenTo = (activitySource) =>
activitySource.Tags.Any(t => t.Key == "DatabricksSourceId" && t.Value?.Equals(this.SourceId) == true)
```
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#discussion_r2246271268
##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs:
##########
@@ -33,8 +33,11 @@ public sealed class ActivityTrace : IDisposable
/// Constructs a new <see cref="ActivityTrace"/> object. If <paramref name="activitySourceName"/> is set, it provides the
/// activity source name, otherwise the current assembly name is used as the activity source name.
/// </summary>
- /// <param name="activitySourceName"></param>
- public ActivityTrace(string? activitySourceName = default, string? activitySourceVersion = default, string? traceParent = default)
+ /// <param name="activitySourceName">The name of the ActivitySource object</param>
+ /// <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)
Review Comment:
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.)
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "jeremytang-db (via GitHub)" <gi...@apache.org>.
jeremytang-db commented on PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#issuecomment-3137895585
@birschick-bq
So if I understand correctly, for the first approach:
1. Each connection generates a GUID
2. Add the GUID to the ActivitySource tags
3. Each connection creates a Listener and passes their own GUID to it
4. Each listener checks for the GUID in the activitySource tags to listen to
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher merged PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218
--
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/Apache.Arrow.Adbc/Tracing): allow ActivitySource tags to be set from TracingConnection [arrow-adbc]
Posted by "jadewang-db (via GitHub)" <gi...@apache.org>.
jadewang-db commented on PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#issuecomment-3137801323
> > So the idea is to be able to do something like this? `ShouldListenTo = (activitySource) => activitySource.Tags["Hosturl"] = "abc.databricks.com"` If there are mulitple instances of DatabricksConnection with the same combination of tags, wouldn't all the listeners pick up on all the activities?
>
> @jeremytang-db
>
> 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.
>
> ```cs
> this.SourceId = CalculateSourceId(properties);
> ...
> ShouldListenTo = (activitySource) =>
> activitySource.Tags.Any(t => t.Key == "DatabricksSourceId" && t.Value?.Equals(this.SourceId) == true)
> ```
can we have a global singleton for this lister?
--
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