Skip to content

feat: add LOG_LEVEL environment variable#2124

Merged
Yeraze merged 3 commits intomainfrom
feat/log-level-env-var
Mar 4, 2026
Merged

feat: add LOG_LEVEL environment variable#2124
Yeraze merged 3 commits intomainfrom
feat/log-level-env-var

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Mar 4, 2026

Summary

  • Adds LOG_LEVEL env var (debug, info, warn, error) to control log verbosity independently of NODE_ENV
  • Logger gates all methods (debug, info, warn, error) based on the configured level
  • Falls back to current behavior when LOG_LEVEL is not set (dev → debug, prod → info)

Changes

  • src/utils/logger.ts — Reads LOG_LEVEL from process.env, gates each log method via shouldLog()
  • src/server/config/environment.ts — Adds logLevel/logLevelProvided to EnvironmentConfig for discoverability
  • .env.example / .env.production.example — Documents the new variable

Test plan

  • npx vitest run — all 2903 tests pass
  • Manual: set LOG_LEVEL=debug with NODE_ENV=production, confirm [DEBUG] messages appear
  • Manual: set LOG_LEVEL=error, confirm only [ERROR] messages appear
  • Manual: unset LOG_LEVEL, confirm existing NODE_ENV behavior unchanged

🤖 Generated with Claude Code

Allows users to set debug/info/warn/error log levels independently of
NODE_ENV, enabling debug logging in production Docker deployments
without side effects like changed rate limits or cookie warnings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

Claude finished @Yeraze's task —— View job


PR Review: LOG_LEVEL Environment Variable

Todo List:

  • Read and analyze all changed files
  • Review logger.ts implementation for correctness
  • Check environment configuration handling
  • Assess performance implications
  • Review security considerations
  • Evaluate test coverage
  • Provide comprehensive feedback

Code Quality & Best Practices

Excellent implementation overall! The code follows established patterns in the codebase:

  1. Architecture Consistency - The integration with environment.ts follows the existing centralized configuration pattern and uses the same parseEnum utility used for other enum-based environment variables.

  2. Type Safety - Strong TypeScript typing with proper union types for log levels and comprehensive interface definitions.

  3. Documentation - Excellent JSDoc comments in logger.ts explaining usage patterns and fallback behavior.

  4. Naming Conventions - Follows existing patterns (logLevel/logLevelProvided in environment config).

🐛 Potential Issues Found

1. Performance Consideration - Log Level Evaluation

The current implementation evaluates shouldLog() on every log call, which involves string lookups in the LOG_LEVEL_ORDER array:

// src/utils/logger.ts:42-46
debug: (...args: any[]) => {
  if (shouldLog('debug')) {  // indexOf() called every time
    console.log('[DEBUG]', ...args);
  }
}

Impact: Minimal for most applications, but could add up in high-frequency logging scenarios.

Recommendation: Consider caching level checks:

const currentLevelIndex = LOG_LEVEL_ORDER.indexOf(currentLevel);
const shouldLogDebug = 0 >= currentLevelIndex;
const shouldLogInfo = 1 >= currentLevelIndex;
// etc.

2. Potential Initialization Timing Issue

The logger evaluates process.env.LOG_LEVEL at module load time (line 31), but the environment config also parses it. If someone were to change process.env.LOG_LEVEL after import, the logger wouldn't reflect the change.

Impact: Low - environment variables are typically set at startup.

Current behavior: Acceptable for normal use cases.

3. Inconsistent Case Handling

The logger uses .toLowerCase() on process.env.LOG_LEVEL (line 22), while environment.ts also calls .toLowerCase() (line 570). This is redundant but harmless.

🔒 Security Analysis

No security concerns identified

  • Input validation is proper (checks against allowed values)
  • No injection vulnerabilities
  • Fallback behavior is safe
  • No sensitive information exposure

🏎️ Performance Assessment

Performance impact is minimal

  • Log level evaluation adds ~2-3 operations per log call
  • Environment parsing happens once at startup
  • No memory leaks or resource issues identified

🧪 Test Coverage Analysis

⚠️ Test coverage gap identified

The implementation lacks unit tests for the new LOG_LEVEL functionality. Current testing:

  • ✅ All existing tests pass (2903 tests)
  • ✅ Integration testing planned in test plan
  • ❌ No unit tests for logger level filtering
  • ❌ No unit tests for environment variable parsing

Recommendation: Add unit tests for:

// Example tests needed:
describe('logger with LOG_LEVEL', () => {
  it('should respect LOG_LEVEL=error and only show errors')
  it('should fall back to NODE_ENV behavior when LOG_LEVEL unset')
  it('should handle invalid LOG_LEVEL values gracefully')
})

🔍 Additional Observations

Positive Design Decisions

  1. Backward Compatibility - Falls back to existing NODE_ENV behavior perfectly
  2. Environment Integration - Proper integration with centralized config system
  3. Consistent Formatting - Maintains existing log format with level prefixes
  4. Clear Documentation - Both code comments and .env examples are excellent

Minor Enhancement Opportunities

  1. Error Logging Exception: Currently even error logs check shouldLog('error'). Since errors should typically always be shown, consider making error logs unconditional.

  2. Development Experience: The current design makes it easy to accidentally suppress all logs with LOG_LEVEL=error. Consider adding a warning when debug logs are disabled in development mode.

📋 Summary

This is a high-quality implementation that properly addresses the requirement. The code is well-structured, follows established patterns, and includes good documentation.

Strengths:

  • ✅ Excellent backward compatibility
  • ✅ Proper TypeScript typing
  • ✅ Good integration with existing systems
  • ✅ Clear documentation

Recommendations for follow-up:

  1. Add unit tests for the new functionality
  2. Consider performance optimization for high-frequency scenarios (optional)
  3. Consider whether error logs should always be shown regardless of level

Approval Status: ✅ Approved - This PR is ready to merge after addressing the test coverage gap.


Covers all log levels, case insensitivity, NODE_ENV fallback,
and invalid value handling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

Claude finished @Yeraze's task —— View job


PR Review: LOG_LEVEL Environment Variable Implementation

Todo List:

  • Read and analyze all changed files
  • Review logger.ts implementation for correctness
  • Check new test coverage in logger.test.ts
  • Review environment configuration changes
  • Assess performance and security considerations
  • Provide comprehensive feedback

Outstanding Implementation!

This is an excellent implementation that addresses the previous review feedback comprehensively. The addition of comprehensive unit tests and the clean, well-documented code make this ready for production.

🎯 Key Improvements Since Last Review

Excellent Test Coverage Added - The new logger.test.ts file provides comprehensive unit tests that cover:

  • ✅ All log level combinations (debug, info, warn, error)
  • ✅ Case insensitivity (LOG_LEVEL=ERROR works correctly)
  • ✅ Fallback behavior when LOG_LEVEL is unset
  • ✅ Invalid value handling with graceful fallback
  • ✅ Proper module reloading to test environment changes

Smart Test Design - The tests use vi.resetModules() and dynamic imports to properly test module-level initialization behavior, which is exactly right for testing environment variable parsing.

🔍 Detailed Code Analysis

Logger Implementation (src/utils/logger.ts)

  • Architecture: Clean separation of concerns with getLogLevel(), shouldLog(), and individual log methods
  • Type Safety: Proper TypeScript types with LogLevel union and read-only arrays
  • Documentation: Excellent JSDoc comments explaining usage patterns
  • Performance: Log level evaluation is efficient with simple array lookups
  • Backward Compatibility: Perfect fallback to existing NODE_ENV behavior

Environment Integration (src/server/config/environment.ts)

  • Consistency: Uses the existing parseEnum utility following established patterns
  • Validation: Proper input validation with case normalization (lines 570)
  • Default Logic: Correct fallback logic matching logger behavior (development → debug, production → info)
  • Documentation: Well-documented in both interface and implementation

Configuration Examples

  • .env.example: Clear documentation with appropriate placement in logging section (lines 127-132)
  • .env.production.example: Concise production-focused documentation (lines 21-23)

🚀 Performance Assessment

Minimal Impact:

  • Log level check is O(1) with indexOf() on a 4-element array
  • Environment variable parsing happens once at startup
  • No memory leaks or performance concerns

🔒 Security Analysis

Secure Implementation:

  • ✅ Input validation prevents injection attacks
  • ✅ Graceful handling of invalid values
  • ✅ No sensitive information exposure
  • ✅ Safe fallback behavior

🧪 Test Coverage Excellence

The test suite is comprehensive and well-designed:

  • All Path Coverage: Tests every log level combination and fallback scenario
  • Edge Cases: Invalid values, case sensitivity, missing environment variables
  • Isolation: Proper environment variable cleanup and module reloading
  • Maintainable: Clean, readable test structure with good assertions

Example of excellent test design:

// Tests module reloading to verify environment changes take effect
async function importLogger() {
  const mod = await import('./logger.js');
  return mod.logger;
}

🎯 Design Decisions (All Excellent)

  1. Log Level Hierarchy: Standard debug < info < warn < error ordering
  2. Case Insensitivity: User-friendly with .toLowerCase() normalization
  3. Fallback Strategy: Maintains existing behavior when LOG_LEVEL unset
  4. Error Logs: Correctly subject to level filtering (some loggers make errors always visible, but this is consistent)
  5. Environment Integration: Properly integrated with centralized config system

📋 Minor Observations

All Design Choices Are Appropriate:

  • Error logs being filtered by level is consistent with the design goal
  • Performance is excellent for typical usage patterns
  • The implementation is simple and maintainable

🏆 Summary

This implementation demonstrates excellent software engineering practices:

Strengths:

  • ✅ Comprehensive test coverage addressing previous feedback
  • ✅ Clean, readable, and well-documented code
  • ✅ Perfect backward compatibility
  • ✅ Proper TypeScript typing throughout
  • ✅ Follows existing codebase patterns and conventions
  • ✅ Minimal performance impact
  • ✅ Secure implementation with proper validation

No issues found - This code is production-ready and well-tested.

Recommendation: ✅ Approved for merge - Excellent work addressing all previous feedback!


Adds a Logging Variables table in the environment variables section
and expands the Logging section with level descriptions and a
production troubleshooting tip.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

Claude finished @Yeraze's task —— View job


PR Review: LOG_LEVEL Environment Variable Implementation

Todo List:

  • Read and analyze all changed files
  • Review logger.ts implementation for correctness
  • Check test coverage and quality
  • Assess performance implications
  • Review security considerations
  • Evaluate code quality and best practices
  • Provide comprehensive feedback

Excellent Implementation - Approved

This is a high-quality, production-ready implementation that demonstrates excellent software engineering practices. The LOG_LEVEL environment variable feature is well-designed, thoroughly tested, and follows all established codebase patterns.

🎯 Implementation Highlights

🔧 Clean Architecture

  • Logger Implementation (src/utils/logger.ts:21-35): Proper separation of concerns with getLogLevel(), shouldLog(), and level-gated log methods
  • Environment Integration (src/server/config/environment.ts:570): Uses existing parseEnum utility following established patterns
  • Type Safety: Strong TypeScript typing with proper union types and read-only arrays

📚 Excellent Documentation

  • JSDoc Comments (src/utils/logger.ts:1-16): Comprehensive usage documentation explaining fallback behavior
  • Configuration Examples: Clear documentation in both .env.example and .env.production.example
  • Architecture Docs (docs/configuration/index.md:163-169): Well-integrated into existing documentation

🧪 Comprehensive Test Coverage
The test suite in src/utils/logger.test.ts is exemplary:

  • All Log Level Combinations: Tests debug, info, warn, error filtering (lines 29-85)
  • Case Insensitivity: Validates LOG_LEVEL=ERROR works correctly (lines 87-96)
  • Fallback Behavior: Tests NODE_ENV fallback when LOG_LEVEL unset (lines 98-118)
  • Invalid Values: Graceful handling of invalid LOG_LEVEL values (lines 120-130)
  • Smart Module Testing: Uses vi.resetModules() and dynamic imports to properly test module initialization

🔍 Technical Analysis

✅ Performance - Excellent

  • Log level evaluation is O(1) with simple array indexOf() on 4-element array
  • Environment parsing happens once at module load
  • Minimal overhead per log call (~2-3 operations)

✅ Security - No Issues

  • Proper input validation with case normalization
  • Graceful fallback for invalid values prevents injection
  • No sensitive information exposure

✅ Backward Compatibility - Perfect

  • Falls back to existing NODE_ENV behavior when LOG_LEVEL unset
  • Maintains existing log format with level prefixes
  • No breaking changes to existing functionality

✅ Code Quality - Outstanding

  • Follows existing codebase patterns and naming conventions
  • Proper error handling and validation
  • Clear, maintainable code structure
  • Consistent with centralized environment configuration approach

🎨 Design Decisions (All Excellent)

  1. Log Level Hierarchy: Standard debug < info < warn < error ordering
  2. Case Insensitivity: User-friendly with automatic normalization
  3. Environment Variable Name: LOG_LEVEL is clear and conventional
  4. Fallback Strategy: Maintains backward compatibility perfectly
  5. Error Logs Subject to Filtering: Consistent design (some loggers make errors always visible, but this approach is more predictable)

📋 Specific Code Review Notes

src/utils/logger.ts

  • Line 22: Perfect case normalization with ?.toLowerCase()
  • Line 23: Proper type checking with includes() and type assertion
  • Lines 31-35: Efficient level comparison using array indices
  • Lines 42-76: Well-documented log methods with clear JSDoc

src/server/config/environment.ts

  • Line 570: Correct use of existing parseEnum utility
  • Lines 272-273: Proper TypeScript interface integration
  • Line 569: Good default logic matching logger behavior

src/utils/logger.test.ts

  • Lines 20-27: Smart test setup with module reset for proper isolation
  • Lines 29-85: Comprehensive coverage of all level combinations
  • Lines 87-96: Important case sensitivity test
  • Lines 98-130: Thorough fallback and error case testing

🚀 No Issues Found

After thorough analysis, I found no bugs, security issues, or design problems. This implementation is:

  • Production Ready: Safe for immediate deployment
  • Well Tested: Comprehensive unit test coverage
  • Documented: Clear usage examples and documentation
  • Performant: Minimal overhead
  • Secure: Proper input validation
  • Maintainable: Clean, readable code

🏆 Recommendation: APPROVE

This PR represents excellent software engineering and should be merged. The implementation addresses the requirement perfectly while maintaining code quality, test coverage, and documentation standards.

Great work on addressing the logging verbosity need while maintaining backward compatibility! 🎉


Job run: https://github.com/Yeraze/meshmonitor/actions/runs/22673148551

@Yeraze Yeraze merged commit 99c8bde into main Mar 4, 2026
13 checks passed
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