Yuki Takei 1 неделя назад
Родитель
Сommit
386af6896c

+ 6 - 6
.kiro/specs/migrate-logger-to-pino/tasks.md

@@ -173,26 +173,26 @@
   - Verify the @growi/logger package's own tests pass
   - _Requirements: 1.4, 8.1, 8.2, 8.3, 8.4, 10.1, 10.2_
 
-- [ ] 10. Improve log output formatting for readability
-- [ ] 10.1 (P) Differentiate pino-pretty singleLine between dev and production FORMAT_NODE_LOG
+- [x] 10. Improve log output formatting for readability
+- [x] 10.1 (P) Differentiate pino-pretty singleLine between dev and production FORMAT_NODE_LOG
   - In the transport factory, change the production + FORMAT_NODE_LOG path to use `singleLine: true` for concise one-liner output
   - Keep the development path at `singleLine: false` so developers see full multi-line context
   - Update unit tests to verify: dev returns `singleLine: false`, production + FORMAT_NODE_LOG returns `singleLine: true`, production without FORMAT_NODE_LOG still returns no transport
   - _Requirements: 5.1, 5.3_
 
-- [ ] 10.2 (P) Add morgan-like HTTP request message formatting to pino-http in apps/app
+- [x] 10.2 (P) Add morgan-like HTTP request message formatting to pino-http in apps/app
   - Configure `customSuccessMessage` to produce `METHOD /url STATUS - TIMEms` format (e.g., `GET /page/path 200 - 12ms`)
   - Configure `customErrorMessage` to include the error message alongside method, URL, and status code
   - Configure `customLogLevel` to return `warn` for 4xx responses and `error` for 5xx or error responses, keeping `info` for successful requests
   - Verify that `/_next/static/` path skipping in dev mode still works after the changes
   - _Requirements: 6.1, 6.4_
 
-- [ ] 10.3 (P) Add morgan-like HTTP request message formatting to pino-http in apps/slackbot-proxy
+- [x] 10.3 (P) Add morgan-like HTTP request message formatting to pino-http in apps/slackbot-proxy
   - Apply the same `customSuccessMessage`, `customErrorMessage`, and `customLogLevel` configuration as apps/app
   - _Requirements: 6.1, 6.4_
 
-- [ ] 11. Validate formatting improvements
-- [ ] 11.1 Run tests and build for affected packages
+- [x] 11. Validate formatting improvements
+- [x] 11.1 Run tests and build for affected packages
   - Run the @growi/logger package tests to confirm transport factory changes pass
   - Run lint and type-check for apps/app and apps/slackbot-proxy
   - Verify the production build succeeds

+ 3 - 1
apps/app/src/server/crowi/index.ts

@@ -2,6 +2,7 @@ import next from 'next';
 import http from 'node:http';
 import path from 'node:path';
 import { createTerminus } from '@godaddy/terminus';
+import { morganLikeFormatOptions } from '@growi/logger';
 import attachmentRoutes from '@growi/remark-attachment-refs/dist/server';
 import lsxRoutes from '@growi/remark-lsx/dist/server/index.cjs';
 import type { Express } from 'express';
@@ -614,9 +615,10 @@ class Crowi {
 
     require('./express-init')(this, express);
 
-    // HTTP request logging with pino-http
+    // HTTP request logging with pino-http (morgan-like one-liner format)
     const httpLoggerOptions: PinoHttpOptions = {
       logger: loggerFactory('express'),
+      ...morganLikeFormatOptions,
       // supress logging for Next.js static files in development mode
       ...(env !== 'production' && {
         autoLogging: {

+ 7 - 2
apps/slackbot-proxy/src/Server.ts

@@ -3,6 +3,7 @@ import '@tsed/swagger';
 import '@tsed/typeorm'; // !! DO NOT MODIFY !! -- https://github.com/tsedio/tsed/issues/1332#issuecomment-837840612
 
 import { createTerminus } from '@godaddy/terminus';
+import { morganLikeFormatOptions } from '@growi/logger';
 import { HttpServer, PlatformApplication } from '@tsed/common';
 import { Configuration, Inject, InjectorService } from '@tsed/di';
 import bodyParser from 'body-parser';
@@ -11,7 +12,7 @@ import cookieParser from 'cookie-parser';
 import type { Express } from 'express';
 import helmet from 'helmet';
 import methodOverride from 'method-override';
-import pinoHttp from 'pino-http';
+import pinoHttp, { type Options as PinoHttpOptions } from 'pino-http';
 import type { ConnectionOptions } from 'typeorm';
 import { getConnectionManager } from 'typeorm';
 
@@ -162,7 +163,11 @@ export class Server {
    * Setup logger for requests
    */
   private setupLogger(): void {
-    const httpLogger = pinoHttp({ logger: loggerFactory('express') });
+    const httpLogger = pinoHttp({
+      // Type assertion needed: @growi/logger returns Logger<string> but pino-http expects Logger<LevelWithSilent>
+      logger: loggerFactory('express') as unknown as PinoHttpOptions['logger'],
+      ...morganLikeFormatOptions,
+    });
     this.app.use(httpLogger);
   }
 }

+ 1 - 0
packages/logger/src/index.ts

@@ -1,6 +1,7 @@
 export { parseEnvLevels } from './env-var-parser';
 export { resolveLevel } from './level-resolver';
 export { initializeLoggerFactory, loggerFactory } from './logger-factory';
+export { morganLikeFormatOptions } from './morgan-like-format-options';
 export {
   createBrowserOptions,
   createNodeTransportOptions,

+ 83 - 0
packages/logger/src/morgan-like-format-options.spec.ts

@@ -0,0 +1,83 @@
+import type { IncomingMessage, ServerResponse } from 'node:http';
+import { describe, expect, it } from 'vitest';
+
+import { morganLikeFormatOptions } from './morgan-like-format-options';
+
+function fakeReq(method: string, url: string): IncomingMessage {
+  return { method, url } as IncomingMessage;
+}
+
+function fakeRes(statusCode: number): ServerResponse {
+  return { statusCode } as unknown as ServerResponse;
+}
+
+describe('morganLikeFormatOptions', () => {
+  describe('customSuccessMessage', () => {
+    it('formats as METHOD /url STATUS - TIMEms', () => {
+      const msg = morganLikeFormatOptions.customSuccessMessage(
+        fakeReq('GET', '/page/path'),
+        fakeRes(200),
+        12.4,
+      );
+      expect(msg).toBe('GET /page/path 200 - 12ms');
+    });
+
+    it('rounds responseTime to nearest integer', () => {
+      const msg = morganLikeFormatOptions.customSuccessMessage(
+        fakeReq('POST', '/api'),
+        fakeRes(201),
+        0.7,
+      );
+      expect(msg).toBe('POST /api 201 - 1ms');
+    });
+  });
+
+  describe('customErrorMessage', () => {
+    it('includes error message', () => {
+      const msg = morganLikeFormatOptions.customErrorMessage(
+        fakeReq('PUT', '/data'),
+        fakeRes(500),
+        new Error('db timeout'),
+      );
+      expect(msg).toBe('PUT /data 500 - db timeout');
+    });
+  });
+
+  describe('customLogLevel', () => {
+    it('returns info for 2xx responses', () => {
+      const level = morganLikeFormatOptions.customLogLevel(
+        fakeReq('GET', '/'),
+        fakeRes(200),
+        undefined,
+      );
+      expect(level).toBe('info');
+    });
+
+    it('returns warn for 4xx responses', () => {
+      const level = morganLikeFormatOptions.customLogLevel(
+        fakeReq('GET', '/'),
+        fakeRes(404),
+        undefined,
+      );
+      expect(level).toBe('warn');
+    });
+
+    it('returns error for 5xx responses', () => {
+      const level = morganLikeFormatOptions.customLogLevel(
+        fakeReq('GET', '/'),
+        fakeRes(503),
+        undefined,
+      );
+      expect(level).toBe('error');
+    });
+
+    it('returns error when error object is present', () => {
+      const level = morganLikeFormatOptions.customLogLevel(
+        fakeReq('GET', '/'),
+        fakeRes(200),
+        new Error('unexpected'),
+      );
+      expect(level).toBe('error');
+    });
+  });
+});

+ 51 - 0
packages/logger/src/morgan-like-format-options.ts

@@ -0,0 +1,51 @@
+import type { IncomingMessage, ServerResponse } from 'node:http';
+
+/**
+ * Morgan-like log message formatters for pino-http.
+ *
+ * Produces concise one-liner messages in the style of morgan's "combined" format:
+ *   GET /page/path 200 - 12ms
+ *
+ * Usage with pino-http:
+ *   pinoHttp({ ...morganLikeFormatOptions, logger })
+ */
+
+type CustomSuccessMessage = (
+  req: IncomingMessage,
+  res: ServerResponse,
+  responseTime: number,
+) => string;
+
+type CustomErrorMessage = (
+  req: IncomingMessage,
+  res: ServerResponse,
+  error: Error,
+) => string;
+
+type LogLevel = 'info' | 'warn' | 'error';
+
+type CustomLogLevel = (
+  req: IncomingMessage,
+  res: ServerResponse,
+  error: Error | undefined,
+) => LogLevel;
+
+export const morganLikeFormatOptions: {
+  customSuccessMessage: CustomSuccessMessage;
+  customErrorMessage: CustomErrorMessage;
+  customLogLevel: CustomLogLevel;
+} = {
+  customSuccessMessage: (req, res, responseTime) => {
+    return `${req.method} ${req.url} ${res.statusCode} - ${Math.round(responseTime)}ms`;
+  },
+
+  customErrorMessage: (req, res, error) => {
+    return `${req.method} ${req.url} ${res.statusCode} - ${error.message}`;
+  },
+
+  customLogLevel: (_req, res, error) => {
+    if (error != null || res.statusCode >= 500) return 'error';
+    if (res.statusCode >= 400) return 'warn';
+    return 'info';
+  },
+};

+ 13 - 0
packages/logger/src/transport-factory.spec.ts

@@ -28,6 +28,12 @@ describe('createNodeTransportOptions', () => {
       expect(popts?.ignore).toContain('pid');
       expect(popts?.ignore).toContain('hostname');
     });
+
+    it('returns singleLine: false for full multi-line context', () => {
+      const opts = createNodeTransportOptions(false);
+      const popts = opts.transport?.options as Record<string, unknown>;
+      expect(popts?.singleLine).toBe(false);
+    });
   });
 
   describe('production mode — raw JSON', () => {
@@ -65,5 +71,12 @@ describe('createNodeTransportOptions', () => {
       expect(opts.transport).toBeDefined();
       expect(opts.transport?.target).toBe('pino-pretty');
     });
+
+    it('returns singleLine: true for concise one-liner output', () => {
+      delete process.env.FORMAT_NODE_LOG;
+      const opts = createNodeTransportOptions(true);
+      const popts = opts.transport?.options as Record<string, unknown>;
+      expect(popts?.singleLine).toBe(true);
+    });
   });
 });

+ 1 - 1
packages/logger/src/transport-factory.ts

@@ -48,7 +48,7 @@ export function createNodeTransportOptions(
       options: {
         translateTime: 'SYS:standard',
         ignore: 'pid,hostname',
-        singleLine: false,
+        singleLine: true,
       },
     },
   };