feat(csharp/src/Drivers/Databricks): Implement telemetry configuration and exporter (phase 2.0)#3664
Conversation
…n and exporter (phase 2.0)
| _hostUrl = !string.IsNullOrEmpty(hostUrl) ? hostUrl : throw new ArgumentException("Host URL cannot be null or empty.", nameof(hostUrl)); | ||
| } | ||
|
|
||
| public async Task ExportAsync( |
There was a problem hiding this comment.
Does this plug into existing Telemetry exporter framework? Does defining this single method work out of the box?
Take a look at FileExporter for example.
There was a problem hiding this comment.
The single-method interface is sufficient because batching and aggregation happen in MetricsAggregator before reaching the exporter.
FileExporter (OpenTelemetry-based):
Activity → OpenTelemetry SDK → BaseExporter → Local File
DatabricksTelemetryExporter (Custom aggregation):
Activity → DatabricksActivityListener → MetricsAggregator → IDatabricksTelemetryExporter → Databricks Service
Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
|
It looks like this overlaps substantially with "phase 1" (#3653) so my comments on this PR will be limited to the four files that are new to it. EDIT: or is that two files? I'm seeing two different things in two different tools :(. |
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks! I have more questions than requests for specific changes. Presumably phase 1 needs to be checked in before this?
| /// Interface for exporting telemetry metrics to Databricks telemetry service. | ||
| /// Implementations must never throw exceptions. | ||
| /// </summary> | ||
| public interface IDatabricksTelemetryExporter |
There was a problem hiding this comment.
| public interface IDatabricksTelemetryExporter | |
| internal interface IDatabricksTelemetryExporter |
| } | ||
| catch (Exception ex) | ||
| { | ||
| Debug.WriteLine($"Telemetry export failed: {ex.Message}"); |
There was a problem hiding this comment.
Should this eventually hook into tracing instead?
| { | ||
| await SendMetricsAsync(metrics, cancellationToken); | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
Should there be a retry mechanism for these?
| string fullUrl = new UriBuilder(_hostUrl) { Path = TelemetryEndpoint }.ToString(); | ||
| string json = SerializeMetrics(metrics); | ||
| var content = new StringContent(json, Encoding.UTF8, "application/json"); | ||
| var request = new HttpRequestMessage(HttpMethod.Post, fullUrl) { Content = content }; |
There was a problem hiding this comment.
This is an unauthenticated endpoint?
5 participants
This PR implements phase 2 (telemetry-activity-based-design.md) of the Activity-based telemetry system for the Databricks ADBC C# driver. It builds upon Phase 1 by adding the foundational infrastructure for collecting and exporting telemetry metrics.
Changes
New Files Added
1. TelemetryConfiguration.cs
Configuration settings for Databricks telemetry collection and export.
Key properties:
Enabled: Enable/disable telemetryBatchSize: Number of metrics to batch before sendingFlushIntervalMs: Interval for periodic flushMaxRetries,RetryDelayMs: Retry configurationCircuitBreakerEnabled,CircuitBreakerThreshold,CircuitBreakerTimeout: Circuit breaker settings2. TelemetryMetric.cs
Data model representing a single telemetry event derived from Activity data.
Properties:
EventType: Type of telemetry event (ConnectionOpen, StatementExecution, Error)Timestamp: When the event occurredTags: Flexible dictionary for event-specific metadata3. IDatabricksTelemetryExporter.cs
Interface for exporting telemetry metrics to external services.
Contract:
ExportAsync(IReadOnlyList<TelemetryMetric> metrics, CancellationToken cancellationToken)4. DatabricksTelemetryExporter.cs
HTTP-based exporter implementation that sends metrics to Databricks telemetry service.
Features:
/telemetry-extendpointUriBuilderfor proper URL constructionDesign Decisions
What's Left in Phase 2
Two components remain for separate review (because of their complex implementation, will raise a separate PR for better review process):
TelemetryMetric