-
Notifications
You must be signed in to change notification settings - Fork 0
feat(telemetry): implement storage layer (Phase 1) #20
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
Changes from all commits
9715ae9
41953f8
c650e68
b16ecd7
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,101 @@ | ||
| # Session Handoff | ||
|
|
||
| Persistent context bridge for autonomous ROADMAP execution across sessions. Each session reads this file at startup and updates it after completing a phase. | ||
|
|
||
| --- | ||
|
|
||
| ## Current State | ||
|
|
||
| | Field | Value | | ||
| | ------------------------ | --------------------------- | | ||
| | **Active Wave** | Wave 1: Core Infrastructure | | ||
| | **Active Package** | telemetry | | ||
| | **Last Completed Phase** | telemetry phase 1 | | ||
| | **Last Session** | 2026-04-04 | | ||
| | **ROADMAP Status** | Phase 1 complete | | ||
|
|
||
| --- | ||
|
|
||
| ## Completed Work | ||
|
|
||
| | Wave | Package | Phase | Title | | ||
| | ------ | --------- | ------- | ------------------------------- | | ||
| | Wave 1 | telemetry | Phase 0 | Foundation (types, scaffolding) | | ||
| | Wave 1 | telemetry | Phase 1 | Storage Layer (Memory + File) | | ||
|
|
||
| --- | ||
|
|
||
| ## Key Decisions (ADR Summary) | ||
|
|
||
| - **Storage backends are pluggable**: `StorageBackend` interface with `append()`, `query()`, `count()`. Consumers choose backend at `createTelemetry()` time (Phase 4). | ||
| - **JSONL file format**: Default zero-config backend uses newline-delimited JSON. Append-only, no external dependencies. | ||
| - **`matchesFilter` is shared**: Extracted to `src/storage/matchesFilter.ts` to avoid duplication across backends. | ||
| - **Lazy directory creation**: `FileStorageBackend` creates directories on first `append()`, not at construction time. | ||
|
|
||
| --- | ||
|
|
||
| ## Project Patterns | ||
|
|
||
| Established patterns -- follow these in all packages: | ||
|
|
||
| - **Module format**: ESM (`"type": "module"` in all packages) | ||
| - **Imports**: Use `.js` extension in relative imports | ||
| - **Types**: Co-locate in `types.ts` per module, export from `index.ts` | ||
| - **Tests**: Place in `tests/` directory at package root, named `*.test.ts` | ||
| - **Quality gates**: `npx nx affected -t build,test,lint` | ||
| - **Shared utilities**: Extract duplicated logic to standalone files named after the export (`filename-match-export` rule) | ||
| - **Empty catch pattern**: Use `flatMap` with `try/catch` returning `[]` for skip-on-error — no logging needed | ||
|
|
||
| --- | ||
|
|
||
| ## Known Issues & Gotchas | ||
|
|
||
| - `count()` in both backends loads all events into memory before counting. Acceptable for Phase 1; optimize in a future phase if needed. | ||
|
|
||
| --- | ||
|
|
||
| ## Lessons Learned | ||
|
|
||
| - The `llm-core/filename-match-export` lint rule requires files with a single export to be named after that export. | ||
| - The comments hook flags all new comments/docstrings — only add when absolutely necessary. | ||
| - `afterEach` must be explicitly imported from vitest (not relied upon as a global). | ||
|
|
||
| --- | ||
|
|
||
| ## Next Phase Context | ||
|
|
||
| ### Target | ||
|
|
||
| Wave 1: @open-forge/telemetry Phase 2 (Telemetry Core) | ||
|
|
||
| ### Goal | ||
|
|
||
| Implement event emission and querying — the `Telemetry` interface: `emit()`, `query()`, `getPipelineSummary()`. | ||
|
|
||
| ### Critical Context | ||
|
|
||
| - Storage backends are complete and tested (33 tests passing) | ||
| - Types already defined in `packages/telemetry/src/types.ts` (including `Telemetry`, `PipelineSummary`) | ||
| - Phase 2 deliverable: `src/telemetry.ts` + `tests/telemetry.test.ts` | ||
| - Sequential dependency: 2.1 (`emit`) → 2.2 (`query`) → 2.3 (`getPipelineSummary`) → 2.4 (tests) | ||
| - Requirements covered: F-001, F-002, F-003, F-004, F-005, F-006, F-007, F-013 | ||
|
|
||
| --- | ||
|
|
||
| ## Session Resumption Instructions | ||
|
|
||
| 1. Read this file: cat HANDOFF.md | ||
| 2. Check root status: bash scripts/forge-helper.sh status | ||
| 3. Check package status: bash scripts/forge-helper.sh status --package telemetry | ||
| 4. Resume work on the next phase | ||
| 5. After completion: update this file | ||
|
|
||
| --- | ||
|
|
||
| ## Changelog | ||
|
|
||
| | Date | Wave | Package | Phase | Notes | | ||
| | ---------- | ------ | --------- | ----- | ---------------------------------------- | | ||
| | 2026-04-03 | -- | -- | -- | Created HANDOFF.md | | ||
| | 2026-04-03 | Wave 1 | telemetry | 1 | Storage Layer -- 4/4 tasks | | ||
| | 2026-04-04 | Wave 1 | telemetry | 1 | PR review fixes, matchesFilter extracted | |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-04-04 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # telemetry-phase-1 | ||
|
|
||
| telemetry Phase 1: Storage Layer |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| ## Context | ||
|
|
||
| The `@open-forge/telemetry` package currently exports only TypeScript types (Phase 0). The storage layer is the first real implementation — it provides the persistence mechanism that `emit()` and `query()` (Phase 2) will use. The existing `PipelineEvent` and `EventFilter` types in `src/types.ts` define the data model. | ||
|
|
||
| Constraint: Zero required runtime dependencies for core functionality (per package ROADMAP). | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
|
|
||
| - Define a `StorageBackend` interface that decouples telemetry logic from persistence | ||
| - Ship two concrete backends: in-memory (for tests/lightweight) and file-based JSONL (for production default) | ||
| - All backends satisfy the same contract and are interchangeable | ||
| - File backend works zero-config with only a path | ||
|
|
||
| **Non-Goals:** | ||
|
|
||
| - SQLite or database backends (deferred per ROADMAP) | ||
| - Concurrent write safety across processes (single-writer assumption for v1) | ||
| - Streaming/pagination for large result sets (can be added later) | ||
| - Hot-swapping backends at runtime | ||
|
|
||
| ## Decisions | ||
|
|
||
| ### 1. StorageBackend as a standalone interface (not extending Telemetry) | ||
|
|
||
| **Decision**: `StorageBackend` is a separate interface with `append()`, `query()`, `count()` — it does NOT extend or overlap with the `Telemetry` interface. | ||
|
|
||
| **Rationale**: `Telemetry.emit()` will do validation and enrichment before calling `StorageBackend.append()`. Keeping them separate enforces single responsibility. The `Telemetry` implementation (Phase 2) composes a `StorageBackend`. | ||
|
|
||
| **Alternative considered**: Having `Telemetry` directly implement storage. Rejected — couples business logic to I/O. | ||
|
|
||
| ### 2. JSONL for file storage format | ||
|
|
||
| **Decision**: One JSON object per line, append-only. | ||
|
|
||
| **Rationale**: Simple, human-readable, no corruption from partial writes (each line is independent), streamable, and requires no dependencies. Alternatives like SQLite or binary formats add complexity for marginal gains at this scale. | ||
|
|
||
| ### 3. File storage creates directories on first write, not on construction | ||
|
|
||
| **Decision**: The `FileStorageBackend` constructor stores the path. Directory/file creation happens on first `append()`. | ||
|
|
||
| **Rationale**: Avoids side effects during construction. `query()` on a non-existent file returns empty — no error. | ||
|
|
||
| ### 4. Query filtering uses in-process filtering | ||
|
|
||
| **Decision**: Both backends load/scan events and filter in-process using `EventFilter` fields. | ||
|
|
||
| **Rationale**: Simplicity. At expected telemetry volumes (hundreds to low thousands of events per pipeline), in-process filtering is fast enough. Indexing can be added in a future phase if needed. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **[Large file performance]** → File backend reads the entire file for queries. Mitigation: acceptable for v1 volumes; SQLite backend is in the deferred items list. | ||
| - **[No concurrent writes]** → Single-writer assumption. Mitigation: pipeline is single-threaded; document the limitation. | ||
| - **[JSONL corruption on crash]** → Partial line write possible if process crashes mid-append. Mitigation: each line is independent; partial lines are skipped on read. | ||
|
|
||
| ## Open Questions | ||
|
|
||
| None — scope is well-defined by the ROADMAP and requirements doc. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ## Why | ||
|
|
||
| The telemetry package currently exports only type definitions (Phase 0). Every downstream consumer — the pipeline orchestrator, constraint evaluator, and evaluations integration — needs real storage backends that can persist and query pipeline events. Without a storage layer, telemetry cannot function. This is the critical path: Wave 1 in the unified ROADMAP, blocking all subsequent waves. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - New `StorageBackend` interface defining the contract for all storage implementations (`append`, `query`, `count`) | ||
| - In-memory storage backend for testing and lightweight/ephemeral use cases | ||
| - File-based JSONL storage backend as the default zero-config production backend (append-only, no external dependencies) | ||
| - All backends are pluggable — consumers choose at `createTelemetry()` time (Phase 4) | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| - `storage-interface`: Defines the `StorageBackend` contract — `append()`, `query()`, `count()` — that all storage implementations must satisfy | ||
| - `memory-storage`: In-memory `StorageBackend` implementation for testing and lightweight use | ||
| - `file-storage`: JSONL append-only file `StorageBackend` implementation as the default zero-config backend | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| <!-- No existing capabilities are being modified — this is all new implementation --> | ||
|
|
||
| ## Impact | ||
|
|
||
| - **New files**: `src/storage/StorageBackend.ts`, `src/storage/MemoryStorageBackend.ts`, `src/storage/FileStorageBackend.ts`, `src/storage/matchesFilter.ts`, `tests/storage.test.ts` | ||
| - **Modified files**: `src/index.ts` (export storage implementations) | ||
| - **APIs**: New `StorageBackend` interface and two concrete implementations | ||
| - **Dependencies**: Zero runtime dependencies (Node.js `fs` and `path` for file backend only) | ||
| - **Downstream**: Unblocks Phase 2 (Telemetry Core) which needs storage to implement `emit()` and `query()` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: File storage implements StorageBackend | ||
|
|
||
| The `FileStorageBackend` class SHALL implement the `StorageBackend` interface, persisting events as JSONL (one JSON object per line) in an append-only file. | ||
|
|
||
| #### Scenario: Create file storage with path | ||
|
|
||
| - **WHEN** a `FileStorageBackend` is instantiated with a file path | ||
| - **THEN** it SHALL create the file (and parent directories) if they do not exist | ||
|
|
||
| ### Requirement: File storage append writes JSONL | ||
|
|
||
| The `FileStorageBackend.append()` method SHALL serialize the event as JSON and append it as a single line to the storage file. | ||
|
|
||
| #### Scenario: Append writes one line per event | ||
|
|
||
| - **WHEN** three events are appended | ||
| - **THEN** the file SHALL contain exactly three lines, each being valid JSON | ||
|
|
||
| #### Scenario: Append is atomic per event | ||
|
|
||
| - **WHEN** `append()` is called | ||
| - **THEN** each event SHALL be written as a complete line (no partial writes visible to readers) | ||
|
|
||
| ### Requirement: File storage query reads and filters JSONL | ||
|
|
||
| The `FileStorageBackend.query()` method SHALL read the JSONL file, parse each line, and filter by the provided `EventFilter`. | ||
|
|
||
| #### Scenario: Query filters persisted events | ||
|
|
||
| - **WHEN** events are appended and then queried with a filter | ||
| - **THEN** only matching events SHALL be returned | ||
|
|
||
| ### Requirement: File storage persists across instances | ||
|
|
||
| Events stored by `FileStorageBackend` SHALL be readable by a new instance pointing to the same file. | ||
|
|
||
| #### Scenario: Data survives instance recreation | ||
|
|
||
| - **WHEN** events are appended, the instance is discarded, and a new instance is created with the same path | ||
| - **THEN** the new instance SHALL return the previously stored events via `query()` | ||
|
|
||
| ### Requirement: File storage count avoids full deserialization | ||
|
|
||
| The `FileStorageBackend.count()` method SHALL count matching events efficiently without requiring all events to be held in memory simultaneously. | ||
|
Comment on lines
+44
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Check FileStorageBackend.count() implementation
# Show the count() method implementation
ast-grep --pattern $'count($$$) {
$$$
}' packages/telemetry/src/storage/FileStorageBackend.ts
# Show if it calls readAll() which loads everything into memory
rg -A 5 'async count\(' packages/telemetry/src/storage/FileStorageBackend.tsRepository: pertrai1/open-forge Length of output: 282 Fix count() to match spec requirement: avoid full deserialization. The spec requires Implement a streaming approach instead—process the file line-by-line, deserializing only to check the filter, without materializing the full event array: async count(filter: EventFilter): Promise<number> {
const content = await readFile(this.filePath, 'utf-8').catch(err => {
if (isNodeError(err) && err.code === 'ENOENT') return '';
throw err;
});
let count = 0;
for (const line of content.split('\n')) {
if (line.trim() === '') continue;
try {
const event = JSON.parse(line) as PipelineEvent;
if (matchesFilter(event, filter)) count++;
} catch {
// Skip malformed lines
}
}
return count;
}For even better memory efficiency with very large files, consider using Node.js streams with 🤖 Prompt for AI Agents |
||
|
|
||
| #### Scenario: Count returns correct number | ||
|
|
||
| - **WHEN** events are appended and `count()` is called with a matching filter | ||
| - **THEN** the correct count SHALL be returned | ||
|
|
||
| ### Requirement: File storage handles empty or missing file | ||
|
|
||
| The `FileStorageBackend` SHALL gracefully handle cases where the file does not yet exist or is empty. | ||
|
|
||
| #### Scenario: Query on non-existent file | ||
|
|
||
| - **WHEN** `query()` is called before any events are appended (file does not exist) | ||
| - **THEN** an empty array SHALL be returned | ||
|
|
||
| #### Scenario: Count on empty file | ||
|
|
||
| - **WHEN** `count()` is called on an empty file | ||
| - **THEN** zero SHALL be returned | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: In-memory storage implements StorageBackend | ||
|
|
||
| The `MemoryStorageBackend` class SHALL implement the `StorageBackend` interface, storing all events in an in-memory array. | ||
|
|
||
| #### Scenario: Create in-memory storage | ||
|
|
||
| - **WHEN** a `MemoryStorageBackend` is instantiated | ||
| - **THEN** it SHALL be ready for use with zero configuration | ||
|
|
||
| ### Requirement: In-memory storage append is synchronous-fast | ||
|
|
||
| The `MemoryStorageBackend.append()` method SHALL resolve immediately after pushing the event to the internal array. | ||
|
|
||
| #### Scenario: Append resolves without delay | ||
|
|
||
| - **WHEN** `append()` is called | ||
| - **THEN** the promise SHALL resolve within 1ms under normal conditions | ||
|
|
||
| ### Requirement: In-memory storage query filters correctly | ||
|
|
||
| The `MemoryStorageBackend.query()` method SHALL filter the internal array by all provided `EventFilter` fields. | ||
|
|
||
| #### Scenario: Query with combined filters | ||
|
|
||
| - **WHEN** multiple events exist for different pipelines and stages | ||
| - **THEN** `query()` with combined filters SHALL return only matching events | ||
|
|
||
| ### Requirement: In-memory storage is ephemeral | ||
|
|
||
| Events stored in `MemoryStorageBackend` SHALL not persist beyond the lifetime of the instance. | ||
|
|
||
| #### Scenario: Data lost on new instance | ||
|
|
||
| - **WHEN** a `MemoryStorageBackend` is created, events are appended, and a new instance is created | ||
| - **THEN** the new instance SHALL have zero events |
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.
Do not continue loop when archive fails.
Line 205’s “warn but continue” risks marking a phase as completed in handoff/status while the change is still unarchived. This can create workflow drift and duplicate future work.
Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents