Skip to content

Conversation

@birschick-bq
Copy link
Contributor

@birschick-bq birschick-bq commented Aug 19, 2025

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:

Platform Folder
Windows %LOCALAPPDATA%/Apache.Arrow.Adbc/Traces
macOS $HOME/Library/Application Support/Apache.Arrow.Adbc/Traces
Linux $HOME/.local/share/Apache.Arrow.Adbc/Traces

By default, up to 999 files of maximum size 1024 KB are written to
the trace folder.

The environment variable OTEL_TRACES_EXPORTER can be used to select one of the
available exporters. Or the database parameter adbc.traces.exporter can be used,
which has precedence over the environment variable.

The following listeners are supported:

Listener Description
adbcfile Exports traces to rotating files in a folder.

The FileActivityListener is designed to allow an instance to be associated with and have the same lifespan as a Connection instance.

@birschick-bq birschick-bq marked this pull request as ready for review August 19, 2025 17:24
@github-actions github-actions bot added this to the ADBC Libraries 20 milestone Aug 19, 2025
@birschick-bq
Copy link
Contributor Author

@davidhcoe - Please review when you have a chance. Thanks.

@birschick-bq birschick-bq changed the title feat(csharp/src/Drivers/BigQuery): instrument tracing exporters for BigQuery driver feat(csharp/src/Drivers/BigQuery): instrument tracing exporters for BigQuery/Apache drivers Aug 21, 2025
@birschick-bq birschick-bq changed the title feat(csharp/src/Drivers/BigQuery): instrument tracing exporters for BigQuery/Apache drivers feat(csharp/src/Drivers): instrument tracing exporters for BigQuery/Apache drivers Aug 21, 2025
Copy link
Contributor Author

@birschick-bq birschick-bq Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CurtHagenlocher

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

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?

  1. Performance/stability of the file exporter?
  2. Only add the file exporter on the drivers and not the OTel-based exporters?
  3. Handle the ability to start and stop file tracing, connection-by-connection, rather than a global static.

@birschick-bq birschick-bq marked this pull request as draft September 4, 2025 17:23
@birschick-bq birschick-bq marked this pull request as draft September 19, 2025 20:40
@birschick-bq birschick-bq marked this pull request as ready for review September 20, 2025 17:42
@birschick-bq
Copy link
Contributor Author

@CurtHagenlocher -

Here is an alternative file exporter solution that does not include other OTel exporters.

It uses the ActivityListener instead of OTel BaseExporter. It is instanced and has the same lifespan as an AdbcConnection.

Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a 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.

@birschick-bq
Copy link
Contributor Author

birschick-bq commented Sep 23, 2025

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.

@CurtHagenlocher

I created a Listeners project to be a more "light-weight' version without needing the OTel libraries. The Listeners library uses the System.Diagnostics.ActivityListener directly. Also, as we now have a filter mechanism, we can match the lifetimes of the tracing with the Connection. Downside is that the trace files will be smaller and more numerous because they won't be any sharing among multiple Connections in the same process.

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 Exporters library/assembly is meant to support OTLP (and console) tracing.

@CurtHagenlocher
Copy link
Contributor

@birschick-bq How do you see this evolving as time goes on? It sounds like the file exporter in the Listeners project isn't fully compatible with the one in the Exporters project -- so switching to Exporters would be a breaking change? Then any new C#-based driver would have to decide which of the two to use/support?

Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a 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.

@CurtHagenlocher CurtHagenlocher merged commit 95fff2a into apache:main Sep 24, 2025
7 checks passed
@birschick-bq
Copy link
Contributor Author

@birschick-bq How do you see this evolving as time goes on? It sounds like the file exporter in the Listeners project isn't fully compatible with the one in the Exporters project -- so switching to Exporters would be a breaking change? Then any new C#-based driver would have to decide which of the two to use/support?

@CurtHagenlocher

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants