Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: logging middleware #135

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Module } from '@nestjs/common';
import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common';
import { APP_GUARD } from '@nestjs/core';
import { JwtService } from '@nestjs/jwt';
import { AppController } from './app.controller';
Expand Down Expand Up @@ -29,6 +29,7 @@
import { ClsPluginTransactional } from '@nestjs-cls/transactional';
import { PrismaService } from '@src/prisma/prisma.service';
import { TransactionalAdapterPrisma } from '@nestjs-cls/transactional-adapter-prisma';
import { AppLoggerMiddleware } from '@src/common/middleware/http.logging.middleware';

@Module({
imports: [

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치에 대한 간단한 검토를 하겠습니다.

개선 제안 및 버그 위험

  1. 미사용 코드 주석 처리:

    • LoggingMiddleware 임포트 문이 주석 처리되어 있는데, 현재 코드에서 사용되지 않는 경우 지워버리는 것이 깔끔합니다. 불필요한 코드가 프로젝트 관리에 혼란을 줄 수 있습니다.
  2. NestModule 구현:

    • NestModule 인터페이스가 추가되었으나, 실제로 이를 구현하는 코드가 없습니다. 필요하다면 configure() 메서드를 추가하여 미들웨어를 설정할 수 있습니다. 그렇지 않으면 불필요한 의존성이 될 수 있습니다.
  3. 주석의 용도:

    • 주석으로 남겨진 코드는 나중에 사용할 계획이 없는 것이라면 삭제하는 게 좋습니다. 주석은 종종 혼란을 초래할 수 있기 때문입니다.
  4. 모듈 구조:

    • 모듈 내 의존성을 명확히 하고, 필요하지 않은 모듈은 제거하도록 검토해야 합니다. 예를 들어, ClsModule이나 PrismaService의 목적이 명확하지 않다면 검토해보는 것이 좋습니다.
  5. 코드 일관성:

    • 전체적인 코드 스타일과 일관성을 유지하는 것이 중요합니다. 다른 파일들과 비교하여 변화를 적용하는 것이 좋습니다.

이러한 점들을 고려하여 코드를 개선하면 더 견고하고 이해하기 쉬운 모듈을 만들 수 있을 것입니다.

Expand Down Expand Up @@ -83,7 +84,7 @@
{
provide: APP_GUARD,
useFactory: () => {
const env = process.env.NODE_ENV;

Check warning on line 87 in src/app.module.ts

View workflow job for this annotation

GitHub Actions / Lint

'env' is assigned a value but never used
},
},
JwtCookieGuard,
Expand All @@ -92,4 +93,8 @@
JwtService,
],
})
export class AppModule {}
export class AppModule implements NestModule {
configure(consumer: MiddlewareConsumer): any {
consumer.apply(AppLoggerMiddleware).forRoutes('*');
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 결과는 다음과 같습니다:

  1. 버그 위험:

    • AppLoggerMiddleware의 구현에 따라 오류가 발생할 수 있습니다. 미들웨어가 요청과 응답을 처리하는 로직에서 예외 처리가 필요합니다.
    • 모든 경로(*)에 대해 미들웨어를 적용하는 것은 부하를 증가시킬 수 있으므로, 특정 경로에만 적용하는 것이 좋습니다.
  2. 개선 제안:

    • AppLoggerMiddleware의 설정 및 사용을 명시적으로 문서화하면 유지보수에 도움이 될 수 있습니다.
    • 만약 미들웨어에서 상태나 다른 서비스와 상호작용을 한다면, 의존성 주입 방식으로 초기화하는 것이 더 좋습니다.
    • 가능하다면 TypeScript 타입을 명확히 하여 코드의 가독성을 높이고 에러를 방지하십시오.
  3. 일반적인 권장 사항:

    • 단위 테스트를 추가하여 미들웨어의 동작을 검증하고, 이상적인 작동을 보장하는 것이 좋습니다.

이러한 점들을 고려하여 코드를 개선하면 더욱 안정적이고 효율적인 애플리케이션을 만들 수 있을 것입니다.

24 changes: 24 additions & 0 deletions src/common/middleware/http.logging.middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Injectable, NestMiddleware, Logger } from '@nestjs/common';

import { Request, Response, NextFunction } from 'express';

@Injectable()
export class AppLoggerMiddleware implements NestMiddleware {
private logger = new Logger('HTTP');

use(request: Request, response: Response, next: NextFunction): void {
const { ip, method, path: url } = request;
const userAgent = request.get('user-agent') || '';

response.on('close', () => {
const { statusCode } = response;
const contentLength = response.get('content-length');

this.logger.log(
`${method} ${url} ${statusCode} ${contentLength} - ${userAgent} ${ip}`,
);
});

next();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 결과는 다음과 같습니다:

장점:

  1. 명확한 역할: Middleware가 HTTP 요청 로그를 기록하는 역할을 명확히 하고 있습니다.
  2. Logger 사용: NestJS의 Logger 클래스를 활용하여 일관된 로그 형식을 제공합니다.

잠재적 문제:

  1. Content-Length 처리: content-length 헤더가 비어있을 경우 로그에 null 또는 undefined가 나타날 수 있습니다. 이를 기본값으로 처리하면 좋습니다.

    const contentLength = response.get('content-length') || '0';
  2. 비동기 로깅: 현재 로그를 기록하는 과정이 비동기적으로 요청이 끝날 때까지 기다립니다. 응답이 완료되는 시점에서 메시지를 기록하기 때문에, 간혹 동시성이 문제가 될 수 있는 고부하 환경에서는 로그가 유실될 가능성이 있습니다. 일반적으로, 로그는 finish 이벤트로 처리하는 것이 더 안전합니다.

개선 제안:

  • 포맷팅 개선: 로그 출력을 좀 더 읽기 쉽게 포맷팅할 수 있습니다. 예를 들어, JSON 형식으로 묶어서 출력하는 방법을 고려할 수 있습니다.
this.logger.log(JSON.stringify({
  method,
  url,
  statusCode,
  contentLength,
  userAgent,
  ip,
}));
  • 에러 핸들링: 가능하면 에러 상황도 로그에 포함시키는 것으로 확장하면 유용할 것입니다. 예를 들어 유효하지 않은 요청 시 로그를 남기는 것도 좋은 접근입니다.

이와 같은 점들을 개선하면 더욱 안정적이고 사용자 친화적인 로그 시스템이 될 것입니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 결과는 다음과 같습니다:

버그 리스크

  1. req.body 처리: POST 외의 메소드에 대한 처리가 모두 req.body로 설정되어 있습니다. 이 부분은 다른 HTTP 메소드에 대해 적절한 로그를 남기지 못할 수 있습니다.
  2. requestId 처리: 기본값으로 UUID 생성이 되어 있지만, 만약 클라이언트가 유효한 uuid를 제공하지 않을 경우 여전히 UUID가 생략될 수 있습니다.
  3. res.on('finish') 사용: 모든 파이프라인에서 finish 이벤트를 등록하고 있으며, 이는 여러 번 호출될 위험이 있습니다. 단일 호출로 제한하는 로직이 필요합니다.

개선 제안

  1. 로깅 포맷 통일화: 요청 및 응답 로깅에서 사용하는 필드들을 일관되게 구성하면 나중에 분석하기 용이합니다.
  2. 예외 처리 향상: 예외가 발생했을 때, 클라이언트에게 더 구체적인 정보를 보내기 위해 res.status().json()을 활용할 수 있습니다.
  3. 메타데이터 포함: 요청의 메타데이터를 더 추가하여 나중에 로그를 통해 유용한 정보(예: 사용자 IP)에 접근하기 쉽게 할 수 있습니다.
  4. 리듀서 추가: 요청 데이터를 간단하게 정리해서 로그에 남길 수 있도록 리듀서를 구현하면 가독성이 개선됩니다.

총평

로깅 인터셉터로써 기능은 잘 유지되고 있으나, 약간의 예외 처리와 코드 정리가 필요합니다. 전반적으로 안정성과 가독성을 높이는 방향으로 개선할 수 있을 것입니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에 대한 간단한 코드 리뷰를 제공하겠습니다.

버그 위험 요소:

  1. UUID가 없는 경우: requestIdundefined일 수 있지만, 로깅 데이터에서는 항상 UUID가 존재해야 한다고 가정하고 사용합니다. 기본값을 제공하는 것이 좋습니다.
  2. POST와 PUT 요청 처리 중복: POST와 기타 모든 메서드에 대해 동일한 조건 처리하는 부분(else 대신 else if (method === 'PUT'))이 있으며, 코드를 더 명확하게 하기 위해 두 개의 분기를 나누는 것이 좋습니다.

개선 제안:

  1. 리퀘스트 및 레스폰스 로깅 구조화: 로그 데이터를 더 구조적으로 만들기 위해 인터페이스를 사용하면 명확성 증가.
  2. 로깅 포맷: 특정 속성을 JSON 형태로 가지도록 포맷을 커스터마이징 하지만, 이 부분을 좀 더 직관적으로 개선할 수 있습니다. 예를 들어, 필요한 정보만 선택적으로 로그에 포함시킬 수 있습니다.
  3. 수동 에러 핸들링 제거: req.body를 읽을 때 발생할 수 있는 오류를 자동으로 처리하도록 NestJS의 유효성 검사 기능을 사용하는 것도 고려해 보세요.
  4. 응답 시간 계산: finish 이벤트 리스너 내에서 응답 시간이 매우 정확하게 계산될 수 있지만, 비동기 실행 흐름에서 로그가 누락될 가능성은 다소 존재합니다. 즉, 모든 로깅이 완벽히 기록되도록 처리할지 검토하는 것이 좋습니다.
  5. 시간대 관리: 서버 머신의 지역적 시간대를 고려할 경우, 로그 기록 시 고정된 시간대 설정이 필요할 수 있습니다.

이러한 점들을 고려하여 코드의 안정성과 가독성을 높일 수 있습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 진행하겠습니다.

코드 리뷰 요약

  1. 로깅 구현:

    • Winston을 이용하여 요청 및 응답 로깅을 잘 설정했습니다. DailyRotateFile 사용으로 인해 로그 파일 관리도 용이합니다.
  2. 예외 처리:

    • 예외 발생 시 빈 객체를 기본으로 설정하는 부분은 좋지만, 구체적인 로그를 남기는 것이 더 유용할 수 있습니다. console.error() 같은 추가 로깅을 고려해 보세요.
  3. 요청 데이터 처리:

    • POST 방식의 요청에서 조건을 잘 나누고 처리는 적절하지만, PUT, PATCH와 같은 다른 HTTP 메서드를 통일감 있게 처리해야 할 수도 있습니다.
  4. 타임존 처리:

    • 타임존을 서울로 하여 좋은 선택입니다. 하지만 UTC로 스탬프를 찍고 클라이언트 쪽에서 변환하는 방법도 고려해보세요.
  5. requestId 처리:

    • UUID를 헤더에서 가져오도록 했는데, 값이 없을 경우의 비즈니스 요구사항에 맞춰 기본값을 설정하는 것도 좋습니다.
  6. 로그 포맷 일관성:

    • 요청 및 응답의 로그 포맷이 통일성을 가지지 않으면 나중에 로그 분석이 어려울 수 있습니다. 필요하다면 포맷팅을 개선해보세요.
  7. 내부 서버 오류 핸들링:

    • HttpException 외에도 다른 종류의 오류 처리를 추가해서 더 많은 세부정보를 로그에 남길 수 있습니다.
  8. 주석 처리된 코드 제거:

    • 주석 처리된 부분은 불필요한 코드를 깔끔하게 정리하여 가독성을 높이는 것이 좋습니다. 필요하다면 명확한 의도를 가진 주석만 유지하세요.

버그 위험

  • req.user.sid가 존재하지 않을 경우 undefined를 빈 문자열로 대체하는 방식은 생각보다 안전하지만 경우에 따라서 값이 더 복잡할 수 있으므로 확인이 필요합니다.
  • 로거의 동시성 문제도 고려하세요. 멀티스레드 컨텍스트에서 여러 요청이 동시에 로깅될 수 있기 때문에 경합 상태(race condition) 발생 가능성이 있습니다.

개선 사항

  • API 요청/응답 구조에 대해서 스키마 검증을 추가하여 로그하기 전에 데이터를 검증하는 방법을 생각해볼 수 있습니다.
  • 테스트 가능한 단위로 로깅 기능을 나누어 각 컴포넌트가 독립적으로 테스트 가능한 구조로 개발하는 것도 도움이 됩니다.

요약하자면, 매우 잘 작성된 코드이며 몇 가지 세부적인 개선을 통해 더욱 견고한 시스템으로 발전할 수 있을 거라 생각됩니다.

Loading