[feat] log filter 추가 및 aop 수정 #39
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 기존에 aop 있던 비즈니스 로직 전, 후 로깅 로직은 filter로 옮김 (실행 시점 및 세부 로그는 일부 달라짐) - aop에는 커스텀 어노테이션으로 로그 남기는 형태로 하나 만들어둠 - 실행시간 측정하는 용도의 커스텀 어노테이션 생성
linglong67
added
🖥️ BackEnd
linglong67
requested review from
HyunJunSon,
chan99k and
gunsight1
as code owners
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
gunsight1
approved these changes
Jan 9, 2024
| @Override | ||
| public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { | ||
| ContentCachingRequestWrapper req = new ContentCachingRequestWrapper((HttpServletRequest) request); | ||
| ContentCachingResponseWrapper res = new ContentCachingResponseWrapper((HttpServletResponse) response); |
There was a problem hiding this comment.
ServletRequest, ServletResponce가 받아오기만 하고 ContentCachingRequestWrapper, ResponseWrapper가 실제 수행하는 변수라면 변수명 request <-> req 바꿔 쓰는 것이 이어지는 하위 코드들의 가독성에는 더 좋을 것이라 생각듭니다!
There was a problem hiding this comment.
말씀해주신 부분 확인하고, 수정해서 커밋했습니다~!
- 변수명 수정
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
chan99k
approved these changes
Jan 10, 2024
Comment on lines
+14
to
+23
| @Around("@annotation(com.kernel360.global.annotation.LogExecutionTime)") | ||
| public Object logExecutionTime(ProceedingJoinPoint joinPoint) throws Throwable { | ||
| long start = System.currentTimeMillis(); | ||
| Object proceed = joinPoint.proceed(); | ||
|
|
||
| long executionTime = System.currentTimeMillis() - start; | ||
| log.info("##### @LogExecutionTime ##### " + joinPoint.getSignature() + " executed in " + executionTime + "ms"); | ||
|
|
||
| return proceed; | ||
| } |
There was a problem hiding this comment.
특정 로그를 남기도록 커스텀 어노테이션으로 추가하고 싶다면 여기 LogAspect.java 에 메서드를 추가하고 어노테이션을 선언하면 되나요?
There was a problem hiding this comment.
네! 맞습니다~!
커스텀 어노테이션 생성 후에 위 코드처럼 메소드 추가해주시면 되요~
HyunJunSon
approved these changes
Jan 10, 2024
HyunJunSon
approved these changes
Jan 10, 2024
Comment on lines
+18
to
+60
| public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException { | ||
| ContentCachingRequestWrapper request = new ContentCachingRequestWrapper((HttpServletRequest) req); | ||
| ContentCachingResponseWrapper response = new ContentCachingResponseWrapper((HttpServletResponse) res); | ||
| log.info("##### INIT URI: {}", request.getRequestURI()); | ||
|
|
||
| chain.doFilter(request, response); | ||
|
|
||
| // Request | ||
| StringBuilder requestHeaderValues = new StringBuilder(); | ||
| request.getHeaderNames().asIterator().forEachRemaining(headerKey -> { | ||
| String headerValue = request.getHeader(headerKey); | ||
|
|
||
| requestHeaderValues | ||
| .append("[") | ||
| .append(headerKey) | ||
| .append(" : ") | ||
| .append(headerValue) | ||
| .append("] "); | ||
| }); | ||
|
|
||
| String requestBody = new String(request.getContentAsByteArray()); | ||
| String uri = request.getRequestURI(); | ||
| String method = request.getMethod(); | ||
| log.info("##### REQUEST ##### uri: {}, method: {}, header: {}, body: {}", uri, method, requestHeaderValues, requestBody); | ||
|
|
||
| // Response | ||
| StringBuilder responseHeaderValues = new StringBuilder(); | ||
| response.getHeaderNames().forEach(headerKey -> { | ||
| String headerValue = response.getHeader(headerKey); | ||
|
|
||
| responseHeaderValues | ||
| .append("[") | ||
| .append(headerKey) | ||
| .append(" : ") | ||
| .append(headerValue) | ||
| .append("] "); | ||
| }); | ||
|
|
||
| String responseBody = new String(response.getContentAsByteArray()); | ||
| log.info("##### RESPONSE ##### uri: {}, method: {}, header: {}, body: {}", uri, method, responseHeaderValues, responseBody); | ||
|
|
||
| response.copyBodyToResponse(); | ||
| } |
There was a problem hiding this comment.
메소드가 라인수가 조금 무거운거 같은데,
리스폰스, 리퀘스트 로직 따로 메소드로 빼는게 어떨까요?
There was a problem hiding this comment.
제가 이건 리팩토링 고려해보고 수정하게 되면 다음에 다시 올릴게요~!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
공통 Log 처리방식 변경🔨 Modified
🌟 More
📋 커밋 전 체크리스트
🤟🏻 PR로 완료된 이슈
closes #38