-
Notifications
You must be signed in to change notification settings - Fork 4
docs: add unified logger epic planning and task breakdown #36
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.
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
0a08858
1a12102
35bf10b
ac5c280
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,266 @@ | ||||||||
| # Unified Logger Epic | ||||||||
|
|
||||||||
| **Status**: 📋 PLANNED | ||||||||
| **Goal**: Create event-driven logging framework for unified telemetry across client and server | ||||||||
|
|
||||||||
| ## Overview | ||||||||
|
|
||||||||
| Developers need a unified logging solution that works consistently across client and server environments to enable reliable telemetry, debugging, and monitoring without coupling to specific dispatch implementations. This logger subscribes to framework events and handles sampling, sanitization, batching, and transport with built-in offline resilience and GDPR compliance. | ||||||||
|
|
||||||||
| --- | ||||||||
|
|
||||||||
| ## Interfaces | ||||||||
|
|
||||||||
| ```sudo | ||||||||
| // Base types | ||||||||
| type ID = string(cuid2) | ||||||||
| type Timestamp = number(epoch ms) | ||||||||
| type LogLevel = "debug" | "info" | "warn" | "error" | "fatal" | ||||||||
| type Sanitizer = (payload: any) => any | ||||||||
| type Serializer = (payload: any) => string | ||||||||
| type RxJSOperator = function(Observable) => Observable | ||||||||
| // Framework interfaces (out of scope - mocked for testing) | ||||||||
| type Dispatch = (event: Event) => void // synchronous, returns nothing | ||||||||
| type Events$ = Observable<Event> // RxJS Observable | ||||||||
| // Event structures (Redux-compatible action objects) | ||||||||
| Event { | ||||||||
| type: string | ||||||||
| payload: any // typically LogPayload but can be any | ||||||||
| } | ||||||||
| LogPayload { | ||||||||
| timestamp: Timestamp // Date.now() at creation | ||||||||
| message: string | ||||||||
| logLevel: LogLevel | ||||||||
| sanitizer?: Sanitizer // optional override | ||||||||
| serializer?: Serializer // optional override | ||||||||
| context?: Record<string, any> // contextual fields | ||||||||
| props?: Record<string, any> // additional structured data | ||||||||
| } | ||||||||
| EnrichedEvent { | ||||||||
| ...Event | ||||||||
| schemaVersion: number // = 1 | ||||||||
| eventId: ID // cuid2() | ||||||||
| userPseudoId: string | ||||||||
| requestId?: ID | ||||||||
| sessionId?: ID | ||||||||
| appVersion?: string | ||||||||
| route?: string | ||||||||
| locale?: string | ||||||||
| createdAt?: Timestamp // server ingestion time | ||||||||
| } | ||||||||
| // Configuration | ||||||||
| LoggerOptions { | ||||||||
| endpoint?: string // POST target (client: '/api/events', server: 'console') | ||||||||
|
||||||||
| endpoint?: string // POST target (client: '/api/events', server: 'console') | |
| endpoint?: string // POST base path (e.g., '/api/events'); implementation appends '/${eventId}' for idempotent delivery (client), or 'console' for server |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing interface definition. The Server Event Endpoint requirement (line 219) references allowedOrigins for origin validation, but this configuration option is not defined in the LoggerOptions interface. Add allowedOrigins?: string[] to the LoggerOptions interface to align with this requirement, or clarify how allowed origins are configured.
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent capitalization of "RxJS". The library name "RxJS" should be consistently capitalized. Line 83 uses lowercase "rxjs pipeable operator" and line 249 uses lowercase "rxjs". These should be "RxJS pipeable operator" and "RxJS" respectively to match the official library name capitalization used in the type name "RxJSOperator".
| sampler?: RxJSOperator // rxjs pipeable operator | |
| sampler?: RxJSOperator // RxJS pipeable operator |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing interface definition. The Schema Validation requirement (line 238) mentions that developers can "register EventConfig with schema", but the EventConfig interface (lines 81-87) does not include a schema field. Add schema?: object or a more specific JSON schema type to the EventConfig interface to align with this requirement.
| level?: LogLevel // default: 'info' | |
| level?: LogLevel // default: 'info' | |
| schema?: object // optional JSON schema for validation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Batching Conflicts with URL Design
The client transport requirements for flushing events are contradictory. The design specifies POSTing to an endpoint with a single event ID in the URL path, but also requires batching multiple events into a single request body. This creates an ambiguous URL structure for batched requests and conflicts with the server's idempotency handling, which expects individual event IDs.
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The requirement mentions checking "referer origin" but the correct HTTP header name is "Referer" (note the misspelling in the HTTP standard is intentional). However, the requirement says "request referer origin does not match origin header" which seems like a security check. Consider clarifying what this check is validating - typically you'd check that the Origin header matches allowed origins, or that Referer matches the request origin, but checking "referer origin does not match origin header" is unclear. This might need to be: "Given request Referer header origin does not match Origin header, should reject with 400 Bad Request" or simply removed if covered by the allowedOrigins check on line 219.
| - Given request referer origin does not match origin header, should reject with 400 Bad Request |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent capitalization of "RxJS". The library name should be "RxJS" (not "rxjs") to match the official library name capitalization used elsewhere in the document (e.g., "RxJSOperator" type name).
| - Given tests need events$ stub, should provide Subject from rxjs for controllable event emission | |
| - Given tests need events$ stub, should provide Subject from RxJS for controllable event emission |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect task count and incomplete task list. The summary states "9 tasks" but the epic contains 10 task sections. Additionally, the task list omits "Server Event Endpoint" (which is distinct from the Client Transport Layer). Update to: "10 tasks (core infrastructure, client implementation, server implementation, event configuration, security/privacy, client transport, server endpoint, schema validation, testing, documentation)".