Skip to content

Commit 012ff1b

Browse files
committed
Made changes
1 parent 9db3de1 commit 012ff1b

2 files changed

Lines changed: 42 additions & 8 deletions

File tree

β€Žspec/telemetry-design.mdβ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ This document outlines an **event-based telemetry design** for the Databricks No
3737
**Production Requirements** (from JDBC driver experience):
3838
- **Feature flag caching**: Per-host caching to avoid rate limiting
3939
- **Circuit breaker**: Protect against telemetry endpoint failures
40-
- **Exception swallowing**: All telemetry exceptions caught with minimal logging
40+
- **🚨 Exception swallowing**: ALL telemetry exceptions caught and logged at LogLevel.debug ONLY (never warn/error)
4141
- **Per-host telemetry client**: One client per host to prevent rate limiting
4242
- **Graceful shutdown**: Proper cleanup with reference counting
4343
- **Smart exception flushing**: Only flush terminal exceptions immediately

β€Žspec/telemetry-sprint-plan.mdβ€Ž

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,18 @@ This sprint plan outlines the implementation of event-based telemetry for the Da
254254
- βœ… Reads config from context
255255

256256
**Acceptance Criteria**:
257-
- All emit methods wrap in try-catch
258-
- Exceptions logged via IDBSQLLogger at debug level
259-
- No exceptions propagate to caller
257+
- **🚨 CRITICAL**: All emit methods wrap in try-catch
258+
- **🚨 CRITICAL**: ALL exceptions logged at LogLevel.debug ONLY (never warn/error)
259+
- **🚨 CRITICAL**: NO exceptions propagate to caller (100% swallowed)
260+
- **🚨 CRITICAL**: NO console.log/debug/error calls (only IDBSQLLogger)
260261
- Events not emitted when disabled
261262
- Uses context for logger and config
262263

264+
**Testing Must Verify**:
265+
- [ ] Throw exception inside emit method β†’ verify swallowed
266+
- [ ] Verify logged at debug level (not warn/error)
267+
- [ ] Verify no exception reaches caller
268+
263269
**Unit Tests**:
264270
- `should emit connection.open event`
265271
- `should emit statement lifecycle events`
@@ -305,7 +311,14 @@ This sprint plan outlines the implementation of event-based telemetry for the Da
305311
- Retryable exceptions buffered
306312
- Batch size from config triggers flush
307313
- Periodic timer from config triggers flush
308-
- All logging via IDBSQLLogger
314+
- **🚨 CRITICAL**: All logging via IDBSQLLogger at LogLevel.debug ONLY
315+
- **🚨 CRITICAL**: All exceptions swallowed (never propagate)
316+
- **🚨 CRITICAL**: NO console logging
317+
318+
**Testing Must Verify**:
319+
- [ ] Exception in processEvent() β†’ verify swallowed
320+
- [ ] Exception in flush() β†’ verify swallowed
321+
- [ ] All errors logged at debug level only
309322

310323
**Unit Tests**:
311324
- `should aggregate events by statement_id`
@@ -352,9 +365,17 @@ This sprint plan outlines the implementation of event-based telemetry for the Da
352365
- Properly formats payload with workspace_id, session_id, statement_id
353366
- Retries on retryable errors (max from config)
354367
- Circuit breaker protects endpoint
355-
- All exceptions swallowed and logged via IDBSQLLogger
368+
- **🚨 CRITICAL**: All exceptions swallowed and logged at LogLevel.debug ONLY
369+
- **🚨 CRITICAL**: NO exceptions propagate (export never throws)
370+
- **🚨 CRITICAL**: NO console logging
356371
- Uses connection provider for HTTP calls
357372

373+
**Testing Must Verify**:
374+
- [ ] Network failure β†’ verify swallowed and logged at debug
375+
- [ ] Circuit breaker OPEN β†’ verify swallowed
376+
- [ ] Invalid response β†’ verify swallowed
377+
- [ ] No exceptions reach caller under any scenario
378+
358379
**Unit Tests**:
359380
- `should export metrics to correct endpoint`
360381
- `should format payload correctly`
@@ -398,10 +419,18 @@ This sprint plan outlines the implementation of event-based telemetry for the Da
398419
- Feature flag checked via FeatureFlagCache instance
399420
- TelemetryClientProvider used for per-host clients
400421
- Reference counting works correctly
401-
- All telemetry errors swallowed and logged via logger
422+
- **🚨 CRITICAL**: All telemetry errors swallowed and logged at LogLevel.debug ONLY
423+
- **🚨 CRITICAL**: Driver NEVER throws exceptions due to telemetry
424+
- **🚨 CRITICAL**: NO console logging in any telemetry code
402425
- Does not impact driver performance or stability
403426
- Follows existing driver patterns
404427

428+
**Testing Must Verify**:
429+
- [ ] Telemetry initialization fails β†’ driver continues normally
430+
- [ ] Feature flag fetch fails β†’ driver continues normally
431+
- [ ] All errors logged at debug level (never warn/error/info)
432+
- [ ] No exceptions propagate to application code
433+
405434
**Integration Tests**:
406435
- `should initialize telemetry on connect`
407436
- `should respect feature flag`
@@ -647,14 +676,19 @@ A task is considered complete when:
647676
- βœ… Code reviewed and approved
648677
- βœ… Documentation updated
649678
- βœ… No regressions in existing tests
650-
- βœ… Exception handling verified (all exceptions swallowed)
679+
- βœ… **🚨 CRITICAL**: Exception handling verified (ALL exceptions swallowed, NONE propagate)
680+
- βœ… **🚨 CRITICAL**: Logging verified (ONLY LogLevel.debug used, NO console logging)
681+
- βœ… **🚨 CRITICAL**: Error injection tested (telemetry failures don't impact driver)
651682

652683
The sprint is considered complete when:
653684
- βœ… All tasks marked as complete
654685
- βœ… All tests passing
655686
- βœ… Code merged to main branch
656687
- βœ… Documentation published
657688
- βœ… Demo prepared for stakeholders
689+
- βœ… **🚨 CRITICAL**: Code review confirms NO exceptions can escape telemetry code
690+
- βœ… **🚨 CRITICAL**: Code review confirms NO console logging exists
691+
- βœ… **🚨 CRITICAL**: Integration tests prove driver works even when telemetry completely fails
658692

659693
---
660694

0 commit comments

Comments
Β (0)