Skip to content

Commit 5cea574

Browse files
MSNevMSNev
authored
docs: Implementation Plan, Summary and review (#2708)
1 parent dc7735a commit 5cea574

2 files changed

Lines changed: 309 additions & 0 deletions

File tree

‎shared/otel-core/planning/IMPLEMENTATION_PLAN.md‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
# OTel Web SDK Implementation Plan
2+
3+
**Last Updated**: February 2026
4+
**Reference**: [CONTEXT.md](../CONTEXT.md)
5+
6+
---
7+
8+
## Current Status
9+
10+
| Category | Status | Notes |
11+
|----------|--------|-------|
12+
| Tracing Core (TracerProvider, Tracer, Span) | ⚠️ Needs Updates | Closure pattern used but missing CONTEXT.md compliance |
13+
| Logging Core (LoggerProvider, Logger, LogRecord) | ⚠️ Needs Updates | Closure pattern used but missing CONTEXT.md compliance |
14+
| Context Management (ContextManager, Context) | ⚠️ Needs Updates | Closure pattern used but missing config pattern |
15+
| Resource/Attributes | ⚠️ Needs Updates | Closure pattern used but missing shutdown cleanup |
16+
| Error Handling (`handleErrors.ts`) | ✅ Complete | Helper functions |
17+
| SDK Entry Point | ❌ Not Started | `OTelSdk` class has commented-out code |
18+
| Samplers | ❌ Interface only | No implementations |
19+
| SpanProcessor | ❌ Interface commented out | No implementations |
20+
| Metrics | ❌ Interfaces only | No implementations |
21+
| Propagation | ❌ Interface commented out | No implementations |
22+
23+
---
24+
25+
## CONTEXT.md Compliance Issues in "Complete" Components
26+
27+
### TracerProvider (`_createTracerProvider`)
28+
29+
| Issue | CONTEXT.md Requirement | Current State |
30+
|-------|------------------------|---------------|
31+
| Naming | Public factories use `create*` | Uses `_create*` (internal naming) |
32+
| Config | Takes config object with deps | Takes `host: ITraceHost` |
33+
| Error handlers | Get from `config.errorHandlers` | No error handling |
34+
| Dynamic config | Use `onConfigChange`, save `IUnloadHook` | Not implemented |
35+
| TypeDoc | Required on all public APIs | Missing |
36+
37+
### LoggerProvider (`createLoggerProvider`)
38+
39+
| Issue | CONTEXT.md Requirement | Current State |
40+
|-------|------------------------|---------------|
41+
| Required deps | Validate upfront, no defaults | Uses `config = {}` default |
42+
| Error handlers | Get from `config.errorHandlers` | Creates empty `handlers = {}` |
43+
| Dynamic config | Use `onConfigChange`, save `IUnloadHook` | Reads config at creation only |
44+
| Shutdown | Call `_configUnload.rm()` | No config listener cleanup |
45+
46+
### ContextManager (`createContextManager`)
47+
48+
| Issue | CONTEXT.md Requirement | Current State |
49+
|-------|------------------------|---------------|
50+
| Config | Takes config object with deps | Takes `parentContext?` only |
51+
| Error handlers | Get from `config.errorHandlers` | No error handling |
52+
| TypeDoc | Required on factory functions | Minimal documentation |
53+
54+
### Resource (`createResource`)
55+
56+
| Issue | CONTEXT.md Requirement | Current State |
57+
|-------|------------------------|---------------|
58+
| Error handlers | Uses config handlers | ✅ Uses `resourceCtx.cfg.errorHandlers` |
59+
| Shutdown | Cleanup method | No explicit shutdown/cleanup |
60+
61+
### Span (`createSpan`)
62+
63+
| Issue | CONTEXT.md Requirement | Current State |
64+
|-------|------------------------|---------------|
65+
| Error handlers | Uses config handlers | ✅ Uses handlers from config |
66+
| Closure pattern | Private state in closures | ✅ Correct |
67+
| Dynamic config | Use `onConfigChange` if caching config | Reads `otelCfg` at creation |
68+
69+
### Common Missing Patterns
70+
71+
1. **No `onConfigChange` usage** - None of the implementations use `onConfigChange` for dynamic config
72+
2. **No `IUnloadHook` management** - No saving of hooks or calling `.rm()` during shutdown
73+
3. **Inconsistent factory signatures** - Some take config objects, some take host/context directly
74+
4. **Missing dependency validation** - Required deps not validated upfront per CONTEXT.md
75+
76+
---
77+
78+
## Implementation Phases
79+
80+
### Phase 0: CONTEXT.md Compliance Fixes (Prerequisite)
81+
82+
Fix existing implementations to comply with CONTEXT.md before building new features.
83+
84+
| Component | Changes Required |
85+
|-----------|-----------------|
86+
| `_createTracerProvider` | Rename to `createTracerProvider`, add config object, add error handlers, add `onConfigChange` |
87+
| `createLoggerProvider` | Remove default config, validate required deps, get handlers from config, add `onConfigChange` |
88+
| `createContextManager` | Add config object with error handlers |
89+
| `createSpan` | Add `onConfigChange` if caching config values |
90+
| `createResource` | Add shutdown/cleanup method |
91+
| All | Add `IUnloadHook` management with `.rm()` calls during shutdown |
92+
| All | Add comprehensive TypeDoc documentation |
93+
94+
**Deliverable**: All existing implementations pass CONTEXT.md validation checklist.
95+
96+
### Phase 1: SDK Foundation (Critical)
97+
98+
| Component | Location | Description |
99+
|-----------|----------|-------------|
100+
| `createOTelWebSdk()` | `src/otel/sdk/` | Main SDK entry point factory |
101+
| `IOTelWebSdkConfig` | `src/interfaces/otel/config/` | SDK configuration interface |
102+
| Deprecate `OTelSdk` class | `src/otel/sdk/OTelSdk.ts` | Remove DynamicProto usage |
103+
104+
**Deliverable**: Functional SDK that can be instantiated with trace+log providers.
105+
106+
### Phase 2: Trace Processing Pipeline
107+
108+
| Component | Location | Description |
109+
|-----------|----------|-------------|
110+
| `IOTelSpanProcessor` | `src/interfaces/otel/trace/` | Uncomment/finalize interface |
111+
| `ISpanExporter` | `src/interfaces/otel/trace/` | Export contract |
112+
| `createSimpleSpanProcessor()` | `src/otel/sdk/` | Immediate export on span end |
113+
| `createBatchSpanProcessor()` | `src/otel/sdk/` | Batched export with configurable delays |
114+
115+
**Deliverable**: Spans can be processed and exported.
116+
117+
### Phase 3: Samplers
118+
119+
| Component | Location | Description |
120+
|-----------|----------|-------------|
121+
| `createAlwaysOnSampler()` | `src/otel/sdk/` | Sample all spans |
122+
| `createAlwaysOffSampler()` | `src/otel/sdk/` | Sample no spans |
123+
| `createTraceIdRatioBasedSampler()` | `src/otel/sdk/` | Ratio-based sampling |
124+
| `createParentBasedSampler()` | `src/otel/sdk/` | Inherit parent decision |
125+
126+
**Deliverable**: Configurable sampling strategies.
127+
128+
### Phase 4: Propagation
129+
130+
| Component | Location | Description |
131+
|-----------|----------|-------------|
132+
| `IOTelPropagationApi` | `src/interfaces/otel/propagation/` | Uncomment/finalize interface |
133+
| `createW3CTraceContextPropagator()` | `src/otel/api/propagation/` | W3C TraceContext |
134+
| `createW3CBaggagePropagator()` | `src/otel/api/propagation/` | W3C Baggage |
135+
| `createCompositePropagator()` | `src/otel/api/propagation/` | Combine propagators |
136+
137+
**Deliverable**: Distributed tracing context propagation.
138+
139+
### Phase 5: Basic Metrics
140+
141+
| Component | Location | Description |
142+
|-----------|----------|-------------|
143+
| `createMeterProvider()` | `src/otel/sdk/` | MeterProvider factory |
144+
| `createMeter()` | `src/otel/sdk/` | Meter factory |
145+
| `createCounter()` | `src/otel/sdk/` | Monotonic counter |
146+
| `createHistogram()` | `src/otel/sdk/` | Value distribution |
147+
| `createGauge()` | `src/otel/sdk/` | Current value |
148+
149+
**Deliverable**: Basic metrics (counter, histogram, gauge).
150+
151+
### Phase 6: Cleanup & Polish
152+
153+
| Component | Location | Description |
154+
|-----------|----------|-------------|
155+
| `createIdGenerator()` | `src/otel/api/trace/` | Factory wrapper for ID utilities |
156+
| `createBaggage()` | `src/otel/api/baggage/` | Baggage creation |
157+
| Config isolation | `src/otel/sdk/config.ts` | Fix `process.env` access |
158+
| Pattern migration | Various | Remove remaining DynamicProto usages |
159+
160+
---
161+
162+
## Pattern Compliance Issues
163+
164+
### Files Requiring Migration (DynamicProto → Closure)
165+
166+
| File | Priority | Action |
167+
|------|----------|--------|
168+
| `OTelSdk.ts` | High | Replace with `createOTelWebSdk()` factory |
169+
| `DiagnosticLogger.ts` | Medium | Assess if migration needed for OTel use |
170+
| `AppInsightsCore.ts` | Low | AI core - may not need migration |
171+
172+
### Files Using Closure Pattern (Need CONTEXT.md Compliance Updates)
173+
174+
| File | Factory | Compliance Issues |
175+
|------|---------|-------------------|
176+
| `tracerProvider.ts` | `_createTracerProvider` | Internal naming, no config object, no `onConfigChange` |
177+
| `tracer.ts` | `_createTracer` | Internal naming, takes host not config |
178+
| `span.ts` | `createSpan` | No `onConfigChange` for cached config |
179+
| `contextManager.ts` | `createContextManager` | No config object, no error handlers |
180+
| `OTelLoggerProvider.ts` | `createLoggerProvider` | Default config, no `onConfigChange`, creates own handlers |
181+
| `OTelLogger.ts` | `createLogger` | Needs review for config compliance |
182+
| `OTelLogRecord.ts` | `createLogRecord` | Needs review for config compliance |
183+
| `resource.ts` | `createResource` | No shutdown method |
184+
| `attributeContainer.ts` | `createAttributeContainer` | Needs review for config compliance |
185+
186+
---
187+
188+
## Test Coverage Gaps
189+
190+
| Area | Status |
191+
|------|--------|
192+
| Trace (Span, Tracer, TraceState) | ✅ Extensive |
193+
| Logging (Logger, LogRecord, Processor) | ✅ Good |
194+
| Attributes | ✅ Yes |
195+
| Error Handling | ✅ Yes |
196+
| SDK Entry Point | ❌ None |
197+
| Samplers | ❌ None |
198+
| SpanProcessor | ❌ None |
199+
| Metrics | ❌ None |
200+
| Propagation | ❌ None |
201+
202+
---
203+
204+
## Priority Order
205+
206+
0. **Phase 0** - CONTEXT.md Compliance Fixes (prerequisite for all other work)
207+
1. **Phase 1** - SDK Entry Point (unblocks SDK usage)
208+
2. **Phase 2** - SpanProcessor (enables trace export)
209+
3. **Phase 3** - Samplers (enables sampling control)
210+
4. **Phase 4** - Propagation (enables distributed tracing)
211+
5. **Phase 5** - Metrics (completes OTel API coverage)
212+
6. **Phase 6** - Polish and cleanup
213+
214+
---
215+
216+
## Validation Criteria
217+
218+
Per [CONTEXT.md](../CONTEXT.md), all implementations must:
219+
220+
- [ ] Use closure/factory pattern (`create*` functions)
221+
- [ ] Return interface types only
222+
- [ ] Inject all dependencies through config
223+
- [ ] Use `IOTelErrorHandlers` for error handling
224+
- [ ] Use config directly (NO spread operator)
225+
- [ ] Use `onConfigChange` for dynamic config (save `IUnloadHook`, call `.rm()` on shutdown)
226+
- [ ] Have comprehensive TypeDoc documentation
227+
- [ ] Have unit tests with cleanup validation

‎shared/otel-core/planning/SUMMARY.md‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# OTel Web SDK - Summary
2+
3+
Concise summary of implementation requirements. Full details in [CONTEXT.md](../CONTEXT.md).
4+
5+
---
6+
7+
## Architectural Principles
8+
9+
**Interface-First Design** — Define all public APIs as TypeScript interfaces before implementation. Factory functions return interface types only, never expose implementation details.
10+
11+
**Inversion of Control (IoC)** — All dependencies explicitly injected through factory config. No hidden globals, no singletons, no static state. Components never reach out to get dependencies — they receive everything they need.
12+
13+
**Multi-Instance Architecture** — Multiple SDK instances coexist without interference. No shared global state between instances.
14+
15+
**No OpenTelemetry Package Imports** — Never import `@opentelemetry/*` packages directly (they have side effects that register globals). Define compatible `IOTel` interfaces instead.
16+
17+
**Complete Unload Support** — Every SDK instance must fully clean up on unload: remove hooks, clear timers, flush pending telemetry, release all resources.
18+
19+
---
20+
21+
## Core Rules
22+
23+
**No Global State** — Never use window properties, static state, or singletons.
24+
25+
**No OpenTelemetry Imports** — Define compatible interfaces with `IOTel` prefix instead.
26+
27+
**Factory Functions Only** — Use `create*` naming, return interface types, inject all dependencies through config.
28+
29+
**Closure Pattern** — Private state in closure variables, public methods on `_self` object, no classes.
30+
31+
**Config Used Directly** — Never copy config with spread operator. Use `onConfigChange` for local caching, save the `IUnloadHook`, call `.rm()` on shutdown.
32+
33+
**Error Handling** — Get handlers from `config.errorHandlers`, use helpers from `handleErrors.ts`.
34+
35+
**No No-Op Code in SDK** — The main SDK must not contain no-op/fallback implementations. A separate standalone no-op package provides API-compatible stubs for unsupported browsers. The SDK Loader decides whether to load the full SDK or the no-op package based on browser capability detection.
36+
37+
---
38+
39+
## Naming
40+
41+
| Type | Convention | Example |
42+
|------|------------|---------|
43+
| Public interfaces | `I` prefix | `IUnloadResult` |
44+
| OTel interfaces | `IOTel` prefix | `IOTelTraceProvider` |
45+
| Internal interfaces | `_I` prefix + `@internal` | `_ISpanProcessor` |
46+
| Const enums | `e` prefix | `eSpanKind` |
47+
| Public enums | use `createEnumStyle` | `SpanKind` |
48+
| Factory functions | `create*` | `createTraceProvider` |
49+
50+
---
51+
52+
## Anti-Patterns
53+
54+
```typescript
55+
window.__OTEL_SDK__; // ❌ Global access
56+
let _config = { ...config }; // ❌ Spread operator
57+
import { trace } from '@opentelemetry/api'; // ❌ OTel imports
58+
export function create(config = {}) { } // ❌ Default deps
59+
onConfigChange(config, () => {}); // ❌ Not saving hook
60+
```
61+
62+
---
63+
64+
## Testing
65+
66+
- Call `core.unload(false)` in cleanup
67+
- Test dynamic config changes
68+
- Use framework helpers (`this.hookFetch()`, `this.useFakeTimers()`)
69+
- Return `IPromise` for async tests
70+
71+
---
72+
73+
## Performance Targets (p95)
74+
75+
| Operation | Target |
76+
|-----------|--------|
77+
| SDK Init | < 5ms |
78+
| Span Creation | < 0.1ms |
79+
| Attribute Addition | < 0.05ms |
80+
| Span Completion | < 0.2ms |
81+
82+
No continuous timers — start only when work exists, stop when complete.

0 commit comments

Comments
 (0)