-
Notifications
You must be signed in to change notification settings - Fork 173
feat(csharp/src/Drivers): instrument tracing exporters for BigQuery/Apache drivers #3315
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): instrument tracing exporters for BigQuery/Apache drivers #3315
Conversation
|
@davidhcoe - Please review when you have a chance. Thanks. |
…ed implementation in Apache drivers
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.
The serialization was failing with this message:
"JsonTypeInfo metadata for type 'Apache.Hive.Service.Rpc.Thrift.TStatusCode' was not provided by TypeInfoResolver of type 'Apache.Arrow.Adbc.Telemetry.Traces.Exporters.FileExporter.SerializableActivityJsonContext'.
If using source generation, ensure that all root types passed to the serializer have been annotated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.
The unsupported member type is located on type 'System.Object'. Path: $.TagObjects."
I take this to mean we'd have to know ALL the types that could be serialized ahead of time. We could be more restrictive on what we allow in the AddTag. Presumably restricting this to string and numeric values.
I'll create a ticket to make that change in a future PR.
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.
Note: merged in changes from #3397
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.
@CurtHagenlocher - Just checking to see if there is something that needs to be addressed before we add exporters.
Can you let me know any of the following are important to look at?
- Performance/stability of the file exporter?
- Only add the file exporter on the drivers and not the OTel-based exporters?
- Handle the ability to start and stop file tracing, connection-by-connection, rather than a global static.
…formance of file exporter
…ick-bq/bigquery-exporter
…ick-bq/bigquery-exporter
|
Here is an alternative file exporter solution that does not include other OTel exporters. It uses the |
CurtHagenlocher
left a comment
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.
Thanks!
Perhaps it's because there's no good way to name things that's not overly generic, but I'm a bit confused about the current split into assemblies and how things depend on each other. What's the difference between "Exporters" and "Listeners"? "Exporters" depends on "Listeners" but nothing depends on "Listeners"?
Anyhow, a few minor changes requested.
csharp/test/Telemetry/Traces/Listeners/FileListener/FileActivityListenerTests.cs
Outdated
Show resolved
Hide resolved
csharp/src/Telemetry/Traces/Listeners/FileListener/TracingFile.cs
Outdated
Show resolved
Hide resolved
I created a Since much of the code for file listener is the same as the file exporter, I chose to make Exporter dependent on Listeners. Currently, there is no (known) consumer of the Exporter assembly/library. I didn't remove it for backwards compatibility reasons. The |
|
@birschick-bq How do you see this evolving as time goes on? It sounds like the file exporter in the |
CurtHagenlocher
left a comment
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.
Thanks! I'm going to check this in as-is, but I do think it would be good to have clarity on a future direction.
My feeling is that the OTel Exporter mechanism is better suited to end-user applications because it is static/global and works better with an end application's lifetime. Consumers of Exporters/Listeners do not inherit from these classes, but just instantiate an instance. So from the driver's standpoint, it shouldn't matter how the telemetry is observed. They could actually mix-and-match an Exporter (OTLP) and a Listener (File) in a single driver. Ideally, I would like to have a Listener version of OTLP where I could re-use the OTel implementation without the overhead/restrictions of the Exporters interface. I will create an issue placeholder to deal with this further. Thanks for your help/input. |
Adds instrumentation to support tracing exporters
Traces Listeners
FileActivityListener
Provides an ActivityListener to write telemetry traces to
rotating files in folder. File names are created with the following pattern:
<trace-source>-<YYYY-MM-DD-HH-mm-ss-fff>-<process-id>.log.For example:
apache.arrow.adbc.drivers.databricks-2025-08-15-10-35-56-012345-99999.log.The default folder used is:
%LOCALAPPDATA%/Apache.Arrow.Adbc/Traces$HOME/Library/Application Support/Apache.Arrow.Adbc/Traces$HOME/.local/share/Apache.Arrow.Adbc/TracesBy default, up to 999 files of maximum size 1024 KB are written to
the trace folder.
The environment variable
OTEL_TRACES_EXPORTERcan be used to select one of theavailable exporters. Or the database parameter
adbc.traces.exportercan be used,which has precedence over the environment variable.
The following listeners are supported:
adbcfileThe
FileActivityListeneris designed to allow an instance to be associated with and have the same lifespan as aConnectioninstance.