-
-
Notifications
You must be signed in to change notification settings - Fork 0
Logger Consistency, Injection #725
Copy link
Copy link
Open
Description
Logger Injection Strategy
Problem Statement
The codebase contains 59+ console statements across packages that are causing ESLint violations and blocking CI/CD pipeline. The @vue-skuilder/db package is used in multiple contexts:
- Browser context (platform-ui package)
- Node.js context (express package - already has Winston logger)
- Testing context
Current console usage includes:
- Database operation debugging
- Error reporting
- Development tracing
- Status updates
Core Issue: Direct console usage creates tight coupling and prevents environment-appropriate logging.
Proposed Solution: Logger Injection Pattern
Architecture Overview
Instead of the DB package owning logging implementation, inject a logger interface that each runtime environment can implement appropriately.
// packages/common/src/logger.ts
export interface Logger {
debug(message: string, ...args: any[]): void;
info(message: string, ...args: any[]): void;
warn(message: string, ...args: any[]): void;
error(message: string, ...args: any[]): void;
}Implementation Strategy
1. DB Package (Consumer)
// Before
class CourseDB {
async getCards() {
console.log('Fetching cards...');
// ... implementation
}
}
// After
class CourseDB {
constructor(private logger: Logger) {}
async getCards() {
this.logger.debug('Fetching cards...');
// ... implementation
}
}2. Express Package (Winston Provider)
// Use existing Winston logger
import { winstonLogger } from './existing-winston-config';
import { CourseDB } from '@vue-skuilder/db';
const courseDB = new CourseDB(winstonLogger);3. Platform-UI Package (Browser Provider)
// Browser-compatible logger implementation
const browserLogger: Logger = {
debug: process.env.NODE_ENV === 'development' ? console.debug : () => {}, // eslint-disable-line no-console
info: console.info, // eslint-disable-line no-console
warn: console.warn, // eslint-disable-line no-console
error: console.error // eslint-disable-line no-console
};
const courseDB = new CourseDB(browserLogger);4. Testing Context (Mock Provider)
const mockLogger: Logger = {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn()
};
const courseDB = new CourseDB(mockLogger);Benefits
Technical Benefits
- ✅ Leverages existing Winston in express package
- ✅ Zero console statements in DB package (no ESLint violations)
- ✅ Environment-appropriate logging (Winston vs console vs mock)
- ✅ Testable with mock loggers
- ✅ Follows SOLID principles (dependency inversion)
Architectural Benefits
- ✅ Clean separation of concerns
- ✅ Runtime flexibility - each context chooses appropriate logging
- ✅ Future-proof - easy to swap logging implementations
- ✅ No competing logging systems
Operational Benefits
- ✅ Immediate CI unblocking
- ✅ Consistent with existing infrastructure
- ✅ Minimal ESLint suppressions (only in browser logger utility)
Implementation Plan
Phase 1: Core Implementation (4-6 hours)
- Create Logger interface in
packages/common/src/logger.ts - Create browser logger utility in common package
- Refactor DB classes to accept logger in constructors
- Replace console statements with
this.logger.method()calls - Update express package to inject existing Winston logger
- Update platform-ui package to inject browser logger
Phase 2: Testing & Validation (1-2 hours)
- Create mock logger for testing
- Update existing tests to use mock logger
- Verify CI pipeline passes with changes
- Validate logging output in both browser and Node.js contexts
Phase 3: Documentation (30 minutes)
- Update README with logger injection usage
- Document logger interface for future developers
- Add examples for each runtime context
Migration Examples
CourseDB Migration
// Current problematic code
export class CourseDB {
async init() {
console.log('Initializing course database...');
try {
// database initialization
console.log('Course database initialized successfully');
} catch (error) {
console.error('Failed to initialize course database:', error);
throw error;
}
}
}
// Migrated code
export class CourseDB {
constructor(private logger: Logger) {}
async init() {
this.logger.info('Initializing course database...');
try {
// database initialization
this.logger.info('Course database initialized successfully');
} catch (error) {
this.logger.error('Failed to initialize course database:', error);
throw error;
}
}
}PouchDataLayerProvider Migration
// Current problematic code
export class PouchDataLayerProvider {
constructor() {
console.log('PouchDataLayerProvider: Running in Node.js environment');
// ... rest of constructor
}
}
// Migrated code
export class PouchDataLayerProvider {
constructor(private logger: Logger) {
this.logger.debug('PouchDataLayerProvider: Running in Node.js environment');
// ... rest of constructor
}
}ESLint Strategy
- DB Package: Zero console statements = zero ESLint violations
- Common Package: Browser logger utility with local ESLint suppressions
- Express Package: Uses Winston logger (no console statements)
- Platform-UI Package: Imports browser logger from common
Total ESLint suppressions needed: ~4 lines (only in browser logger utility)
Risks & Mitigations
Risk: Constructor Breaking Changes
- Mitigation: Use factory methods where possible to minimize breaking changes
- Mitigation: Provide default logger implementations for backward compatibility
Risk: Implementation Complexity
- Mitigation: Start with simple logger interface, extend as needed
- Mitigation: Provide clear examples and documentation
Risk: Performance Impact
- Mitigation: Logger interface calls have negligible overhead
- Mitigation: Browser logger can be no-op in production
Success Criteria
- CI pipeline passes without console-related ESLint violations
- Express package uses Winston logger for DB operations
- Browser context uses console wrapper for DB operations
- Tests can inject mock loggers for isolated testing
- Zero console statements remain in DB package
- Existing functionality preserved across all contexts
Future Enhancements
Short Term
- Add log level configuration per environment
- Implement structured logging in browser context
- Add log filtering capabilities
Long Term
- Consider log aggregation for browser context
- Evaluate remote logging endpoints for error reporting
- Implement log rotation policies for file-based logging
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels