Skip to content

[4/7] Telemetry Event Emission and Aggregation #327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
samikshya-db wants to merge 9 commits into
base: telemetry-3-client-management
Choose a base branch
from
Open

[4/7] Telemetry Event Emission and Aggregation #327

samikshya-db wants to merge 9 commits into from
+4,905 −0

Conversation

@samikshya-db
Copy link
Contributor

@samikshya-db samikshya-db commented Jan 28, 2026

Part 4 of 7-part Telemetry Implementation Stack

This layer adds event emission and per-statement aggregation with smart batching.

Summary

Implements TelemetryEventEmitter for event-driven telemetry and MetricsAggregator for efficient per-statement aggregation with smart flushing.

Components

TelemetryEventEmitter (lib/telemetry/TelemetryEventEmitter.ts)

Event-driven architecture using Node.js EventEmitter:

  • Type-safe emission methods for each event type
  • Respects telemetryEnabled configuration flag
  • All exceptions swallowed and logged at debug level only
  • Zero performance impact when disabled (early return)

Event Types:

  • connection.open - Successful connection establishment
  • statement.start - Statement execution begins
  • statement.complete - Statement execution completes
  • cloudfetch.chunk - CloudFetch chunk downloaded
  • error - Exception occurred with terminal classification

MetricsAggregator (lib/telemetry/MetricsAggregator.ts)

Per-statement aggregation with smart batching:

Aggregation Strategy:

  • Connection events → emit immediately (no aggregation)
  • Statement events → buffer until completeStatement() called
  • Terminal errors → flush immediately (critical failures)
  • Retryable errors → buffer until statement complete (optimize batching)

Flush Triggers:

  • Batch size reached (default: 100 metrics)
  • Periodic timer fired (default: 5000ms)
  • Terminal exception occurred (immediate flush)
  • Manual flush() called

Memory Management:

  • Bounded buffers prevent memory leaks
  • Completed statements removed from memory
  • Periodic timer cleanup

Smart Batching Benefits

  • Reduces HTTP overhead: Fewer export calls
  • Optimizes bandwidth: Batch multiple metrics
  • Critical error priority: Terminal errors flushed immediately
  • Efficient aggregation: Per-statement grouping reduces data size

Testing

  • 31 unit tests for TelemetryEventEmitter (100% function coverage)
  • 32 unit tests for MetricsAggregator (94% line, 82% branch coverage)
  • Tests verify exception swallowing (CRITICAL requirement)
  • Tests verify debug-only logging (CRITICAL requirement)
  • Tests verify batch size and timer triggers
  • Tests verify terminal vs retryable error handling

Next Steps

This PR is followed by:

  • [5/7] Export: DatabricksTelemetryExporter
  • [6/7] Integration: Wire into DBSQLClient
  • [7/7] Testing & Documentation

Dependencies

Depends on:

This was referenced Jan 28, 2026
Open
Open
Open
@samikshya-db samikshya-db marked this pull request as draft January 28, 2026 22:41
@samikshya-db samikshya-db marked this pull request as ready for review January 29, 2026 08:22
@samikshya-db
Copy link
Contributor Author

samikshya-db commented Jan 29, 2026

The emission format confirms to the telemetry proto, marked this ready for review.

@samikshya-db samikshya-db force-pushed the telemetry-3-client-management branch from 87d1e85 to 32003e9 Compare January 29, 2026 20:21
samikshya-db and others added 7 commits January 29, 2026 20:21
@samikshya-db
Add telemetry infrastructure: CircuitBreaker and FeatureFlagCache
dfab781 
This is part 2 of 7 in the telemetry implementation stack.

Components:
- CircuitBreaker: Per-host endpoint protection with state management
- FeatureFlagCache: Per-host feature flag caching with reference counting
- CircuitBreakerRegistry: Manages circuit breakers per host

Circuit Breaker:
- States: CLOSED (normal), OPEN (failing), HALF_OPEN (testing recovery)
- Default: 5 failures trigger OPEN, 60s timeout, 2 successes to CLOSE
- Per-host isolation prevents cascade failures
- All state transitions logged at debug level

Feature Flag Cache:
- Per-host caching with 15-minute TTL
- Reference counting for connection lifecycle management
- Automatic cache expiration and refetch
- Context removed when refCount reaches zero

Testing:
- 32 comprehensive unit tests for CircuitBreaker
- 29 comprehensive unit tests for FeatureFlagCache
- 100% function coverage, >80% line/branch coverage
- CircuitBreakerStub for testing other components

Dependencies:
- Builds on [1/7] Types and Exception Classifier
@samikshya-db
Add telemetry client management: TelemetryClient and Provider
9f7d84c 
This is part 3 of 7 in the telemetry implementation stack.

Components:
- TelemetryClient: HTTP client for telemetry export per host
- TelemetryClientProvider: Manages per-host client lifecycle with reference counting

TelemetryClient:
- Placeholder HTTP client for telemetry export
- Per-host isolation for connection pooling
- Lifecycle management (open/close)
- Ready for future HTTP implementation

TelemetryClientProvider:
- Reference counting tracks connections per host
- Automatically creates clients on first connection
- Closes and removes clients when refCount reaches zero
- Thread-safe per-host management

Design Pattern:
- Follows JDBC driver pattern for resource management
- One client per host, shared across connections
- Efficient resource utilization
- Clean lifecycle management

Testing:
- 31 comprehensive unit tests for TelemetryClient
- 31 comprehensive unit tests for TelemetryClientProvider
- 100% function coverage, >80% line/branch coverage
- Tests verify reference counting and lifecycle

Dependencies:
- Builds on [1/7] Types and [2/7] Infrastructure
@samikshya-db
Add telemetry event emission and aggregation
03b8f77 
This is part 4 of 7 in the telemetry implementation stack.

Components:
- TelemetryEventEmitter: Event-based telemetry emission using Node.js EventEmitter
- MetricsAggregator: Per-statement aggregation with batch processing

TelemetryEventEmitter:
- Event-driven architecture using Node.js EventEmitter
- Type-safe event emission methods
- Respects telemetryEnabled configuration flag
- All exceptions swallowed and logged at debug level
- Zero impact when disabled

Event Types:
- connection.open: On successful connection
- statement.start: On statement execution
- statement.complete: On statement finish
- cloudfetch.chunk: On chunk download
- error: On exception with terminal classification

MetricsAggregator:
- Per-statement aggregation by statement_id
- Connection events emitted immediately (no aggregation)
- Statement events buffered until completeStatement() called
- Terminal exceptions flushed immediately
- Retryable exceptions buffered until statement complete
- Batch size (default 100) triggers flush
- Periodic timer (default 5s) triggers flush

Batching Strategy:
- Optimizes export efficiency
- Reduces HTTP overhead
- Smart flushing based on error criticality
- Memory efficient with bounded buffers

Testing:
- 31 comprehensive unit tests for TelemetryEventEmitter
- 32 comprehensive unit tests for MetricsAggregator
- 100% function coverage, >90% line/branch coverage
- Tests verify exception swallowing
- Tests verify debug-only logging

Dependencies:
- Builds on [1/7] Types, [2/7] Infrastructure, [3/7] Client Management
@samikshya-db @claude
Add authentication support for REST API calls
a00c482 
Implements getAuthHeaders() method for authenticated REST API requests:
- Added getAuthHeaders() to IClientContext interface
- Implemented in DBSQLClient using authProvider.authenticate()
- Updated FeatureFlagCache to fetch from connector-service API with auth
- Added driver version support for version-specific feature flags
- Replaced placeholder implementation with actual REST API calls

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@samikshya-db @claude
Fix feature flag and telemetry export endpoints
33fcc22 
- Change feature flag endpoint to use NODEJS client type
- Fix telemetry endpoints to /telemetry-ext and /telemetry-unauth
- Update payload to match proto with system_configuration
- Add shared buildUrl utility for protocol handling

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@samikshya-db
Match JDBC telemetry payload format
1b49399 
- Change payload structure to match JDBC: uploadTime, items, protoLogs
- protoLogs contains JSON-stringified TelemetryFrontendLog objects
- Remove workspace_id (JDBC doesn't populate it)
- Remove debug logs added during testing
@samikshya-db
Fix lint errors
29376a6 
- Fix import order in FeatureFlagCache
- Replace require() with import for driverVersion
- Fix variable shadowing
- Disable prefer-default-export for urlUtils
@samikshya-db samikshya-db force-pushed the telemetry-4-event-aggregation branch from 31f847e to 29376a6 Compare January 29, 2026 20:21
@samikshya-db
Add missing getAuthHeaders method to ClientContextStub
4effbc5 
Fix TypeScript compilation error by implementing getAuthHeaders
method required by IClientContext interface.
@samikshya-db samikshya-db force-pushed the telemetry-4-event-aggregation branch from e5a0d5c to 4effbc5 Compare January 29, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jadewang-db jadewang-db Awaiting requested review from jadewang-db

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@samikshya-db