Skip to content

Extract TelemetryHelper for shared telemetry logic\n\nTask ID: task-2.1-extract-telemetry-helper#346

Draft
jadewang-db wants to merge 24 commits intomainfrom
stack/pr-phase2-sea-telemetry
Draft

Extract TelemetryHelper for shared telemetry logic\n\nTask ID: task-2.1-extract-telemetry-helper#346
jadewang-db wants to merge 24 commits intomainfrom
stack/pr-phase2-sea-telemetry

Conversation

@jadewang-db
Copy link
Collaborator

@jadewang-db jadewang-db commented Mar 13, 2026

🥞 Stacked PR

Use this link to review incremental changes.


What's Changed

Please fill in a description of the changes here.

This contains breaking changes.

Closes #NNN.

Jade Wang and others added 24 commits March 13, 2026 22:04
Comprehensive gap analysis of telemetry proto field coverage including:
- SEA connections have zero telemetry (highest priority)
- ChunkDetails.SetChunkDetails() defined but never called
- Missing fields: auth_type, WorkspaceId, runtime_vendor, client_app_name
- Composition via TelemetryHelper chosen over abstract base class
- E2E test strategy for all proto fields across both protocols

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ion\n\nTask ID: task-1.2-system-config-missing-fields
…Task ID: task-1.11-metadata-operation-telemetry
The demo directory was tracked as a git submodule in the index but had no
entry in .gitmodules, causing all CI jobs to fail at checkout.

Co-authored-by: Isaac
…remove scratch files

- Remove scratch/design docs not meant for PR (PHASE1_TEST_RESULTS.md,
  TELEMETRY_TIMING_ISSUE.md, fix-telemetry-gaps-design.md)
- Remove accidentally committed backup file (DatabricksConnection.cs.backup)
- Fix trailing whitespace in test and doc files
- Fix license header in ChunkMetrics.cs (was using "modified" header for new file)

Co-authored-by: Isaac
… resource cleanup

- Restore AsyncLocal for ExporterOverride to prevent parallel test interference
- Add missing IsInternalCall assertion in InternalCallTests
- Replace silent-pass with Skip.If when no telemetry captured in baseline tests
- Use await instead of .Result for async calls in MetadataOperationTests
- Add TimestampMillis to metadata operation telemetry Context
- Cache Process.GetCurrentProcess() call in BuildSystemConfiguration
- Move reader disposal to finally blocks in ChunkMetricsReaderTests

Co-authored-by: Isaac
…tch\n\nTask ID: task-2.4-sea-chunk-details
@jadewang-db jadewang-db force-pushed the stack/pr-phase2-sea-telemetry branch from adca408 to 5b65a42 Compare March 13, 2026 22:08
@jadewang-db
Copy link
Collaborator Author

Range-diff: stack/pr-phase1-thrift-telemetry-gaps (adca408 -> 5b65a42)
csharp/src/DatabricksConnection.cs
@@ -127,6 +127,7 @@
 -        private Telemetry.Proto.DriverSystemConfiguration BuildSystemConfiguration()
 -        {
 -            var osVersion = System.Environment.OSVersion;
+-            var processName = System.Diagnostics.Process.GetCurrentProcess().ProcessName;
 -            return new Telemetry.Proto.DriverSystemConfiguration
 -            {
 -                DriverVersion = s_assemblyVersion,
@@ -139,16 +140,16 @@
 -                RuntimeVendor = "Microsoft",
 -                LocaleName = System.Globalization.CultureInfo.CurrentCulture.Name,
 -                CharSetEncoding = System.Text.Encoding.Default.WebName,
--                ProcessName = System.Diagnostics.Process.GetCurrentProcess().ProcessName,
--                ClientAppName = GetClientAppName()
+-                ProcessName = processName,
+-                ClientAppName = GetClientAppName(processName)
 -            };
 -        }
 -
--        private string GetClientAppName()
+-        private string GetClientAppName(string processName)
 -        {
 -            // Check connection property first, fall back to process name
 -            Properties.TryGetValue("adbc.databricks.client_app_name", out string? appName);
--            return appName ?? Process.GetCurrentProcess().ProcessName;
+-            return appName ?? processName;
 -        }
 -
 -        private Telemetry.Proto.DriverConnectionParameters BuildDriverConnectionParams(bool isAuthenticated)
csharp/src/StatementExecution/StatementExecutionConnection.cs
@@ -183,5 +183,5 @@
 +        }
 +
          // TracingConnection provides IActivityTracer implementation
-         public override string AssemblyVersion => GetType().Assembly.GetName().Version?.ToString() ?? "1.0.0";
-         public override string AssemblyName => "AdbcDrivers.Databricks";
\ No newline at end of file
+         internal bool EnableComplexDatatypeSupport => _enableComplexDatatypeSupport;
+ 
\ No newline at end of file
csharp/src/StatementExecution/StatementExecutionStatement.cs
@@ -169,12 +169,18 @@
 +                            { "total_byte_count", response.Manifest.TotalByteCount }
 +                        }));
 +                }
-+
+ 
+-            // Create appropriate reader based on result disposition
+-            IArrowArrayStream reader = CreateReader(response, cancellationToken);
 +                // Create appropriate reader based on result disposition
 +                IArrowArrayStream reader = CreateReader(response, cancellationToken);
  
--            // Create appropriate reader based on result disposition
--            IArrowArrayStream reader = CreateReader(response, cancellationToken);
+-            // When EnableComplexDatatypeSupport=false (default), serialize complex Arrow types to JSON strings
+-            // so that SEA behavior matches Thrift (which sets ComplexTypesAsArrow=false).
+-            if (!_enableComplexDatatypeSupport)
+-            {
+-                reader = new ComplexTypeSerializingStream(reader);
+-            }
 +                // Get schema from reader
 +                var schema = reader.Schema;
  
@@ -184,8 +190,8 @@
 +                long rowCount = response.Manifest?.TotalRowCount ?? -1;
 +                QueryResult result = new QueryResult(rowCount, reader);
  
--            // Return query result - use -1 if row count is not available
--            long rowCount = response.Manifest?.TotalRowCount ?? -1;
+-            // Return query result - use 0 if row count is not available
+-            long rowCount = response.Manifest?.TotalRowCount ?? 0;
 -            return new QueryResult(rowCount, reader);
 +                // Record success and determine result format from disposition
 +                if (ctx != null)
@@ -342,7 +348,7 @@
 +                throw;
              }
 -
--            // For updates, we don't need to read the results - just return the row count
+-            // For updates, we don't need to read the results - just return the row count.
 -            long rowCount = response.Manifest?.TotalRowCount ?? 0;
 -            return new UpdateResult(rowCount);
          }

Reproduce locally: git range-diff 065621b..adca408 d504b0a..5b65a42 | Disable: git config gitstack.push-range-diff false

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.

1 participant