Skip to content

feat: structured-debug-logging — structured logging for LangGraph client and skills#261

Closed
avoidwork wants to merge 5 commits into
mainfrom
feat/structured-debug-logging
Closed

feat: structured-debug-logging — structured logging for LangGraph client and skills#261
avoidwork wants to merge 5 commits into
mainfrom
feat/structured-debug-logging

Conversation

@avoidwork

@avoidwork avoidwork commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Description

Add structured, config-driven logging for LangGraph client events (tool calls, LLM responses, errors) and skill execution. Introduces a logging section in config.yaml with configurable log level (default: "info"), creates src/logging/config.js (config-driven pino logger factory) and src/logging/handlers.js (stream event handlers), and instruments src/agent/react.js and src/skills/registry.js with structured logging.

Type of Change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature which would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor (no functional changes)
  • Performance improvement
  • CI / build / tooling

Testing

  • Unit tests for src/logging/config.js: 16 tests covering default level, custom levels, invalid level fallback, singleton methods
  • Unit tests for src/logging/handlers.js: 22 tests covering all event handlers, wrapCallback, tool duration tracking
  • All 1132 existing tests pass (no regression)
  • Manual verification: start app with different log levels, verify structured logs appear in madz.log

Coverage

  • 100% line coverage maintained

Checklist

  • npm run lint passes
  • Tests pass with 100% line coverage
  • No forbidden patterns used
  • Conventional Commit style applied

Threat Model

  • Log volume: Debug-level logging could produce large logs. Mitigated by default "info" level and input/output truncation (200 chars).
  • Sensitive data: Tool inputs may contain sensitive data. Truncation limits exposure; users aware that debug logs may contain interaction data.
  • No new dependencies: Uses existing pino logger, no new external packages introduced.

Files Changed

  • config.yaml — added logging section with level: info and format: json
  • src/config/schemas.js — added LoggingSchema with level and format enums
  • src/logging/config.js — NEW: config-driven pino logger factory with OS-aware log directory, dual-file output, test suppression
  • src/logging/handlers.js — NEW: event handler factory for LangGraph stream events (tool_start/end/error, llm_response/error, compaction)
  • src/agent/react.js — instrumented callReactAgentStreaming() and callReactAgent() with structured logging
  • src/skills/registry.js — instrumented get() method with skill execution logging
  • tests/unit/logging/config.test.js — NEW: 16 unit tests for logger factory
  • tests/unit/logging/handlers.test.js — NEW: 22 unit tests for event handlers
  • openspec/specs/structured-logging/spec.md — archived spec with 4 requirements

@avoidwork avoidwork self-assigned this Jun 15, 2026
@avoidwork

Copy link
Copy Markdown
Owner Author

Audit Results: structured-debug-logging

Goal Fulfillment

Goal 1 (Config-Driven Logging Infrastructure): ✅ COMPLETE

  • src/config/schemas.js updated with LoggingSchema (level: debug|info|warn|error, format: json|text)
  • config.yaml updated with logging: { level: info, format: json }
  • src/logging/config.js created — config-driven pino logger factory with OS-aware log directory, dual-file output, test suppression
  • Backward compatibility maintained — src/logger.js unchanged

Goal 2 (LangGraph Stream Event Logging): ✅ COMPLETE

  • src/logging/handlers.js created with event handlers for: tool_start, tool_end, tool_error, llm_response, llm_error, compaction
  • src/agent/react.js callReactAgentStreaming() modified to use wrapCallback with event handlers
  • All stream events logged with structured type field
  • Input/output truncation (200 chars) prevents log bloat
  • Tool duration tracking implemented

Goal 3 (LangGraph Invoke Path Logging): ✅ COMPLETE

  • src/agent/react.js callReactAgent() modified to log llm_response, llm_error, and compaction events
  • Errors include full context for debugging

Goal 4 (Skill Execution Logging): ✅ COMPLETE

  • src/skills/registry.js get() method wrapped with try/catch and structured logging
  • Logs skill_call, skill_success, and skill_error events
  • Stack traces truncated to 500 chars

Goal 5 (Testing): ✅ COMPLETE

  • tests/unit/logging/config.test.js — 16 tests covering default level, custom levels, invalid level fallback, singleton methods
  • tests/unit/logging/handlers.test.js — 22 tests covering all event handlers, wrapCallback, tool duration tracking
  • All 1132 existing tests pass (no regression)

Goal 6 (Verification): ⏳ MANUAL (not automated)

  • Requires running the app with different log levels to verify structured logs appear in madz.log

Spec Compliance

All spec requirements met:

  • ✅ Config-driven log level with default "info"
  • ✅ Invalid level fallback to "info" with stderr warning
  • ✅ Structured stream event logging with type field
  • ✅ Tool start/end/error logging with truncation and duration
  • ✅ LLM response/error logging
  • ✅ Compaction logging
  • ✅ Skill execution logging (skill_call, skill_success, skill_error)
  • ✅ Stack trace truncation (500 chars)
  • ✅ Test coverage for all new modules

Task Completion

All 24 tasks completed:

  • 1.1-1.2: Config schema and default config ✅
  • 2.1-2.4: Logger factory module ✅
  • 3.1-3.7: Stream event handler module ✅
  • 4.1-4.3: React agent instrumentation ✅
  • 5.1-5.3: Skill registry instrumentation ✅
  • 6.1-6.5: Testing ✅

Quality Check

  • No forbidden patterns used (no console.log, no silent catch blocks)
  • All public functions have JSDoc comments
  • 2 new test files with 38 total tests
  • All 1132 existing tests pass
  • Code follows project conventions (2-space indent, max 100 char lines)
  • No hardcoded secrets or credentials
  • Backward compatible — existing code continues to work

Deviations from Original Plan

None. Implementation matches the spec and design documents exactly.

@avoidwork

Copy link
Copy Markdown
Owner Author

This PR introduces a second, independent logger instance in alongside the existing . Having two separate pino logger instances with their own file handles, flush mechanisms, and configuration is an incorrect architectural solution.

The proper approach is to extend the existing logger in with the structured logging capabilities (event handlers, config-driven levels, etc.) rather than creating a parallel logging system. This keeps a single source of truth for logging, avoids duplicate file handles, and ensures consistent shutdown/flush behavior.

Closing this PR — please resubmit as an enhancement to instead.

@avoidwork avoidwork closed this Jun 15, 2026
@avoidwork avoidwork deleted the feat/structured-debug-logging branch June 15, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant