feat(csharp): implement telemetry context models (StatementTelemetryContext, TelemetrySessionContext) by jadewang-db · Pull Request #301 · adbc-drivers/databricks

jadewang-db · 2026-03-05T16:04:09Z

🥞 Stacked PR

Use this link to review incremental changes.

stack/itr5-pr-telemetry-models [Files changed]
- stack/itr5-pr-telemetry-client-impl [Files changed]
  - stack/itr5-pr-e2e-tests [Files changed]

What's Changed

Please fill in a description of the changes here.

This contains breaking changes.

Closes #NNN.

Copilot

Pull request overview

Implements C# telemetry context models to support the V3 “direct object” telemetry approach, adds unit tests for the new contexts, and updates telemetry JSON formatting/exporter behavior plus related documentation.

Changes:

Added TelemetrySessionContext (per-connection) and StatementTelemetryContext (per-statement) models, with proto-building via BuildTelemetryLog().
Added unit tests covering construction/defaults and BuildTelemetryLog() output scenarios.
Updated proto JSON conversion to emit snake_case field names and hardened telemetry exporter endpoint URL building.

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
csharp/src/Telemetry/TelemetrySessionContext.cs	Introduces session-level telemetry context + placeholder `ITelemetryClient`.
csharp/src/Telemetry/StatementTelemetryContext.cs	Introduces statement-level telemetry context + proto log construction.
csharp/src/Telemetry/ProtoJsonConverter.cs	Switches proto JSON formatting to preserve proto field names (snake_case).
csharp/src/Telemetry/DatabricksTelemetryExporter.cs	Ensures host has an http/https scheme when building endpoint URL.
csharp/test/Unit/Telemetry/TelemetrySessionContextTests.cs	Adds unit tests for session context construction/defaults.
csharp/test/Unit/Telemetry/StatementTelemetryContextTests.cs	Adds unit tests for statement context lifecycle + proto building.
csharp/doc/telemetry-design.md	Updates design doc to V3 direct-object telemetry approach and related requirements.
csharp/doc/telemetry-sprint-plan-2026-03-05.md	Adds/updates sprint plan documentation for telemetry work.
csharp/doc/sprint-plan-telemetry-completion-2026-03-03.md	Adds/updates earlier sprint plan documentation.
csharp/CODING_GUIDELINES.md	Adds C# coding/testing guidelines.
.gitignore	Ignores `generated_task_specs.json`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

csharp/test/Unit/Telemetry/TelemetrySessionContextTests.cs

Copilot · 2026-03-06T00:54:16Z

csharp/doc/telemetry-design.md

+| Proto Field | Validation |
+|-------------|------------|
+| `error_name` | Non-empty, contains exception type name |
+| `stack_trace` | Non-empty, contains error message |


The DriverErrorInfo table says stack_trace should be non-empty and contain the error message, but the proto comment describes stack_trace as Databricks-owned stack info, and the current implementation intentionally sets it to empty for privacy. Please align this doc section with the actual schema/behavior (and consider documenting whether client error messages are intentionally excluded).

Suggested change

| `stack_trace` | Non-empty, contains error message |

| `stack_trace` | Optional; currently empty for privacy; reserved for Databricks-owned stack info (client error messages are not included) |

Copilot · 2026-03-06T00:54:17Z

csharp/CODING_GUIDELINES.md

+- Test files: `<ClassName>Test.cs`
+- Test class: `<ClassName>Test`
+- Test methods: Descriptive names explaining the scenario


The guideline here states test files/classes should be named <ClassName>Test, but the existing test suite uses multiple patterns (e.g., *Tests.cs, *UnitTests.cs). To avoid confusion, consider rewording this as a recommendation or updating it to match the predominant convention used in this repo.

Suggested change

- Test files: `<ClassName>Test.cs`

- Test class: `<ClassName>Test`

- Test methods: Descriptive names explaining the scenario

- Prefer naming test files after the class under test, for example: `<ClassName>Tests.cs` or `<ClassName>Test.cs`.

- Prefer naming test classes after the class under test, for example: `<ClassName>Tests` or `<ClassName>Test`.

- When adding tests to existing areas, follow the predominant naming convention in that project or directory (e.g., `*Tests.cs`, `*UnitTests.cs`) to keep things consistent.

- Test methods: Use descriptive names that clearly explain the scenario and expected behavior.

Copilot · 2026-03-06T00:54:17Z

csharp/src/Telemetry/TelemetrySessionContext.cs

+    /// <remarks>
+    /// This class holds frozen session-level data that does not change for the lifetime of a connection.
+    /// All properties have private setters to ensure immutability after construction.
+    /// Use the constructor or object initializer to set values.
+    /// </remarks>


The remarks say the session context is immutable with private setters / init-only semantics, but the properties in this type are declared with public setters. Either make the properties init-only/private-set (and adjust construction accordingly) or update the remarks to match the actual mutability.

Copilot · 2026-03-06T00:54:17Z

csharp/src/Telemetry/StatementTelemetryContext.cs

+            // Add chunk details if present
+            if (TotalChunksPresent.HasValue || TotalChunksIterated.HasValue)


ChunkDetails is only included when TotalChunksPresent or TotalChunksIterated is set, but other chunk-related fields (InitialChunkLatencyMs/SlowestChunkLatencyMs/SumChunksDownloadTimeMs) can be set independently via public setters and would be dropped from the emitted proto. Consider expanding the condition to include any chunk-detail field, or make chunk fields private-set and require SetChunkDetails() so the invariants are enforced.

Suggested change

// Add chunk details if present

if (TotalChunksPresent.HasValue || TotalChunksIterated.HasValue)

// Add chunk details if any chunk-related metric is present

if (TotalChunksPresent.HasValue

|| TotalChunksIterated.HasValue

|| InitialChunkLatencyMs.HasValue

|| SlowestChunkLatencyMs.HasValue

|| SumChunksDownloadTimeMs.HasValue)

Copilot · 2026-03-06T00:54:18Z

csharp/test/Unit/Telemetry/StatementTelemetryContextTests.cs

+            Thread.Sleep(10); // Small delay to ensure stopwatch is running
+
+            // Assert
+            Assert.NotNull(context.ExecuteStopwatch);
+            Assert.True(context.ExecuteStopwatch.IsRunning);
+            Assert.True(context.ExecuteStopwatch.ElapsedMilliseconds > 0);


These unit tests rely on Thread.Sleep with very small delays to assert Stopwatch timing, which can be flaky under CI load/clock granularity. Prefer a deterministic approach (e.g., assert ordering without requiring >0ms, use SpinWait with a bounded timeout, or abstract time/stopwatch so tests can control elapsed time).

Suggested change

Thread.Sleep(10); // Small delay to ensure stopwatch is running

// Assert

Assert.NotNull(context.ExecuteStopwatch);

Assert.True(context.ExecuteStopwatch.IsRunning);

Assert.True(context.ExecuteStopwatch.ElapsedMilliseconds > 0);

// Assert

Assert.NotNull(context.ExecuteStopwatch);

Assert.True(context.ExecuteStopwatch.IsRunning);

csharp/src/Telemetry/TelemetrySessionContext.cs

## 🥞 Stacked PR Use this [link](https://github.com/adbc-drivers/databricks/pull/286/files) to review incremental changes. - [**stack/add-design-plan**](#286) [[Files changed](https://github.com/adbc-drivers/databricks/pull/286/files)] - [stack/itr5-pr-telemetry-models](#301) [[Files changed](https://github.com/adbc-drivers/databricks/pull/301/files/5ad8bcbf56dd6d6d57d8138f33108f80c36eb42e..aa902284c764d97c256d41f2ab8da169b1068b52)] - [stack/itr5-pr-telemetry-client-impl](#305) [[Files changed](https://github.com/adbc-drivers/databricks/pull/305/files/aa902284c764d97c256d41f2ab8da169b1068b52..7ee3e3f38747c8538acccc305a77c9d9e4d96a0b)] - [stack/itr5-pr-e2e-tests](#308) [[Files changed](https://github.com/adbc-drivers/databricks/pull/308/files/7ee3e3f38747c8538acccc305a77c9d9e4d96a0b..06e6163ad49d2079216f7808234e0ffdd8300975)] --------- ## What's Changed Please fill in a description of the changes here. **This contains breaking changes.**  Closes #NNN. --------- Co-authored-by: Jade Wang <jade.wang+data@databricks.com>

…etric-model

…sk ID: task-1.2-statement-telemetry-context

GetEndpointUrl now ensures the host has a scheme prefix before constructing the telemetry endpoint URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Use JsonFormatter with PreserveProtoFieldNames(true) instead of JsonFormatter.Default to produce snake_case field names (session_id, system_configuration, etc.) matching the JDBC driver and proto schema. Default produces camelCase (sessionId) which is incorrect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This file was accidentally committed and should not be tracked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

samikshya-db

This has been verified to match with https://github.com/databricks-eng/universe/blob/master/proto/logs/frontend/oss-sql-driver-telemetry/sql_driver_telemetry.proto right?

jadewang-db · 2026-03-10T00:00:34Z

This has been verified to match with https://github.com/databricks-eng/universe/blob/master/proto/logs/frontend/oss-sql-driver-telemetry/sql_driver_telemetry.proto right?

yes

github-actions · 2026-03-10T00:04:26Z

🚀 Integration tests triggered! View workflow run

jadewang-db had a problem deploying to azure-prod March 5, 2026 16:04 — with GitHub Actions Error

jadewang-db had a problem deploying to azure-prod March 5, 2026 18:31 — with GitHub Actions Error

jadewang-db had a problem deploying to azure-prod March 5, 2026 22:51 — with GitHub Actions Error

jadewang-db had a problem deploying to azure-prod March 5, 2026 23:38 — with GitHub Actions Error

jadewang-db marked this pull request as ready for review March 5, 2026 23:39

jadewang-db changed the title ~~Implement TelemetryMetric data model\n\nTask ID: task-1.1-telemetry-metric-model~~ feat(csharp): Implement telemetry context models (StatementTelemetryContext, TelemetrySessionContext) Mar 5, 2026

jadewang-db changed the title ~~feat(csharp): Implement telemetry context models (StatementTelemetryContext, TelemetrySessionContext)~~ feat(csharp): implement telemetry context models (StatementTelemetryContext, TelemetrySessionContext) Mar 5, 2026

eric-wang-1990 requested a review from Copilot March 6, 2026 00:49

Copilot started reviewing on behalf of eric-wang-1990 March 6, 2026 00:49 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

eric-wang-1990 reviewed Mar 6, 2026

View reviewed changes

csharp/src/Telemetry/TelemetrySessionContext.cs Show resolved Hide resolved

jadewang-db requested a review from eric-wang-1990 March 6, 2026 17:58

Jade Wang and others added 7 commits March 6, 2026 23:46

Implement TelemetryMetric data model\n\nTask ID: task-1.1-telemetry-m…

c396c33

…etric-model

Implement StatementTelemetryContext and TelemetrySessionContext\n\nTa…

b3a7f2b

…sk ID: task-1.2-statement-telemetry-context

Fix exporter endpoint URL to prepend https:// when missing

9c81195

GetEndpointUrl now ensures the host has a scheme prefix before constructing the telemetry endpoint URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove generated_task_specs.json from repo

8e524b9

This file was accidentally committed and should not be tracked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add generated_task_specs.json to .gitignore

57b3968

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove unused TelemetryMetric model superseded by proto-generated code

7e6c6cd

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jadewang-db force-pushed the stack/itr5-pr-telemetry-models branch from aa90228 to 7e6c6cd Compare March 6, 2026 23:51

jadewang-db had a problem deploying to azure-prod March 6, 2026 23:51 — with GitHub Actions Error

samikshya-db approved these changes Mar 7, 2026

View reviewed changes

eric-wang-1990 approved these changes Mar 9, 2026

View reviewed changes

Change mode serialization

cfac5af

jadewang-db requested a deployment to azure-prod March 9, 2026 22:02 — with GitHub Actions Waiting

jadewang-db added this pull request to the merge queue Mar 9, 2026

jadewang-db added the integration-test Trigger integration tests in internal repo label Mar 10, 2026

Merged via the queue into main with commit ac92ee8 Mar 10, 2026
30 of 34 checks passed

jadewang-db deleted the stack/itr5-pr-telemetry-models branch March 10, 2026 00:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(csharp): implement telemetry context models (StatementTelemetryContext, TelemetrySessionContext)#301

feat(csharp): implement telemetry context models (StatementTelemetryContext, TelemetrySessionContext)#301
jadewang-db merged 8 commits intomainfrom
stack/itr5-pr-telemetry-models

jadewang-db commented Mar 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Uh oh!

samikshya-db left a comment

Uh oh!

jadewang-db commented Mar 10, 2026

Uh oh!

github-actions bot commented Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	\| `stack_trace` \| Non-empty, contains error message \|
	\| `stack_trace` \| Optional; currently empty for privacy; reserved for Databricks-owned stack info (client error messages are not included) \|

-- Test files: `<ClassName>Test.cs`
-- Test class: `<ClassName>Test`
-- Test methods: Descriptive names explaining the scenario
+- Prefer naming test files after the class under test, for example: `<ClassName>Tests.cs` or `<ClassName>Test.cs`.
+- Prefer naming test classes after the class under test, for example: `<ClassName>Tests` or `<ClassName>Test`.
+- When adding tests to existing areas, follow the predominant naming convention in that project or directory (e.g., `*Tests.cs`, `*UnitTests.cs`) to keep things consistent.
+- Test methods: Use descriptive names that clearly explain the scenario and expected behavior.

		// Add chunk details if present
		if (TotalChunksPresent.HasValue \|\| TotalChunksIterated.HasValue)

Conversation

jadewang-db commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🥞 Stacked PR

What's Changed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samikshya-db left a comment

Choose a reason for hiding this comment

Uh oh!

jadewang-db commented Mar 10, 2026

Uh oh!

github-actions bot commented Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jadewang-db commented Mar 5, 2026 •

edited

Loading