feat: implement telemetry and observability layer#170
Open
shantanushok wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new telemetry/observability layer to the Memact engine by introducing a lightweight telemetry collector + in-process metrics registry, plus an instrumented-engine wrapper and new stats-server endpoints to expose health and metrics.
Changes:
- Added a telemetry collector (
src/telemetry.mjs) with pub/sub events, span timing, buffering, sinks, and basic per-operation aggregation. - Added a metrics registry (
src/telemetry-metrics.mjs) and an instrumented engine decorator (src/instrumented-engine.mjs) to emit events and record metrics around engine operations. - Extended the stats server with
/healthand/metrics, and updated engine/store stats factoring + tests to cover the new modules.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| test/telemetry.test.mjs | Adds unit coverage for collector emit/on/off/span/flush/metrics/reset behaviors. |
| test/telemetry-metrics.test.mjs | Adds unit coverage for counter/gauge/histogram and registry snapshot/reset. |
| test/instrumented-engine.test.mjs | Verifies instrumented engine pass-through correctness and event emission. |
| test/audit-trail-test.mjs | Switches audit verification from auditTrailLog output to telemetry event capture. |
| src/telemetry.mjs | Implements the core telemetry collector/event bus and span timing. |
| src/telemetry-metrics.mjs | Implements counters/gauges/histograms and a standard registry factory. |
| src/stats-server.mjs | Adds /health and /metrics endpoints and wires in optional collector. |
| src/instrumented-engine.mjs | Adds decorator wrapper that instruments engine operations with telemetry + metrics. |
| src/engine.mjs | Factors store stats computation and removes Object.defineProperty auditTrailLog attachment. |
| package.json | Exposes new telemetry modules via package exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * span timing, buffered flush, and metrics aggregation. | ||
| * | ||
| * @param {Object} [options={}] | ||
| * @param {number} [options.bufferSize=200] - Max events to buffer before auto-flush |
Comment on lines
1472
to
1476
| graph: buildMemoryGraph(memories, memoryStore.relations || []), | ||
| relations: memoryStore.relations || [], | ||
| actions: [...(memoryStore.actions || []), finalAction], | ||
| stats: { | ||
| ...(memoryStore.stats || {}), | ||
| memoryCount: memories.length, | ||
| activityMemoryCount: memories.filter((memory) => memory.type === "activity_memory").length, | ||
| intentMemoryCount: memories.filter(isIntentMemory).length, | ||
| schemaMemoryCount: memories.filter(isSchemaMemory).length, | ||
| }, | ||
| stats: computeStoreStats(memories, buildMemoryGraph(memories, memoryStore.relations || [])), | ||
| }; |
Comment on lines
+58
to
+61
| type, | ||
| timestamp: new Date().toISOString(), | ||
| trace_id: context.trace_id || generateTraceId(), | ||
| duration_ms: context.duration_ms ?? null, |
Comment on lines
+192
to
+194
| function startSpan(eventType, context = {}) { | ||
| const traceId = context.trace_id || generateTraceId(); | ||
| const startTime = performance.now(); |
8 tasks
Author
|
I have addressed the above comments in the latest commit if you do have any suggestions , changes or modifications I may have missed or misunderstood please reach out on this conversation or on the discord channel . |
Member
|
@keepsloading Hey! I tried to merge this but it has merge conflicts with |
# Conflicts: # src/stats-server.mjs # src/telemetry.mjs # test/telemetry.test.mjs
Added test for invalid trace_id handling in span.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a robust Telemetry and Observability layer to the Memact engine. It implements a zero-dependency, pluggable architecture using a pub/sub event bus, metrics aggregation (counters, gauges, histograms) it wraps the engine functions without modifying their underlying logic. It exposes these insights via new /metrics and /health HTTP endpoints, laying the foundation for log storage and system monitoring. Trace context IDs are fully W3C-compatible (32-character hex strings) to allow potential transition to OpenTelemetry SDK in the future.
Protocol Alignment
Changes Made
1 ] src/telemetry.mjs : Core telemetry event bus supporting span timing, typed events, wildcard subscribers, and buffered sink flushing. Generates W3C-compliant trace IDs.
2 ] src/telemetry-metrics.mjs : Lightweight, in-process metrics registry supplying Counters, Gauges, and Histograms for operation tracking.
3 ] src/instrumented-engine.mjs : Decorator wrapper that overlays span telemetry and metrics onto all public engine.mjs operations.
4 ] src/engine.mjs : Removed the auditTrailLog Object.defineProperty in retrieveMemories and extracted redundant inline stat computations into a shared computeStoreStats helper.
5 ] src/stats-server.mjs : Added the GET /metrics and GET /health endpoints, maintaining compatibility for the root / stats route.
6 ] test/audit-trail-test.mjs : Migrated to utilize the new telemetry collector pattern instead of the legacy object properties.
7 ] test/telemetry.test.mjs : Verifies the event bus behavior, ensuring correct pub/sub routing, span timing, sink invocation, and trace ID generation.
8 ] test/telemetry-metrics.test.mjs : Tests the initialization and aggregation boundaries of all standard operational metrics (counters, histogram percentiles, gauges).
9 ] test/instrumented-engine.test.mjs : Confirms the decorator cleanly wraps engine functions, correctly emits events upon memory lifecycle operations, and safely bypasses when disabled.
10] package.json - added the path of the new files created.
Testing
1] Added 3 new dedicated test suites containing 59 new assertions.
2] Verified W3C trace context format validation.
3] All 184 tests in the suite pass successfully with zero regressions.
Checklist
Linked Issue
Closes #128