Skip to content

Conversation

@amikofalvy
Copy link
Collaborator

Summary

  • Add trace() method to PinoLogger for very verbose logging
  • Add isDebugEnabled() and isTraceEnabled() helpers for conditional logging
  • Change default LOG_LEVEL from 'debug' to 'info' in both agents-run-api and agents-manage-api

Motivation

This is Phase 1 of the log reduction effort. These changes enable:

  1. Gated logging - Expensive operations like JSON.stringify() can be wrapped in if (logger.isDebugEnabled()) checks
  2. Quieter default operation - Default LOG_LEVEL=info means production and local dev are quieter by default
  3. Trace level support - For cache operations and other very verbose logs that are useful during deep debugging

Test plan

  • Verify LOG_LEVEL=info shows only info/warn/error logs
  • Verify LOG_LEVEL=debug shows debug logs + above
  • Verify LOG_LEVEL=trace shows all logs including trace

🤖 Generated with Claude Code

- Add trace() method to PinoLogger for very verbose logging
- Add isDebugEnabled() and isTraceEnabled() helpers for conditional logging
- Change default LOG_LEVEL from 'debug' to 'info' in both APIs

This enables gated logging of expensive operations (like JSON.stringify)
and prepares for downstream log level downgrade changes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@changeset-bot
Copy link

changeset-bot bot commented Jan 8, 2026

⚠️ No Changeset found

Latest commit: bee2ba0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jan 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
agents-docs Ready Ready Preview, Comment Jan 8, 2026 10:50pm
agents-manage-api Ready Ready Preview, Comment Jan 8, 2026 10:50pm
agents-manage-ui Ready Ready Preview, Comment Jan 8, 2026 10:50pm
agents-run-api Ready Ready Preview, Comment Jan 8, 2026 10:50pm

@claude
Copy link

claude bot commented Jan 8, 2026

Claude finished @amikofalvy's task —— View job


PR Review: Logger Infrastructure for Log Reduction

Reviewing the logger infrastructure changes as a Staff Engineer and System Architect.

Review Progress

  • Examine changed files and implementation details
  • Research existing logging patterns in codebase
  • Analyze impact of default log level changes
  • Check for consistency and architectural considerations
  • Provide comprehensive technical feedback

Summary

This PR introduces solid logging infrastructure improvements with minimal breaking changes. The implementation is clean and follows existing patterns, but there are several architectural and user impact considerations that should be addressed.


Technical Implementation ✅

Strengths:

  • Clean API design: The new trace(), isDebugEnabled(), and isTraceEnabled() methods follow pino's established patterns
  • Proper integration: Uses pino's native isLevelEnabled() method, ensuring consistency with pino's log level checking
  • Non-breaking addition: New methods are additive and don't modify existing behavior

Code Quality:

  • Implementation at packages/agents-core/src/utils/logger.ts:162-172 is straightforward and correct
  • Consistent method signatures match existing logger methods
  • No security or performance concerns with the helper methods

System Architecture & Design Concerns ⚠️

1. Breaking Change Impact - HIGH PRIORITY

The default LOG_LEVEL change from 'debug' to 'info' is a significant behavioral change that affects:

Impact Analysis:

  • Found 20+ existing logger.debug() calls across the codebase (agents-run-api/src/agents/Agent.ts:715, agents-run-api/src/agents/ToolSessionManager.ts:77, etc.)
  • These debug logs will now be silent by default in development and production
  • Existing debug logging includes critical operational information like:
    • Tool approval checks (Agent.ts:715-723)
    • Session management (ToolSessionManager.ts:102)
    • Agent configuration loading (Agent.ts:1391)

Recommendation: Consider this a minor version change rather than patch, as it changes default runtime behavior.

2. Documentation Inconsistency - MEDIUM PRIORITY

README files still reference the old defaults:

  • agents-run-api/README.md:50: Still shows LOG_LEVEL=debug|info|warn|error
  • agents-manage-api/README.md:45: Same issue
  • These should document trace level and mention the new info default

3. Missing Changeset - BLOCKING

The changeset-bot correctly flagged this needs a changeset. Given the behavioral impact, this should include:

---
"@inkeep/agents-manage-api": minor
"@inkeep/agents-run-api": minor  
"@inkeep/agents-core": minor
---

BREAKING: Change default LOG_LEVEL from 'debug' to 'info'. Existing debug logs will be silent by default. Add logger.trace() and conditional logging helpers.

User Experience & Customer Considerations ⚠️

1. Developer Experience Impact

  • Positive: Quieter defaults reduce noise for new developers
  • Negative: Existing debugging workflows may break if developers rely on default debug output
  • Migration path needed: Consider adding a migration guide or temporary notice

2. Operational Observability

  • Important debugging information will be hidden by default
  • Operations teams may need to adjust monitoring/alerting if they depend on debug logs
  • Consider: Are any of the current debug logs important enough to be info level?

Phase 1 Strategy & Future Considerations 📋

1. Conditional Logging Readiness

Strength: The infrastructure supports the stated goal of gated expensive operations.

Gap: No immediate usage examples of the new helpers. Consider adding a follow-up example:

// Example of intended usage pattern
if (logger.isDebugEnabled()) {
  logger.debug({ complexObject: JSON.stringify(complexData) }, 'Complex operation debug');
}

2. Architectural Consistency

The PinoLogger class at packages/agents-core/src/utils/logger.ts already handles test environments appropriately (line 35: ENVIRONMENT === 'test' ? 'silent' : 'info'), which is good design.

3. Trace Level Usage Strategy

Consider documenting intended use cases for trace level:

  • Cache operations (as mentioned in PR description)
  • Very verbose debugging
  • Development-only diagnostics

Recommendations

High Priority

  1. Add changeset with proper semver classification (minor/breaking change)
  2. Update documentation to reflect new defaults and trace level
  3. Consider audit of existing debug logs - promote important ones to info level

Medium Priority

  1. Add usage examples in code comments or documentation
  2. Plan communication to teams about the behavior change
  3. Consider gradual rollout strategy if this impacts production debugging

Low Priority

  1. Future enhancement: Consider adding logger.isInfoEnabled() for consistency
  2. Performance: The helper methods add minimal overhead and are well-implemented

Verdict

Approve with required changes. The technical implementation is solid, but the behavioral change needs proper versioning and documentation updates before merge.

The infrastructure correctly enables the stated Phase 1 goals, but the immediate impact on existing debug logging should be communicated and documented.


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.

2 participants