Skip to content

Commit 9c48425

Browse files
Cleanup debug logs and implement unified logger (#109)
* Implement unified logging system - Create shared logger package with structured logging - Integrate logger into sandbox container and DO layer - Remove duplicate and unnecessary console.log statements - Add trace context support for request tracking - Update logging configuration and middleware * Delete dead code
1 parent 485cf61 commit 9c48425

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+1671
-1386
lines changed

packages/sandbox-container/src/config.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
/**
2+
* Log level for the sandbox container.
3+
*
4+
* Default: info
5+
* Environment variable: SANDBOX_LOG_LEVEL
6+
*/
7+
const SANDBOX_LOG_LEVEL = process.env.SANDBOX_LOG_LEVEL || 'info';
8+
9+
/**
10+
* Log format for the sandbox container.
11+
*
12+
* Default: json
13+
* Set to "pretty" to enable colored, human-readable output.
14+
* Environment variable: SANDBOX_LOG_FORMAT
15+
*/
16+
const SANDBOX_LOG_FORMAT = process.env.SANDBOX_LOG_FORMAT || 'json';
17+
118
/**
219
* How long to wait for an interpreter process to spawn and become ready.
320
* If an interpreter doesn't start within this time, something is fundamentally
@@ -65,6 +82,8 @@ const STREAM_CHUNK_DELAY_MS = 100;
6582
const DEFAULT_CWD = '/workspace';
6683

6784
export const CONFIG = {
85+
SANDBOX_LOG_LEVEL,
86+
SANDBOX_LOG_FORMAT,
6887
INTERPRETER_SPAWN_TIMEOUT_MS,
6988
INTERPRETER_EXECUTION_TIMEOUT_MS,
7089
COMMAND_TIMEOUT_MS,

packages/sandbox-container/src/core/container.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { Logger } from '@repo/shared';
2+
import { createLogger } from '@repo/shared';
13
import { ExecuteHandler } from '../handlers/execute-handler';
24
import { FileHandler } from '../handlers/file-handler';
35
import { GitHandler } from '../handlers/git-handler';
@@ -17,8 +19,6 @@ import { InMemoryPortStore, PortService } from '../services/port-service';
1719
import { InMemoryProcessStore, ProcessService } from '../services/process-service';
1820
import { SessionManager } from '../services/session-manager';
1921
import { RequestValidator } from '../validation/request-validator';
20-
import { ConsoleLogger } from './logger';
21-
import type { Logger } from './types';
2222

2323
export interface Dependencies {
2424
// Services
@@ -76,7 +76,7 @@ export class Container {
7676
}
7777

7878
// Initialize infrastructure
79-
const logger = new ConsoleLogger();
79+
const logger = createLogger({ component: 'container' });
8080
const security = new SecurityService(logger);
8181
const securityAdapter = new SecurityServiceAdapter(security);
8282
const validator = new RequestValidator();
@@ -85,10 +85,8 @@ export class Container {
8585
const processStore = new InMemoryProcessStore();
8686
const portStore = new InMemoryPortStore();
8787

88-
// Initialize SessionManager (always - no test-specific conditionals)
89-
console.log('[Container] Initializing SessionManager');
88+
// Initialize SessionManager
9089
const sessionManager = new SessionManager(logger);
91-
console.log('[Container] SessionManager created');
9290

9391
// Initialize services
9492
const processService = new ProcessService(processStore, logger, sessionManager);
@@ -146,9 +144,4 @@ export class Container {
146144
isInitialized(): boolean {
147145
return this.initialized;
148146
}
149-
150-
// Helper method to get all dependencies (for testing)
151-
getAllDependencies(): Partial<Dependencies> {
152-
return { ...this.dependencies };
153-
}
154147
}

packages/sandbox-container/src/core/logger.ts

Lines changed: 0 additions & 32 deletions
This file was deleted.

packages/sandbox-container/src/core/router.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Centralized Router for handling HTTP requests
22

3+
import type { Logger } from '@repo/shared';
34
import type { ErrorResponse } from '@repo/shared/errors';
45
import { ErrorCode } from '@repo/shared/errors';
56
import type {
@@ -14,6 +15,11 @@ import type {
1415
export class Router {
1516
private routes: RouteDefinition[] = [];
1617
private globalMiddleware: Middleware[] = [];
18+
private logger: Logger;
19+
20+
constructor(logger: Logger) {
21+
this.logger = logger;
22+
}
1723

1824
/**
1925
* Register a route with optional middleware
@@ -43,14 +49,12 @@ export class Router {
4349
async route(request: Request): Promise<Response> {
4450
const method = this.validateHttpMethod(request.method);
4551
const pathname = new URL(request.url).pathname;
46-
47-
console.log(`[Router] Routing ${method} ${pathname}`);
4852

4953
// Find matching route
5054
const route = this.matchRoute(method, pathname);
51-
55+
5256
if (!route) {
53-
console.log(`[Router] No route found for ${method} ${pathname}`);
57+
this.logger.debug('No route found', { method, pathname });
5458
return this.createNotFoundResponse();
5559
}
5660

@@ -74,7 +78,11 @@ export class Router {
7478
route.handler
7579
);
7680
} catch (error) {
77-
console.error(`[Router] Error handling ${method} ${pathname}:`, error);
81+
this.logger.error('Error handling request', error as Error, {
82+
method,
83+
pathname,
84+
requestId: context.requestId
85+
});
7886
return this.createErrorResponse(error instanceof Error ? error : new Error('Unknown error'));
7987
}
8088
}

packages/sandbox-container/src/core/types.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,6 @@ export type RequestHandler = (
201201
context: RequestContext
202202
) => Promise<Response>;
203203

204-
// Logger interface
205-
export interface Logger {
206-
info(message: string, meta?: Record<string, unknown>): void;
207-
warn(message: string, meta?: Record<string, unknown>): void;
208-
error(message: string, error?: Error, meta?: Record<string, unknown>): void;
209-
debug(message: string, meta?: Record<string, unknown>): void;
210-
}
211-
212204
// Session types
213205
export interface SessionData {
214206
id: string;

packages/sandbox-container/src/handlers/base-handler.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { Logger } from '@repo/shared';
2+
import { TraceContext } from '@repo/shared';
13
import {
24
type ErrorCode,
35
type ErrorResponse,
@@ -7,7 +9,6 @@ import {
79
} from "@repo/shared/errors";
810
import type {
911
Handler,
10-
Logger,
1112
RequestContext,
1213
ServiceError,
1314
} from "../core/types";
@@ -103,4 +104,32 @@ export abstract class BaseHandler<TRequest, TResponse>
103104
const url = new URL(request.url);
104105
return url.searchParams.get(param);
105106
}
107+
108+
/**
109+
* Extract traceId from request headers
110+
* Returns the traceId if present, null otherwise
111+
*/
112+
protected extractTraceId(request: Request): string | null {
113+
return TraceContext.fromHeaders(request.headers);
114+
}
115+
116+
/**
117+
* Create a child logger with trace context for this request
118+
* Includes traceId if available in request headers
119+
*/
120+
protected createRequestLogger(request: Request, operation?: string): Logger {
121+
const traceId = this.extractTraceId(request);
122+
const context: Record<string, string> = {};
123+
124+
if (traceId) {
125+
context.traceId = traceId;
126+
}
127+
if (operation) {
128+
context.operation = operation;
129+
}
130+
131+
return context.traceId || context.operation
132+
? this.logger.child(context)
133+
: this.logger;
134+
}
106135
}

packages/sandbox-container/src/handlers/execute-handler.ts

Lines changed: 2 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Execute Handler
2-
import type { ExecResult, ProcessStartResult } from '@repo/shared';
2+
import type { ExecResult, Logger, ProcessStartResult } from '@repo/shared';
33
import { ErrorCode } from '@repo/shared/errors';
44

5-
import type { ExecuteRequest, Logger, RequestContext } from '../core/types';
5+
import type { ExecuteRequest, RequestContext } from '../core/types';
66
import type { ProcessService } from '../services/process-service';
77
import { BaseHandler } from './base-handler';
88

@@ -35,13 +35,6 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {
3535
// Parse request body directly
3636
const body = await this.parseRequestBody<ExecuteRequest>(request);
3737
const sessionId = body.sessionId || context.sessionId;
38-
39-
this.logger.info('Executing command', {
40-
requestId: context.requestId,
41-
command: body.command,
42-
sessionId,
43-
background: body.background
44-
});
4538

4639
// If this is a background process, start it as a process
4740
if (body.background) {
@@ -51,22 +44,10 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {
5144
});
5245

5346
if (!processResult.success) {
54-
this.logger.error('Background process start failed', undefined, {
55-
requestId: context.requestId,
56-
command: body.command,
57-
sessionId,
58-
errorCode: processResult.error.code,
59-
errorMessage: processResult.error.message,
60-
});
6147
return this.createErrorResponse(processResult.error, context);
6248
}
6349

6450
const processData = processResult.data;
65-
this.logger.info('Background process started successfully', {
66-
requestId: context.requestId,
67-
processId: processData.id,
68-
command: body.command,
69-
});
7051

7152
const response: ProcessStartResult = {
7253
success: true,
@@ -86,23 +67,10 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {
8667
});
8768

8869
if (!result.success) {
89-
this.logger.error('Command execution failed', undefined, {
90-
requestId: context.requestId,
91-
command: body.command,
92-
sessionId,
93-
errorCode: result.error.code,
94-
errorMessage: result.error.message,
95-
});
9670
return this.createErrorResponse(result.error, context);
9771
}
9872

9973
const commandResult = result.data;
100-
this.logger.info('Command executed successfully', {
101-
requestId: context.requestId,
102-
command: body.command,
103-
exitCode: commandResult.exitCode,
104-
success: commandResult.success,
105-
});
10674

10775
const response: ExecResult = {
10876
success: commandResult.success,
@@ -122,37 +90,18 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {
12290
// Parse request body directly
12391
const body = await this.parseRequestBody<ExecuteRequest>(request);
12492
const sessionId = body.sessionId || context.sessionId;
125-
126-
this.logger.info('Starting streaming command execution', {
127-
requestId: context.requestId,
128-
command: body.command,
129-
sessionId
130-
});
13193

13294
// Start the process for streaming
13395
const processResult = await this.processService.startProcess(body.command, {
13496
sessionId,
13597
});
13698

13799
if (!processResult.success) {
138-
this.logger.error('Streaming process start failed', undefined, {
139-
requestId: context.requestId,
140-
command: body.command,
141-
sessionId,
142-
errorCode: processResult.error.code,
143-
errorMessage: processResult.error.message,
144-
});
145100
return this.createErrorResponse(processResult.error, context);
146101
}
147102

148103
const process = processResult.data;
149104

150-
this.logger.info('Streaming process started successfully', {
151-
requestId: context.requestId,
152-
processId: process.id,
153-
command: body.command,
154-
});
155-
156105
// Create SSE stream
157106
const stream = new ReadableStream({
158107
start(controller) {

0 commit comments

Comments
 (0)