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

[BE] 요청 로그에 사용자 정보를 추가 #298

Merged
merged 11 commits into from
Aug 23, 2024
8 changes: 8 additions & 0 deletions backend/src/main/java/kr/momo/config/WebConfig.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
package kr.momo.config;

import java.util.List;
import kr.momo.config.filter.LogInterceptor;
Copy link
Contributor

Choose a reason for hiding this comment

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

패키지 이름도 수정하면 좋을 것 같아요!!

Copy link
Contributor

Choose a reason for hiding this comment

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

동의합니다!

import kr.momo.controller.auth.AuthArgumentResolver;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

@Configuration
@RequiredArgsConstructor
public class WebConfig implements WebMvcConfigurer {

private final AuthArgumentResolver authArgumentResolver;
private final LogInterceptor logInterceptor;

@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(authArgumentResolver);
}

@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(logInterceptor).addPathPatterns("/api/v1/**");
}
}
29 changes: 0 additions & 29 deletions backend/src/main/java/kr/momo/config/filter/FilterConfig.java

This file was deleted.

42 changes: 0 additions & 42 deletions backend/src/main/java/kr/momo/config/filter/LogFilter.java

This file was deleted.

35 changes: 33 additions & 2 deletions backend/src/main/java/kr/momo/config/filter/LogGenerator.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,51 @@
package kr.momo.config.filter;

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.util.Arrays;
import java.util.Optional;
import kr.momo.service.auth.JwtManager;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;

@Slf4j
@Component
@RequiredArgsConstructor
public class LogGenerator {

private static final String ANONYMOUS = "ANONYMOUS";
private static final String ACCESS_TOKEN = "ACCESS_TOKEN";

private final JwtManager jwtManager;

public void logRequest(String traceId, HttpServletRequest request) {
String httpMethod = request.getMethod();
String requestURI = request.getRequestURI();
String remoteAddr = request.getRemoteAddr();
Cookie[] cookies = request.getCookies();
String userInfo = getCookieValue(cookies).orElse(ANONYMOUS);

if (isLoginUser(userInfo)) {
userInfo = String.valueOf(jwtManager.extract(userInfo));
}

log.info("REQUEST [{}][USERID:{}][{} {}]", traceId, userInfo, httpMethod, requestURI);
Copy link
Contributor

Choose a reason for hiding this comment

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

응답 로그에도 사용자 정보를 남기면 좋을 것 같은데, 요청 로그에만 명시한 이유가 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

요청에 사용자 정보를 남겨 두었으니, 응답 로그는 traceId로 해당 사용자가 요청하는 것인지 판단할 수 있다고 생각했습니다.

pr 본문을 예로 들면, userId가 2인 요청 로그의 traceId8032f46f 이므로 userId가 2인 사용자의 응답 로그를 보려면 traceId8032f46f인 응답 로그를 찾아보는 식입니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

응답 로그만 확인했을 때도 정보가 명확했으면 좋겠어요 😊
사용자 정보를 응답 로그에 추가하는 건 별로일까요?

Copy link
Member

Choose a reason for hiding this comment

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

동의합니다. 방대한 양의 로그를 한 번 찾는 것도 수고로운데, 다른 로그를 다시 찾아봐야 한다면 오버헤드가 더 클 것 같아요

}

private Optional<String> getCookieValue(Cookie[] cookies) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 로직은 LogGenerator 클래스 역할과 어울리지 않는 것 같아요
사용자 정보를 매개변수로 받는게 한 가지 일만 수행하는 관점에서 더 적절해보이는데 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 맞습니다. 어색하다고 느꼈는데, 인터셉터를 분리하는 방향으로 해결할 수 있겠네요!

제 개인적인 생각에는 현재 로그 인터셉터에서 JWT 로직에 관한 부분만 새로운 인터셉터를 생성해서 책임 분리하는 방법도 좋아보여요.

위 내용을 반영해서 수정했습니다~

if (cookies == null) {
return Optional.empty();
}

return Arrays.stream(cookies)
.filter(cookie -> ACCESS_TOKEN.equals(cookie.getName()))
.map(Cookie::getValue)
.findFirst();
}

log.info("REQUEST [{}][{} {}][{}]", traceId, httpMethod, requestURI, remoteAddr);
private boolean isLoginUser(String token) {
return !ANONYMOUS.equals(token);
}

public void logResponse(String traceId, long duration, HttpServletRequest request, HttpServletResponse response) {
Expand Down
40 changes: 40 additions & 0 deletions backend/src/main/java/kr/momo/config/filter/LogInterceptor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package kr.momo.config.filter;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.slf4j.MDC;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.HandlerInterceptor;

@Slf4j
@RequiredArgsConstructor
@Component
public class LogInterceptor implements HandlerInterceptor {

public static final String TRACE_ID = "traceId";

private final TraceIdGenerator traceIdGenerator;
private final LogGenerator logGenerator;

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
String traceId = traceIdGenerator.generateShortUuid();
MDC.put(TRACE_ID, traceId);
logGenerator.logRequest(traceId, request);
request.setAttribute("startTime", System.currentTimeMillis());
return true;
}

@Override
public void afterCompletion(
HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex
) {
long startTime = (Long) request.getAttribute("startTime");
Copy link
Contributor

Choose a reason for hiding this comment

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

하드 코딩된 startTime도 상수로 분리해보는건 어떨까요?

long duration = System.currentTimeMillis() - startTime;
String traceId = MDC.get(TRACE_ID);
logGenerator.logResponse(traceId, duration, request, response);
MDC.remove(TRACE_ID);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package kr.momo.exception;

import kr.momo.config.filter.LogFilter;
import kr.momo.config.filter.LogInterceptor;
import lombok.extern.slf4j.Slf4j;
import org.slf4j.MDC;
import org.springframework.http.HttpStatus;
Expand All @@ -20,7 +20,7 @@ public class GlobalExceptionHandler {

@ExceptionHandler
public ProblemDetail handleMomoException(MomoException ex) {
String traceId = MDC.get(LogFilter.TRACE_ID);
String traceId = MDC.get(LogInterceptor.TRACE_ID);
log.warn(EXCEPTION_LOG_FORMAT, traceId, ex);

ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(ex.httpStatus(), ex.message());
Expand All @@ -29,7 +29,7 @@ public ProblemDetail handleMomoException(MomoException ex) {

@ExceptionHandler
public ProblemDetail handleMethodArgumentNotValidException(MethodArgumentNotValidException ex) {
String traceId = MDC.get(LogFilter.TRACE_ID);
String traceId = MDC.get(LogInterceptor.TRACE_ID);
log.warn(EXCEPTION_LOG_FORMAT, traceId, ex);

ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(
Expand All @@ -40,7 +40,7 @@ public ProblemDetail handleMethodArgumentNotValidException(MethodArgumentNotVali

@ExceptionHandler
public ProblemDetail handleInternalException(Exception ex) {
String traceId = MDC.get(LogFilter.TRACE_ID);
String traceId = MDC.get(LogInterceptor.TRACE_ID);
log.error(EXCEPTION_LOG_FORMAT, traceId, ex);

return ProblemDetail.forStatusAndDetail(HttpStatus.INTERNAL_SERVER_ERROR, INTERNAL_SERVER_ERROR_MESSAGE);
Expand Down