Skip to content

Conversation

@jadewang-db
Copy link
Contributor

No description provided.

@jadewang-db jadewang-db marked this pull request as ready for review October 28, 2025 21:55
@github-actions github-actions bot added this to the ADBC Libraries 21 milestone Oct 28, 2025
@jadewang-db jadewang-db changed the title Databricks adbc telemetry lld feat(csharp/src/Drivers/Databricks): Client Telemetry Design Oct 28, 2025
@jadewang-db jadewang-db changed the title feat(csharp/src/Drivers/Databricks): Client Telemetry Design docs(csharp/src/Drivers/Databricks): Client Telemetry Design Oct 28, 2025
/// <summary>
/// Tag definitions for Connection.Open events.
/// </summary>
internal static class ConnectionOpenEvent
Copy link
Contributor

Choose a reason for hiding this comment

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

I am trying to think if this is the best long term apporach to have each event be a separate class.
Currently the events are pretty free-formed and sometimes it is used to designate the start/end of an operation like: db.operation.cancel_operation.starting, db.operation.cancel_operation.end.
If the event name grows there probably is a big overhead. The alternative is to move all the events into a isngle class file and starts from there. Then it means the event and tags can be mixed together, the tag can belong to any event and event can contain any tag. this kind of give us another level of freedom and avoid defining duplicate tags in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say think of this as the generated code such as thrift protocol. and just follow the json schema design, easy of use should be the main purpose.

unless we want to make the telemetry json schema more generic, but this will need bigger change and should be out of the scope of this work

Copy link
Contributor

Choose a reason for hiding this comment

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

Talked offline, there should not be too many event type name, let's just stick to this for now

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.

5 participants