-
Notifications
You must be signed in to change notification settings - Fork 0
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/Drivers/Apache): tracing proof-of-concept #5
base: main
Are you sure you want to change the base?
Conversation
For this first iteration, is the thought how to establish a framework for the C# drivers to implement? I was thinking it was targeting the higher-level classes like AdbcConnection so that any of the drivers, including the ones used via Interop, would have the same base logging. |
Okay. That is possible. public abstract class AdbcConnection : IDisposable
{
public AdbcConnection(IReadOnlyDictionary<string, string>? options)
{
// Check for trace options and add if requested ...
EnsureListener();
ActivitySource = new(s_activitySourceName, s_assemblyVersion);
}
public ActivitySource? ActivitySource { get; }
} Will we need to introduce this in a new interface version (1.2)? Or is it possible to "sneak" this into the existing version using I can make some of these changes to move the implementation into |
I don't think we need a separate interface version, unless we are proposing specific options that should be captured for the Statement, like an ActivityId or something specific. |
@davidhcoe - Thanks for the input. |
@@ -6,7 +6,7 @@ | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="ApacheThrift" Version="0.21.0" /> | |||
<PackageReference Include="System.Text.Json" Version="8.0.5" /> | |||
<PackageReference Include="System.Net.Http" Version="4.3.4" /> |
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.
System.Net.Http
4.3.0 (from ApacheThrift
nuget) is marked a vulnerable. We can remove this when ApacheThrift
is updated.
@davidhcoe
There is pretty small amount of instrumentation and boiler plate code that is necessary from the driver standpoint, I think. Two features I'd like to pursue, assuming we're on the right track with this POC, ...
|
…ort for AdbcConnection11 and AdbcStatement11
|
POC
Assumptions
CSharp
In csharp, using System.Diagnostics.DiagnosticSource package employing as it is the suggested way to enable OpenTelemetry compatible tracing.
ActivitySource
ActivityListener
Activity
ActivityEvent
Tags
Intent is to allow the passing of the "parent" span and trace id to the
AdbcStatement
usingSetOption
. The Execute* API methods will populate the parent context if the option is set. Implicit is the ability to clear the parent context.Sample output:
Successful activity (multiple calls to
ExecuteUpdateAsync
)Exception activity
Instrumentation
StartActivity
in each significant method (seeOpenAsync
, for example)ActivityEvent
before and after its execution.Adbc*
implemented methods (i.e.,AdbcStatement.ExecuteQuery
, etc.) to catch and trace any exceptions then rethrow the exception.Missing Features