Skip to content

Commit 1f1c6d4

Browse files
asithadeasithade
authored
refactor(logging): migrate to singleton logger service (#207)
* refactor(logging): migrate to singleton logger service - Created LoggerService singleton with WeakMap-based operation tracking - Prevents duplicate logs and memory leaks - CloudWatch-optimized structured JSON logging - Updated 10 controllers, 2 middleware, 7 services, 2 utils - Improved error handling with centralized error middleware logging - Removed redundant error logs before next(error) calls - Net reduction of 706 lines LFXV2-903 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]> * refactor(logging): extend logger service for infrastructure operations Extended LoggerService to support both request-scoped and infrastructure operations by making req parameter optional. Migrated all 41 direct serverLogger calls across services, utilities, and routes to use the unified logger service pattern. Changes: - Extended LoggerService methods to accept Request | undefined - Migrated 11 request-scoped operations to pass req parameter - Migrated 30 infrastructure operations to use logger with undefined - Updated ai.service, user.service, project.service method signatures - Replaced serverLogger calls in nats, snowflake, lock-manager services - Updated CLAUDE.md with comprehensive logging documentation - Removed serverLogger imports from all service/utility files Benefits: - Unified logging interface for all operations - Better request correlation when context available - Consistent error handling with err field - Infrastructure operations now use same pattern LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]> * docs(logging): update docs for infrastructure logging Updated logging-monitoring.md to reflect the new dual-mode logger service that supports both request-scoped and infrastructure operations. Added documentation for the new server-logger.ts file that breaks the circular dependency between server.ts and logger.service.ts. Changes: - Added logging architecture layers diagram showing server-logger.ts - Updated all method signatures to show req | undefined pattern - Added infrastructure logging examples for all methods - Documented circular dependency resolution - Clarified when to use request-scoped vs infrastructure logging LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]> * refactor(meetings): move agenda generation to controller Moved AI agenda generation logic from inline route handler to MeetingController.generateAgenda method for better separation of concerns and consistency with other route patterns. Also improved logger usage in middleware and services: - auth.middleware: better error tracking with startTime - error-handler.middleware: use operation startTime when available - access-check.service: move startOperation before try block - etag.service: use debug instead of silent startOperation - persona-helper: use success/error instead of warning LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]> * docs(logging): update architecture diagram for server-logger Updated CLAUDE.md logging architecture diagram to reflect the new server-logger.ts file that breaks the circular dependency between server.ts and logger.service.ts. LFXV2-903 Signed-off-by: Asitha de Silva <[email protected]> * fix: claude.md linting issues Signed-off-by: Asitha de Silva <[email protected]> * refactor(meetings): standardize generateAgenda error handling LFXV2-903 Use ServiceValidationError.fromFieldErrors() and next() for validation errors instead of manual 400 response, consistent with other controller methods. Provides field-level error details and centralized error handling through middleware. Signed-off-by: Asitha de Silva <[email protected]> * fix: linting issues for docs Signed-off-by: Asitha de Silva <[email protected]> --------- Signed-off-by: Asitha de Silva <[email protected]>
1 parent 1c6cf99 commit 1f1c6d4

37 files changed

+2458
-2915
lines changed

‎.vscode/settings.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"confirmdialog",
1111
"contentful",
1212
"Contentful",
13+
"customfield",
1314
"DATEADD",
1415
"daygrid",
1516
"dismissable",

‎CLAUDE.md‎

Lines changed: 219 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -98,26 +98,224 @@ lfx-v2-ui/
9898
- **Protected routes middleware** handles selective authentication logic
9999
- **Custom login handler** at `/login` with URL validation and secure redirects
100100
- Authentication is handled by Auth0/Authelia with express-openid-connect middleware
101-
- **Logging System**: Uses Pino for structured JSON logs with sensitive data redaction
102-
- **Logger Helper Pattern**: All controller functions must use Logger helper methods:
103-
- `Logger.start(req, 'operation_name', metadata)` - Returns startTime, logs at INFO
104-
- `Logger.success(req, 'operation_name', startTime, metadata)` - Logs at INFO
105-
- `Logger.error(req, 'operation_name', startTime, error, metadata)` - Logs at ERROR with 'err' field
106-
- `Logger.warning(req, 'operation_name', message, metadata)` - Logs at WARN
107-
- `Logger.validation(req, 'operation_name', errors, metadata)` - Logs at WARN
108-
- **Error Logging Standard**: Always use `err` field for errors to leverage Pino's error serializer
109-
- ✅ Correct: `req.log.error({ err: error, ...metadata }, 'message')`
110-
- ✅ Correct: `serverLogger.error({ err: error }, 'message')`
111-
- ✅ Correct: `Logger.error(req, 'operation', startTime, error, metadata)`
112-
- ❌ Incorrect: `{ error: error.message }` or `{ error: error instanceof Error ? error.message : error }`
113-
- Benefits: Complete stack traces (production/debug), clean single-line errors (development), proper AWS CloudWatch format
114-
- Custom serializer: `/server/helpers/error-serializer.ts` - excludes verbose stacks in dev, includes in prod
115-
- **Log Level Guidelines** (What to log at each level):
116-
- **INFO**: Business operation completions (created, updated, deleted), successful data retrieval (fetched, retrieved)
117-
- **WARN**: Error conditions leading to exceptions, data quality issues, user not found, fallback behaviors
118-
- **DEBUG**: Internal operations, preparation steps (sanitizing, creating payload), intent statements (resolving, fetching), NATS lookups
119-
- **ERROR**: System failures, unhandled exceptions, critical errors requiring immediate attention
120-
- **Filtered URLs**: Health checks (/health, /api/health) and /.well-known URLs are not logged to reduce noise
101+
102+
## Logging System
103+
104+
### Architecture Overview
105+
106+
- **Base Logger**: `serverLogger` created in `server.ts` - base Pino instance with all configuration
107+
- **HTTP Logger**: `pinoHttp` middleware uses `serverLogger` as base, creates `req.log` for each request
108+
- **Logger Service**: Singleton service (`logger.service.ts`) - unified interface for all application logging
109+
- **Format**: Structured JSON logs with Pino for AWS CloudWatch compatibility
110+
111+
### Logger Service Pattern (Primary Interface)
112+
113+
**Import and Usage:**
114+
115+
```typescript
116+
import { logger } from './logger.service';
117+
118+
// In controllers/routes with request context:
119+
const startTime = logger.startOperation(req, 'operation_name', metadata);
120+
logger.success(req, 'operation_name', startTime, metadata);
121+
122+
// In services/utilities without request context:
123+
const startTime = logger.startOperation(undefined, 'nats_connect', metadata);
124+
logger.success(undefined, 'nats_connect', startTime, metadata);
125+
```
126+
127+
**Available Methods:**
128+
129+
- `logger.startOperation(req|undefined, 'operation', metadata, options?)` → Returns startTime, logs at INFO (silent option available)
130+
- `logger.success(req|undefined, 'operation', startTime, metadata)` → Logs at INFO with duration
131+
- `logger.error(req|undefined, 'operation', startTime, error, metadata, options?)` → Logs at ERROR with 'err' field
132+
- `logger.warning(req|undefined, 'operation', message, metadata)` → Logs at WARN
133+
- `logger.validation(req|undefined, 'operation', errors[], metadata)` → Logs at ERROR with validation details
134+
- `logger.debug(req|undefined, 'operation', message, metadata)` → Logs at DEBUG
135+
- `logger.etag(req|undefined, 'operation', resourceType, resourceId, etag?, metadata)` → Logs ETag operations
136+
137+
**When to Use Each Pattern:**
138+
139+
- **Request-scoped** (pass `req`): Controllers, route handlers, service methods called from routes
140+
- **Infrastructure** (pass `undefined`): NATS connections, Snowflake pool, server startup/shutdown, scheduled jobs
141+
142+
### Error Logging Standard
143+
144+
**Always use `err` field** for proper error serialization:
145+
146+
```typescript
147+
// ✅ CORRECT
148+
logger.error(req, 'operation', startTime, error, metadata);
149+
logger.error(undefined, 'operation', startTime, error, metadata);
150+
151+
// ❌ INCORRECT
152+
serverLogger.error({ error: error.message }, 'message'); // Don't use serverLogger directly
153+
req.log.error({ error: error }, 'message'); // Should use logger service
154+
{
155+
error: error.message;
156+
} // Loses stack trace
157+
```
158+
159+
**Benefits:**
160+
161+
- Complete stack traces in production/debug
162+
- Clean single-line errors in development
163+
- Proper AWS CloudWatch format
164+
- Custom serializer: `/server/helpers/error-serializer.ts`
165+
166+
### Log Level Guidelines
167+
168+
**INFO** - Business operation completions:
169+
170+
- Successful data operations (created, updated, deleted, fetched)
171+
- Important state changes
172+
- Operation success with duration
173+
174+
**WARN** - Recoverable issues:
175+
176+
- Error conditions leading to exceptions
177+
- Data quality issues, user not found
178+
- Fallback behaviors, NATS failures with graceful handling
179+
180+
**DEBUG** - Internal operations:
181+
182+
- Preparation steps (sanitizing, creating payload)
183+
- Internal lookups (NATS, database queries)
184+
- Intent statements (resolving, fetching)
185+
- Infrastructure operations (connections, pool state)
186+
187+
**ERROR** - Critical failures:
188+
189+
- System failures, unhandled exceptions
190+
- Critical errors requiring immediate attention
191+
- Operations that cannot continue
192+
193+
### Features
194+
195+
- **Deduplication**: Prevents duplicate logs for same operation (request-scoped only)
196+
- **Duration Tracking**: Automatic calculation from startTime to completion
197+
- **Request Correlation**: `request_id` field for tracing (when req provided)
198+
- **Sensitive Data Redaction**: Automatic redaction of tokens, auth headers, cookies
199+
- **AWS Trace ID**: Automatic capture from Lambda environment
200+
- **Filtered URLs**: Health checks (`/health`, `/api/health`) and `/.well-known` not logged
201+
202+
### Logging Architecture
203+
204+
```text
205+
server-logger.ts (breaks circular dependency)
206+
└─ Creates and exports serverLogger (base Pino instance)
207+
└─ Configuration: levels, serializers, formatters, redaction
208+
209+
server.ts
210+
├─ Imports serverLogger from server-logger.ts
211+
└─ Creates httpLogger (pinoHttp middleware)
212+
└─ Uses serverLogger as base
213+
└─ Attaches req.log to each request
214+
215+
logger.service.ts
216+
├─ Imports serverLogger from server-logger.ts
217+
└─ Singleton logger service
218+
├─ Request-scoped: uses req.log when req provided
219+
├─ Infrastructure: uses serverLogger when req = undefined
220+
└─ Provides unified API for all logging
221+
```
222+
223+
**Direct serverLogger Usage:**
224+
225+
- ❌ **Never** call `serverLogger` directly from services/routes/controllers
226+
- ✅ **Always** use `logger` service methods
227+
- ℹ️ `serverLogger` only exists in `server-logger.ts` (created), `server.ts` (imported), and `logger.service.ts` (imported)
228+
229+
### Common Logging Patterns
230+
231+
**Controller/Route Handler Pattern:**
232+
233+
```typescript
234+
export const getUser = async (req: Request, res: Response, next: NextFunction) => {
235+
const startTime = logger.startOperation(req, 'get_user', { userId: req.params.id });
236+
237+
try {
238+
const user = await userService.getUserById(req, req.params.id);
239+
240+
logger.success(req, 'get_user', startTime, { userId: user.id });
241+
return res.json(user);
242+
} catch (error) {
243+
logger.error(req, 'get_user', startTime, error, { userId: req.params.id });
244+
return next(error);
245+
}
246+
};
247+
```
248+
249+
**Service Method with Request Context:**
250+
251+
```typescript
252+
public async updateUser(req: Request, userId: string, data: UpdateData): Promise<User> {
253+
const startTime = logger.startOperation(req, 'update_user', { userId });
254+
255+
try {
256+
// Validation
257+
if (!data.email) {
258+
logger.validation(req, 'update_user', ['Email required'], { userId });
259+
throw new ValidationError('Email required');
260+
}
261+
262+
const user = await this.performUpdate(userId, data);
263+
logger.success(req, 'update_user', startTime, { userId, updatedFields: Object.keys(data) });
264+
return user;
265+
} catch (error) {
266+
logger.error(req, 'update_user', startTime, error, { userId });
267+
throw error;
268+
}
269+
}
270+
```
271+
272+
**Infrastructure Operation (No Request Context):**
273+
274+
```typescript
275+
public async connect(): Promise<void> {
276+
const startTime = logger.startOperation(undefined, 'db_connect', { host: this.config.host });
277+
278+
try {
279+
await this.pool.connect();
280+
logger.success(undefined, 'db_connect', startTime, { poolSize: this.pool.size });
281+
} catch (error) {
282+
logger.error(undefined, 'db_connect', startTime, error, { host: this.config.host });
283+
throw error;
284+
}
285+
}
286+
```
287+
288+
**Internal Service Operation (Called from method with req):**
289+
290+
```typescript
291+
// Parent method has req
292+
public async getProjectBySlug(req: Request, slug: string): Promise<Project> {
293+
return this.fetchFromNats(req, slug); // Pass req down
294+
}
295+
296+
// Internal method receives req for logging correlation
297+
private async fetchFromNats(req: Request, slug: string): Promise<Project> {
298+
logger.debug(req, 'fetch_from_nats', 'Fetching project from NATS', { slug });
299+
// ... implementation
300+
}
301+
```
302+
303+
### Logging Checklist
304+
305+
**Before logging:**
306+
307+
- [ ] Using `logger` service, not `serverLogger` directly?
308+
- [ ] Passing `req` if available, `undefined` if infrastructure?
309+
- [ ] Using `err` field for errors (not `error`)?
310+
- [ ] Appropriate log level (INFO/WARN/DEBUG/ERROR)?
311+
- [ ] Operation name in snake_case?
312+
- [ ] Sensitive data sanitized?
313+
314+
**For operations with duration:**
315+
316+
- [ ] Called `logger.startOperation()` and captured `startTime`?
317+
- [ ] Calling `logger.success()` or `logger.error()` with `startTime`?
318+
- [ ] Passing relevant metadata for debugging?
121319
- All shared types, interfaces, and constants are centralized in @lfx-one/shared package
122320
- **AI Service Integration**: Claude Sonnet 4 model via LiteLLM proxy for meeting agenda generation
123321
- **AI Environment Variables**: AI_PROXY_URL and AI_API_KEY required for AI functionality
@@ -128,7 +326,7 @@ lfx-v2-ui/
128326
- **License headers are required on all source files** - run `./check-headers.sh` to verify
129327
- **Pre-commit hooks enforce license headers** - commits will fail without proper headers
130328
- Always run yarn format from the root of the project to ensure formatting is done after you have made all your changes
131-
- Always preprend "Generated with [Claude Code](https://claude.ai/code)" if you assisted with the code
329+
- Always prepend "Generated with [Claude Code](https://claude.ai/code)" if you assisted with the code
132330
- Do not nest ternary expressions
133331
- Always run yarn lint before yarn build to validate your linting
134332
- The JIRA project key for this is LFXV2. All tickets associated to this repo should generally be in there.

‎apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ export class MeetingCardComponent implements OnInit {
473473

474474
private initAttachments(): Signal<MeetingAttachment[]> {
475475
return runInInjectionContext(this.injector, () => {
476-
return toSignal(this.meetingService.getMeetingAttachments(this.meeting().uid).pipe(catchError(() => of([]))), {
476+
return toSignal(this.meetingService.getMeetingAttachments(this.meetingInput().uid).pipe(catchError(() => of([]))), {
477477
initialValue: [],
478478
});
479479
});
@@ -482,7 +482,7 @@ export class MeetingCardComponent implements OnInit {
482482
private initRecording(): void {
483483
runInInjectionContext(this.injector, () => {
484484
toSignal(
485-
this.meetingService.getPastMeetingRecording(this.meeting().uid).pipe(
485+
this.meetingService.getPastMeetingRecording(this.meetingInput().uid).pipe(
486486
catchError(() => of(null)),
487487
tap((recording) => this.recording.set(recording))
488488
),
@@ -494,7 +494,7 @@ export class MeetingCardComponent implements OnInit {
494494
private initSummary(): void {
495495
runInInjectionContext(this.injector, () => {
496496
toSignal(
497-
this.meetingService.getPastMeetingSummary(this.meeting().uid).pipe(
497+
this.meetingService.getPastMeetingSummary(this.meetingInput().uid).pipe(
498498
catchError(() => of(null)),
499499
tap((summary) => this.summary.set(summary))
500500
),

0 commit comments

Comments
 (0)