-
Notifications
You must be signed in to change notification settings - Fork 4
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
요청, 응답 로그 설정 #481
요청, 응답 로그 설정 #481
Changes from all commits
f54d2b3
3c17a29
f0fe020
97acd64
a6f3980
14e78cd
6f4c26d
f70e421
73b499a
9f8323e
86fd8ef
01855f6
4b1c7cc
1e221ec
ab5fe36
33df9b1
c46cb72
2c9ff8e
809a6dc
e3cd7a7
75cb541
b084845
7e452ba
548ec20
ad952d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,24 @@ | ||
package com.votogether.global.config; | ||
|
||
import com.votogether.global.jwt.JwtAuthenticationFilter; | ||
import com.votogether.global.jwt.JwtAuthorizationArgumentResolver; | ||
import com.votogether.global.jwt.TokenProcessor; | ||
import com.votogether.global.log.context.MemberIdHolder; | ||
import com.votogether.global.log.presentation.RequestLogInterceptor; | ||
import com.votogether.global.log.presentation.RequestResponseCacheFilter; | ||
import java.util.List; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.boot.web.servlet.FilterRegistrationBean; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
import org.springframework.http.HttpHeaders; | ||
import org.springframework.web.cors.CorsConfiguration; | ||
import org.springframework.web.cors.UrlBasedCorsConfigurationSource; | ||
import org.springframework.web.filter.CorsFilter; | ||
import org.springframework.web.method.support.HandlerMethodArgumentResolver; | ||
import org.springframework.web.servlet.config.annotation.CorsRegistry; | ||
import org.springframework.web.servlet.config.annotation.InterceptorRegistry; | ||
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; | ||
|
||
@RequiredArgsConstructor | ||
@Configuration | ||
public class WebMvcConfig implements WebMvcConfigurer { | ||
|
||
|
@@ -16,25 +27,57 @@ public class WebMvcConfig implements WebMvcConfigurer { | |
private static final String DEV_SERVER = "https://dev.votogether.com"; | ||
private static final String PROD_SERVER = "https://votogether.com"; | ||
|
||
private final MemberIdHolder memberIdHolder; | ||
private final TokenProcessor tokenProcessor; | ||
private final RequestLogInterceptor requestLogInterceptor; | ||
private final JwtAuthorizationArgumentResolver jwtAuthorizationArgumentResolver; | ||
|
||
public WebMvcConfig(final JwtAuthorizationArgumentResolver jwtAuthorizationArgumentResolver) { | ||
this.jwtAuthorizationArgumentResolver = jwtAuthorizationArgumentResolver; | ||
@Bean | ||
public FilterRegistrationBean<CorsFilter> corsFilterRegistrationBean() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
혹시 setOrder(0)을 통해 가장 먼저 해당 필터를 거치게 하기 위한 것인지, 그게 맞다면 왜 그렇게 하는 것인지, 아니면 따로 더 설정해줘야 하는 부분들이 있어서 그런지, 아니면 아예 다른 이유가 따로 있는 것인지 궁금해요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개발 환경, 운영 환경에서는 하지만 로컬에서 현재 저희는 인증을 위해 현재 JwtFilter에는 |
||
final CorsConfiguration config = new CorsConfiguration(); | ||
config.setAllowCredentials(true); | ||
config.addAllowedOrigin(LOCALHOST_FRONTEND); | ||
config.addAllowedHeader("*"); | ||
config.addAllowedMethod("*"); | ||
config.setMaxAge(6000L); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로컬에서만 사용되는 기능이기 때문에 큰 의도없이 설정한 값입니다! 해당 값을 어느 정도로 하면 좋을까요? 🤔 |
||
|
||
final UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource(); | ||
source.registerCorsConfiguration("/**", config); | ||
|
||
final FilterRegistrationBean<CorsFilter> filterBean = new FilterRegistrationBean<>(new CorsFilter(source)); | ||
filterBean.setOrder(0); | ||
|
||
return filterBean; | ||
} | ||
|
||
@Bean | ||
public FilterRegistrationBean<RequestResponseCacheFilter> requestResponseCacheFilter() { | ||
final FilterRegistrationBean<RequestResponseCacheFilter> filterRegistrationBean = new FilterRegistrationBean<>(); | ||
filterRegistrationBean.setFilter(new RequestResponseCacheFilter()); | ||
filterRegistrationBean.addUrlPatterns("/*"); | ||
filterRegistrationBean.setOrder(1); | ||
return filterRegistrationBean; | ||
} | ||
|
||
@Bean | ||
public FilterRegistrationBean<JwtAuthenticationFilter> jwtAuthenticationFilter() { | ||
final FilterRegistrationBean<JwtAuthenticationFilter> filterRegistrationBean = new FilterRegistrationBean<>(); | ||
filterRegistrationBean.setFilter(new JwtAuthenticationFilter(memberIdHolder, tokenProcessor)); | ||
filterRegistrationBean.addUrlPatterns("/*"); | ||
filterRegistrationBean.setOrder(2); | ||
return filterRegistrationBean; | ||
} | ||
|
||
@Override | ||
public void addArgumentResolvers(final List<HandlerMethodArgumentResolver> resolvers) { | ||
resolvers.add(jwtAuthorizationArgumentResolver); | ||
public void addInterceptors(final InterceptorRegistry registry) { | ||
registry.addInterceptor(requestLogInterceptor) | ||
.addPathPatterns("/**") | ||
.order(1); | ||
} | ||
|
||
@Override | ||
public void addCorsMappings(final CorsRegistry registry) { | ||
registry.addMapping("/**") | ||
.allowedHeaders("*") | ||
.allowedOrigins(HTTPS_LOCALHOST_FRONTEND, LOCALHOST_FRONTEND, DEV_SERVER, PROD_SERVER) | ||
.allowedMethods("*") | ||
.allowCredentials(true) | ||
.exposedHeaders(HttpHeaders.LOCATION, HttpHeaders.SET_COOKIE); | ||
public void addArgumentResolvers(final List<HandlerMethodArgumentResolver> resolvers) { | ||
resolvers.add(jwtAuthorizationArgumentResolver); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package com.votogether.global.jwt; | ||
|
||
import com.votogether.global.log.context.MemberIdHolder; | ||
import jakarta.servlet.FilterChain; | ||
import jakarta.servlet.ServletException; | ||
import jakarta.servlet.http.HttpServletRequest; | ||
|
@@ -12,11 +13,9 @@ | |
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.HttpHeaders; | ||
import org.springframework.http.HttpMethod; | ||
import org.springframework.stereotype.Component; | ||
import org.springframework.web.filter.OncePerRequestFilter; | ||
|
||
@RequiredArgsConstructor | ||
@Component | ||
public class JwtAuthenticationFilter extends OncePerRequestFilter { | ||
|
||
private static final List<String> ALLOWED_URIS = List.of( | ||
|
@@ -43,6 +42,7 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { | |
) | ||
); | ||
|
||
private final MemberIdHolder memberIdHolder; | ||
private final TokenProcessor tokenProcessor; | ||
|
||
@Override | ||
|
@@ -54,6 +54,8 @@ protected void doFilterInternal( | |
final String token = request.getHeader(HttpHeaders.AUTHORIZATION); | ||
final String tokenWithoutType = tokenProcessor.resolveToken(token); | ||
tokenProcessor.validateToken(tokenWithoutType); | ||
final TokenPayload tokenPayload = tokenProcessor.parseToken(tokenWithoutType); | ||
memberIdHolder.setId(tokenPayload.memberId()); | ||
Comment on lines
+57
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
filterChain.doFilter(request, response); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package com.votogether.global.log.aop; | ||
|
||
import com.votogether.global.log.context.QueryCount; | ||
import java.sql.PreparedStatement; | ||
import lombok.RequiredArgsConstructor; | ||
import org.aopalliance.intercept.MethodInterceptor; | ||
import org.aopalliance.intercept.MethodInvocation; | ||
import org.springframework.aop.framework.ProxyFactory; | ||
|
||
@RequiredArgsConstructor | ||
public class ConnectionMethodInterceptor implements MethodInterceptor { | ||
|
||
private final QueryCount queryCount; | ||
|
||
@Override | ||
public Object invoke(final MethodInvocation invocation) throws Throwable { | ||
final Object result = invocation.proceed(); | ||
if (result instanceof PreparedStatement ps) { | ||
final ProxyFactory proxyFactory = new ProxyFactory(ps); | ||
proxyFactory.addAdvice(new PreparedStatementMethodInterceptor(queryCount)); | ||
return proxyFactory.getProxy(); | ||
} | ||
return result; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
package com.votogether.global.log.aop; | ||
|
||
import java.util.Objects; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.aspectj.lang.ProceedingJoinPoint; | ||
import org.aspectj.lang.annotation.Around; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.aspectj.lang.annotation.Pointcut; | ||
import org.springframework.stereotype.Component; | ||
import org.springframework.web.context.request.RequestContextHolder; | ||
|
||
@Slf4j | ||
@RequiredArgsConstructor | ||
@Aspect | ||
@Component | ||
public class LogAop { | ||
|
||
private static final String PROXY = "Proxy"; | ||
|
||
private final Logger logger; | ||
|
||
@Pointcut("@within(org.springframework.web.bind.annotation.RestController)") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Pointcut 표현식이 존재하는데, 위의 표현식은 |
||
public void restControllerAnnotatedClass() { | ||
} | ||
|
||
@Pointcut("@within(org.springframework.stereotype.Service)") | ||
public void serviceAnnotatedClass() { | ||
} | ||
|
||
@Pointcut("execution(* com.votogether.domain..*Repository+.*(..))") | ||
public void repositoryClass() { | ||
} | ||
|
||
@Around("restControllerAnnotatedClass() || serviceAnnotatedClass() || repositoryClass()") | ||
public Object doLog(final ProceedingJoinPoint joinPoint) throws Throwable { | ||
if (!isRequestScope()) { | ||
return joinPoint.proceed(); | ||
} | ||
final String className = getClassSimpleName(joinPoint); | ||
final String methodName = getMethodName(joinPoint); | ||
logger.methodCall(className, methodName); | ||
try { | ||
final Object result = joinPoint.proceed(); | ||
logger.methodReturn(className, methodName); | ||
return result; | ||
} catch (Throwable e) { | ||
logger.throwException(className, methodName, e); | ||
throw e; | ||
} | ||
} | ||
|
||
private boolean isRequestScope() { | ||
return Objects.nonNull(RequestContextHolder.getRequestAttributes()); | ||
} | ||
|
||
private String getClassSimpleName(final ProceedingJoinPoint joinPoint) { | ||
final Class<?> clazz = joinPoint.getTarget().getClass(); | ||
String className = clazz.getSimpleName(); | ||
if (className.contains(PROXY)) { | ||
className = clazz.getInterfaces()[0].getSimpleName(); | ||
} | ||
return className; | ||
} | ||
|
||
private String getMethodName(final ProceedingJoinPoint joinPoint) { | ||
return joinPoint.getSignature().getName(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package com.votogether.global.log.aop; | ||
|
||
import com.votogether.global.log.context.LogContext; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.springframework.stereotype.Component; | ||
|
||
@Slf4j | ||
@RequiredArgsConstructor | ||
@Component | ||
public class Logger { | ||
|
||
private static final String DOT = "."; | ||
private static final String PARENTHESES = "()"; | ||
private static final String CALL_PREFIX = "--->"; | ||
private static final String RETURN_PREFIX = "<---"; | ||
private static final String EX_PREFIX = "<X--"; | ||
|
||
private final LogContext logContext; | ||
|
||
public void methodCall(final String className, final String methodName) { | ||
logContext.increaseMethodCall(); | ||
log.info("[{}] {}", | ||
logContext.getLogId(), | ||
formattedClassAndMethod(logContext.depthPrefix(CALL_PREFIX), className, methodName) | ||
); | ||
} | ||
|
||
public void methodReturn(final String className, final String methodName) { | ||
final long currentTimeMillis = System.currentTimeMillis(); | ||
log.info("[{}] {} - [execution time = {}ms] [total time = {}ms]", | ||
logContext.getLogId(), | ||
formattedClassAndMethod(logContext.depthPrefix(RETURN_PREFIX), className, methodName), | ||
logContext.executionTime(currentTimeMillis), | ||
logContext.totalTakenTime(currentTimeMillis) | ||
); | ||
logContext.decreaseMethodCall(); | ||
} | ||
|
||
public void throwException(final String className, final String methodName, final Throwable exception) { | ||
final long currentTimeMillis = System.currentTimeMillis(); | ||
log.warn("[{}] {} - [execution time = {}ms] [total time = {}ms] [throws {}]", | ||
logContext.getLogId(), | ||
formattedClassAndMethod(logContext.depthPrefix(EX_PREFIX), className, methodName), | ||
logContext.executionTime(currentTimeMillis), | ||
logContext.totalTakenTime(currentTimeMillis), | ||
exception.getClass().getSimpleName() | ||
); | ||
logContext.decreaseMethodCall(); | ||
} | ||
|
||
private String formattedClassAndMethod(final String prefix, final String className, final String methodName) { | ||
return prefix + className + DOT + methodName + PARENTHESES; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package com.votogether.global.log.aop; | ||
|
||
import com.votogether.global.log.context.QueryCount; | ||
import java.lang.reflect.Method; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.Set; | ||
import lombok.RequiredArgsConstructor; | ||
import org.aopalliance.intercept.MethodInterceptor; | ||
import org.aopalliance.intercept.MethodInvocation; | ||
import org.springframework.web.context.request.RequestContextHolder; | ||
|
||
@RequiredArgsConstructor | ||
public class PreparedStatementMethodInterceptor implements MethodInterceptor { | ||
|
||
private static final Set<String> QUERY_METHODS = new HashSet<>(List.of("execute", "executeQuery", "executeUpdate")); | ||
|
||
private final QueryCount queryCount; | ||
|
||
@Override | ||
public Object invoke(final MethodInvocation invocation) throws Throwable { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
다즐의 의견이 궁금합니다 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 같이 얘기를 나눴던 대로 하나의 로직으로 합쳐도 괜찮을 것 같다는 생각이 들어요 ! 2가지 방법 모두 괜찮은 방법이라고 생각이 들어서 결정하기 힘들기 때문에 우선은 그대로 가는 방향이 좋을 것 같아요 ㅎㅎ |
||
if (isExecuteQuery(invocation.getMethod()) && isRequestScope()) { | ||
queryCount.increase(); | ||
} | ||
return invocation.proceed(); | ||
} | ||
|
||
private boolean isExecuteQuery(final Method method) { | ||
final String methodName = method.getName(); | ||
return QUERY_METHODS.contains(methodName); | ||
} | ||
|
||
private boolean isRequestScope() { | ||
return Objects.nonNull(RequestContextHolder.getRequestAttributes()); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package com.votogether.global.log.aop; | ||
|
||
import com.votogether.global.log.context.QueryCount; | ||
import org.aspectj.lang.ProceedingJoinPoint; | ||
import org.aspectj.lang.annotation.Around; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.springframework.aop.framework.ProxyFactory; | ||
import org.springframework.stereotype.Component; | ||
|
||
@Aspect | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
해당 클래스 내부에는 어떤 행위를 수행할지에 대한 |
||
@Component | ||
public class QueryCountAop { | ||
|
||
private final QueryCount queryCount; | ||
|
||
public QueryCountAop(final QueryCount queryCount) { | ||
this.queryCount = queryCount; | ||
} | ||
|
||
@Around("execution(* javax.sql.DataSource.getConnection())") | ||
public Object getConnection(final ProceedingJoinPoint joinPoint) throws Throwable { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
final Object connection = joinPoint.proceed(); | ||
final ProxyFactory proxyFactory = new ProxyFactory(connection); | ||
proxyFactory.addAdvice(new ConnectionMethodInterceptor(queryCount)); | ||
return proxyFactory.getProxy(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package com.votogether.global.log.context; | ||
|
||
import java.util.UUID; | ||
|
||
public class AnonymousLogId implements LogId { | ||
|
||
private static final String POSTFIX = "(anonymous)"; | ||
|
||
private final String id; | ||
|
||
public AnonymousLogId() { | ||
this.id = UUID.randomUUID().toString().substring(0, 8); | ||
} | ||
|
||
Comment on lines
+11
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 큰 의도는 존재하지 않았습니다 😀 홀수보다는 안정성 있는 짝수로 설정하고 싶었고, 8자리 정도면 같은 사용자가 여러 요청을 동시에 보내더라도 랜덤하게 설정된 값으로 유니크하지 않을까라는 생각으로 8자리를 선택했던 것 같습니다 ㅎㅎ |
||
@Override | ||
public String getId() { | ||
return id + POSTFIX; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q
어떤 이유로 webflux의존성을 추가하셨나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
운영 환경에서
error
레벨의 로그가 남겨지면 즉각 대응이 필요한 상황이기에 이를 바로 알리기 위해 슬랙 알림이 필요하였습니다!저희는
Logback
이 아닌Log4j2
를 사용하고 있기 때문에Logback
의SlackAppender
라이브러리를 사용할 수 없었습니다.Log4j2
도SlackAppender
라이브러리가 존재하는지 찾아보았으나, 오래된 라이브러리만 존재했고 적절한 라이브러리를 찾을 수 없어서 직접 구현해서 사용하기로 결정하였습니다. 마침 직접 구현해서 사용한 분이 정리한 글이 있어 해당 글을 참고하였습니다.슬랙 알림을 보내기 위해 HTTP 통신이 필요한 상황에서 3가지의 선택지가 존재했습니다.
RestTemplate
WebClient
FeignClient
RestTemplate, FeignClient는 기본적으로 Blocking 방식으로 동작하기 때문에 슬랙 알림을 위해 제어권을 뺏기게 되는 상황으로 성능에 영향을 줄 수 있습니다. 알림은 비동기로 처리해도 되는 로직이기에 비동기를 지원하는 WebClient의 선택이 필요했고, 이를 사용하기 위해
webflux
의존성을 추가하게 되었습니다 :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존에 존재하는
web
의존성과 충돌이 나지 않을까라는 걱정도 했던 것 같아요.2개의 의존성이 모두 존재하는 경우, 스프링 부트 애플리케이션은
SERVLET
방식을 선택하기 때문에Netty
서버가 아닌Tomcat
서버로 동작하게 되는 것을 문서를 통해 확인할 수 있었고, Tomcat 서버로 가동하면서 WebClient 기술만 사용할 수 있다는 것도 알게 되었습니다!