From d4ab3286713bbbe354ad285882362bc773e36950 Mon Sep 17 00:00:00 2001 From: pomelo-nwu Date: Wed, 5 Nov 2025 18:49:04 +0800 Subject: [PATCH] feat: support for custom OpenAI logging directory configuration --- packages/cli/src/config/config.ts | 8 + packages/cli/src/config/settingsSchema.ts | 10 + packages/cli/src/gemini.test.tsx | 1 + .../__tests__/openaiTimeoutHandling.test.ts | 3 + packages/core/src/core/contentGenerator.ts | 1 + .../openaiContentGenerator.ts | 1 + .../telemetryService.ts | 17 +- packages/core/src/utils/openaiLogger.test.ts | 381 ++++++++++++++++++ packages/core/src/utils/openaiLogger.ts | 17 +- 9 files changed, 432 insertions(+), 7 deletions(-) create mode 100644 packages/core/src/utils/openaiLogger.test.ts diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index bd016d76..d747e128 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -114,6 +114,7 @@ export interface CliArgs { openaiLogging: boolean | undefined; openaiApiKey: string | undefined; openaiBaseUrl: string | undefined; + openaiLoggingDir: string | undefined; proxy: string | undefined; includeDirectories: string[] | undefined; tavilyApiKey: string | undefined; @@ -317,6 +318,11 @@ export async function parseArguments(settings: Settings): Promise { description: 'Enable logging of OpenAI API calls for debugging and analysis', }) + .option('openai-logging-dir', { + type: 'string', + description: + 'Custom directory path for OpenAI API logs. Overrides settings files.', + }) .option('openai-api-key', { type: 'string', description: 'OpenAI API key to use for authentication', @@ -764,6 +770,8 @@ export async function loadCliConfig( (typeof argv.openaiLogging === 'undefined' ? settings.model?.enableOpenAILogging : argv.openaiLogging) ?? false, + openAILoggingDir: + argv.openaiLoggingDir || settings.model?.openAILoggingDir, }, cliVersion: await getCliVersion(), webSearch: buildWebSearchConfig( diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index bd87163a..da504c29 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -558,6 +558,16 @@ const SETTINGS_SCHEMA = { description: 'Enable OpenAI logging.', showInDialog: true, }, + openAILoggingDir: { + type: 'string', + label: 'OpenAI Logging Directory', + category: 'Model', + requiresRestart: false, + default: undefined as string | undefined, + description: + 'Custom directory path for OpenAI API logs. If not specified, defaults to logs/openai in the current working directory.', + showInDialog: true, + }, generationConfig: { type: 'object', label: 'Generation Configuration', diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 76d2c772..a5b34922 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -327,6 +327,7 @@ describe('gemini.tsx main function kitty protocol', () => { openaiLogging: undefined, openaiApiKey: undefined, openaiBaseUrl: undefined, + openaiLoggingDir: undefined, proxy: undefined, includeDirectories: undefined, tavilyApiKey: undefined, diff --git a/packages/core/src/core/__tests__/openaiTimeoutHandling.test.ts b/packages/core/src/core/__tests__/openaiTimeoutHandling.test.ts index 7f4eec69..07d3b930 100644 --- a/packages/core/src/core/__tests__/openaiTimeoutHandling.test.ts +++ b/packages/core/src/core/__tests__/openaiTimeoutHandling.test.ts @@ -21,6 +21,9 @@ vi.mock('../../telemetry/loggers.js', () => ({ })); vi.mock('../../utils/openaiLogger.js', () => ({ + OpenAILogger: vi.fn().mockImplementation(() => ({ + logInteraction: vi.fn(), + })), openaiLogger: { logInteraction: vi.fn(), }, diff --git a/packages/core/src/core/contentGenerator.ts b/packages/core/src/core/contentGenerator.ts index 3258cd5c..4d0d33a9 100644 --- a/packages/core/src/core/contentGenerator.ts +++ b/packages/core/src/core/contentGenerator.ts @@ -58,6 +58,7 @@ export type ContentGeneratorConfig = { vertexai?: boolean; authType?: AuthType | undefined; enableOpenAILogging?: boolean; + openAILoggingDir?: string; // Timeout configuration in milliseconds timeout?: number; // Maximum retries for failed requests diff --git a/packages/core/src/core/openaiContentGenerator/openaiContentGenerator.ts b/packages/core/src/core/openaiContentGenerator/openaiContentGenerator.ts index 91e69527..ae1f43e5 100644 --- a/packages/core/src/core/openaiContentGenerator/openaiContentGenerator.ts +++ b/packages/core/src/core/openaiContentGenerator/openaiContentGenerator.ts @@ -32,6 +32,7 @@ export class OpenAIContentGenerator implements ContentGenerator { telemetryService: new DefaultTelemetryService( cliConfig, contentGeneratorConfig.enableOpenAILogging, + contentGeneratorConfig.openAILoggingDir, ), errorHandler: new EnhancedErrorHandler( (error: unknown, request: GenerateContentParameters) => diff --git a/packages/core/src/core/openaiContentGenerator/telemetryService.ts b/packages/core/src/core/openaiContentGenerator/telemetryService.ts index 23560793..9fa47263 100644 --- a/packages/core/src/core/openaiContentGenerator/telemetryService.ts +++ b/packages/core/src/core/openaiContentGenerator/telemetryService.ts @@ -7,7 +7,7 @@ import type { Config } from '../../config/config.js'; import { logApiError, logApiResponse } from '../../telemetry/loggers.js'; import { ApiErrorEvent, ApiResponseEvent } from '../../telemetry/types.js'; -import { openaiLogger } from '../../utils/openaiLogger.js'; +import { OpenAILogger } from '../../utils/openaiLogger.js'; import type { GenerateContentResponse } from '@google/genai'; import type OpenAI from 'openai'; @@ -43,10 +43,17 @@ export interface TelemetryService { } export class DefaultTelemetryService implements TelemetryService { + private logger: OpenAILogger; + constructor( private config: Config, private enableOpenAILogging: boolean = false, - ) {} + openAILoggingDir?: string, + ) { + // Always create a new logger instance to ensure correct working directory + // If no custom directory is provided, undefined will use the default path + this.logger = new OpenAILogger(openAILoggingDir); + } async logSuccess( context: RequestContext, @@ -68,7 +75,7 @@ export class DefaultTelemetryService implements TelemetryService { // Log interaction if enabled if (this.enableOpenAILogging && openaiRequest && openaiResponse) { - await openaiLogger.logInteraction(openaiRequest, openaiResponse); + await this.logger.logInteraction(openaiRequest, openaiResponse); } } @@ -97,7 +104,7 @@ export class DefaultTelemetryService implements TelemetryService { // Log error interaction if enabled if (this.enableOpenAILogging && openaiRequest) { - await openaiLogger.logInteraction( + await this.logger.logInteraction( openaiRequest, undefined, error as Error, @@ -137,7 +144,7 @@ export class DefaultTelemetryService implements TelemetryService { openaiChunks.length > 0 ) { const combinedResponse = this.combineOpenAIChunksForLogging(openaiChunks); - await openaiLogger.logInteraction(openaiRequest, combinedResponse); + await this.logger.logInteraction(openaiRequest, combinedResponse); } } diff --git a/packages/core/src/utils/openaiLogger.test.ts b/packages/core/src/utils/openaiLogger.test.ts new file mode 100644 index 00000000..17a07486 --- /dev/null +++ b/packages/core/src/utils/openaiLogger.test.ts @@ -0,0 +1,381 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as path from 'node:path'; +import * as os from 'os'; +import { promises as fs } from 'node:fs'; +import { OpenAILogger } from './openaiLogger.js'; + +describe('OpenAILogger', () => { + let originalCwd: string; + let testTempDir: string; + const createdDirs: string[] = []; + + beforeEach(() => { + originalCwd = process.cwd(); + testTempDir = path.join(os.tmpdir(), `openai-logger-test-${Date.now()}`); + createdDirs.length = 0; // Clear array + }); + + afterEach(async () => { + // Clean up all created directories + const cleanupPromises = [ + testTempDir, + ...createdDirs, + path.resolve(process.cwd(), 'relative-logs'), + path.resolve(process.cwd(), 'custom-logs'), + path.resolve(process.cwd(), 'test-relative-logs'), + path.join(os.homedir(), 'custom-logs'), + path.join(os.homedir(), 'test-openai-logs'), + ].map(async (dir) => { + try { + await fs.rm(dir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } + }); + + await Promise.all(cleanupPromises); + process.chdir(originalCwd); + }); + + describe('constructor', () => { + it('should use default directory when no custom directory is provided', () => { + const logger = new OpenAILogger(); + // We can't directly access private logDir, but we can verify behavior + expect(logger).toBeInstanceOf(OpenAILogger); + }); + + it('should accept absolute path as custom directory', () => { + const customDir = '/absolute/path/to/logs'; + const logger = new OpenAILogger(customDir); + expect(logger).toBeInstanceOf(OpenAILogger); + }); + + it('should resolve relative path to absolute path', async () => { + const relativeDir = 'custom-logs'; + const logger = new OpenAILogger(relativeDir); + const expectedDir = path.resolve(process.cwd(), relativeDir); + createdDirs.push(expectedDir); + expect(logger).toBeInstanceOf(OpenAILogger); + }); + + it('should expand ~ to home directory', () => { + const customDir = '~/custom-logs'; + const logger = new OpenAILogger(customDir); + expect(logger).toBeInstanceOf(OpenAILogger); + }); + + it('should expand ~/ to home directory', () => { + const customDir = '~/custom-logs'; + const logger = new OpenAILogger(customDir); + expect(logger).toBeInstanceOf(OpenAILogger); + }); + + it('should handle just ~ as home directory', () => { + const customDir = '~'; + const logger = new OpenAILogger(customDir); + expect(logger).toBeInstanceOf(OpenAILogger); + }); + }); + + describe('initialize', () => { + it('should create directory if it does not exist', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const dirExists = await fs + .access(testTempDir) + .then(() => true) + .catch(() => false); + expect(dirExists).toBe(true); + }); + + it('should create nested directories recursively', async () => { + const nestedDir = path.join(testTempDir, 'nested', 'deep', 'path'); + const logger = new OpenAILogger(nestedDir); + await logger.initialize(); + + const dirExists = await fs + .access(nestedDir) + .then(() => true) + .catch(() => false); + expect(dirExists).toBe(true); + }); + + it('should not throw if directory already exists', async () => { + await fs.mkdir(testTempDir, { recursive: true }); + const logger = new OpenAILogger(testTempDir); + await expect(logger.initialize()).resolves.not.toThrow(); + }); + }); + + describe('logInteraction', () => { + it('should create log file with correct format', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + const logPath = await logger.logInteraction(request, response); + + expect(logPath).toContain(testTempDir); + expect(logPath).toMatch( + /openai-\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}\.\d{3}Z-[a-f0-9]{8}\.json/, + ); + + const fileExists = await fs + .access(logPath) + .then(() => true) + .catch(() => false); + expect(fileExists).toBe(true); + }); + + it('should write correct log data structure', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + const logPath = await logger.logInteraction(request, response); + const logContent = JSON.parse(await fs.readFile(logPath, 'utf-8')); + + expect(logContent).toHaveProperty('timestamp'); + expect(logContent).toHaveProperty('request', request); + expect(logContent).toHaveProperty('response', response); + expect(logContent).toHaveProperty('error', null); + expect(logContent).toHaveProperty('system'); + expect(logContent.system).toHaveProperty('hostname'); + expect(logContent.system).toHaveProperty('platform'); + expect(logContent.system).toHaveProperty('release'); + expect(logContent.system).toHaveProperty('nodeVersion'); + }); + + it('should log error when provided', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const error = new Error('Test error'); + + const logPath = await logger.logInteraction(request, undefined, error); + const logContent = JSON.parse(await fs.readFile(logPath, 'utf-8')); + + expect(logContent).toHaveProperty('error'); + expect(logContent.error).toHaveProperty('message', 'Test error'); + expect(logContent.error).toHaveProperty('stack'); + expect(logContent.response).toBeNull(); + }); + + it('should use custom directory when provided', async () => { + const customDir = path.join(testTempDir, 'custom-logs'); + const logger = new OpenAILogger(customDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + const logPath = await logger.logInteraction(request, response); + + expect(logPath).toContain(customDir); + expect(logPath.startsWith(customDir)).toBe(true); + }); + + it('should resolve relative path correctly', async () => { + const relativeDir = 'relative-logs'; + const logger = new OpenAILogger(relativeDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + const logPath = await logger.logInteraction(request, response); + const expectedDir = path.resolve(process.cwd(), relativeDir); + createdDirs.push(expectedDir); + + expect(logPath).toContain(expectedDir); + }); + + it('should expand ~ correctly', async () => { + const customDir = '~/test-openai-logs'; + const logger = new OpenAILogger(customDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + const logPath = await logger.logInteraction(request, response); + const expectedDir = path.join(os.homedir(), 'test-openai-logs'); + createdDirs.push(expectedDir); + + expect(logPath).toContain(expectedDir); + }); + }); + + describe('getLogFiles', () => { + it('should return empty array when directory does not exist', async () => { + const logger = new OpenAILogger(testTempDir); + const files = await logger.getLogFiles(); + expect(files).toEqual([]); + }); + + it('should return log files after initialization', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + await logger.logInteraction(request, response); + const files = await logger.getLogFiles(); + + expect(files.length).toBeGreaterThan(0); + expect(files[0]).toMatch(/openai-.*\.json$/); + }); + + it('should return only log files matching pattern', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + // Create a log file + await logger.logInteraction({ test: 'request' }, { test: 'response' }); + + // Create a non-log file + await fs.writeFile(path.join(testTempDir, 'other-file.txt'), 'content'); + + const files = await logger.getLogFiles(); + expect(files.length).toBe(1); + expect(files[0]).toMatch(/openai-.*\.json$/); + }); + + it('should respect limit parameter', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + // Create multiple log files + for (let i = 0; i < 5; i++) { + await logger.logInteraction( + { test: `request-${i}` }, + { test: `response-${i}` }, + ); + // Small delay to ensure different timestamps + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + const allFiles = await logger.getLogFiles(); + expect(allFiles.length).toBe(5); + + const limitedFiles = await logger.getLogFiles(3); + expect(limitedFiles.length).toBe(3); + }); + + it('should return files sorted by most recent first', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const files: string[] = []; + for (let i = 0; i < 3; i++) { + const logPath = await logger.logInteraction( + { test: `request-${i}` }, + { test: `response-${i}` }, + ); + files.push(logPath); + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + const retrievedFiles = await logger.getLogFiles(); + expect(retrievedFiles[0]).toBe(files[2]); // Most recent first + expect(retrievedFiles[1]).toBe(files[1]); + expect(retrievedFiles[2]).toBe(files[0]); + }); + }); + + describe('readLogFile', () => { + it('should read and parse log file correctly', async () => { + const logger = new OpenAILogger(testTempDir); + await logger.initialize(); + + const request = { + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }; + const response = { id: 'test-id', choices: [] }; + + const logPath = await logger.logInteraction(request, response); + const logData = await logger.readLogFile(logPath); + + expect(logData).toHaveProperty('timestamp'); + expect(logData).toHaveProperty('request', request); + expect(logData).toHaveProperty('response', response); + }); + + it('should throw error when file does not exist', async () => { + const logger = new OpenAILogger(testTempDir); + const nonExistentPath = path.join(testTempDir, 'non-existent.json'); + + await expect(logger.readLogFile(nonExistentPath)).rejects.toThrow(); + }); + }); + + describe('path resolution', () => { + it('should normalize absolute paths', () => { + const absolutePath = '/tmp/test/logs'; + const logger = new OpenAILogger(absolutePath); + expect(logger).toBeInstanceOf(OpenAILogger); + }); + + it('should resolve relative paths based on current working directory', async () => { + const relativePath = 'test-relative-logs'; + const logger = new OpenAILogger(relativePath); + await logger.initialize(); + + const request = { test: 'request' }; + const response = { test: 'response' }; + + const logPath = await logger.logInteraction(request, response); + const expectedBaseDir = path.resolve(process.cwd(), relativePath); + createdDirs.push(expectedBaseDir); + + expect(logPath).toContain(expectedBaseDir); + }); + + it('should handle paths with special characters', async () => { + const specialPath = path.join(testTempDir, 'logs-with-special-chars'); + const logger = new OpenAILogger(specialPath); + await logger.initialize(); + + const request = { test: 'request' }; + const response = { test: 'response' }; + + const logPath = await logger.logInteraction(request, response); + expect(logPath).toContain(specialPath); + }); + }); +}); diff --git a/packages/core/src/utils/openaiLogger.ts b/packages/core/src/utils/openaiLogger.ts index 8e53e86e..a4fc41ec 100644 --- a/packages/core/src/utils/openaiLogger.ts +++ b/packages/core/src/utils/openaiLogger.ts @@ -18,10 +18,23 @@ export class OpenAILogger { /** * Creates a new OpenAI logger - * @param customLogDir Optional custom log directory path + * @param customLogDir Optional custom log directory path (supports relative paths, absolute paths, and ~ expansion) */ constructor(customLogDir?: string) { - this.logDir = customLogDir || path.join(process.cwd(), 'logs', 'openai'); + if (customLogDir) { + // Resolve relative paths to absolute paths + // Handle ~ expansion + let resolvedPath = customLogDir; + if (customLogDir === '~' || customLogDir.startsWith('~/')) { + resolvedPath = path.join(os.homedir(), customLogDir.slice(1)); + } else if (!path.isAbsolute(customLogDir)) { + // If it's a relative path, resolve it relative to current working directory + resolvedPath = path.resolve(process.cwd(), customLogDir); + } + this.logDir = path.normalize(resolvedPath); + } else { + this.logDir = path.join(process.cwd(), 'logs', 'openai'); + } } /**