-
Notifications
You must be signed in to change notification settings - Fork 5
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
[Feature] - 로깅 프레임워크 적용 #177
Conversation
Test Results52 tests 51 ✅ 4s ⏱️ Results for commit 47b7f4d. ♻️ This comment has been updated with latest results. |
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.
클로버 고생하셨습니다 👍
간단한 질문 남겼어요 확인 부탁드립니다 🙇🏻♂️
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/h2-console/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.POST, "/h2-console/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/favicon/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/swagger-ui/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/swagger-resources/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/v3/api-docs/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.OPTIONS, "/**") |
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.
요고 괜찮은가요? 저희 상위 패키지 명이 바뀌었는데 🤔
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.
좋은지적 감사합니다리
바로 반영 완료 굿
@Slf4j | ||
@Component | ||
public class LoggingFilter extends OncePerRequestFilter { | ||
|
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.
인터셉터 말고 필터에 한 이유 간단히 설명 들을 수 있을까요?
참고하신 링크 정도만 주셔도 좋을 것 같습니다.
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.
Logging은 모든 응답에 대해 기록돼야하므로 모든 요청에 전역적인 적용이 되는 Filter를 활용했습니다!
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.
리비 코멘트 반영 완료!
좋은 리뷰 감사합니다람쥐
@Slf4j | ||
@Component | ||
public class LoggingFilter extends OncePerRequestFilter { | ||
|
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.
Logging은 모든 응답에 대해 기록돼야하므로 모든 요청에 전역적인 적용이 되는 Filter를 활용했습니다!
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/h2-console/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.POST, "/h2-console/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/favicon/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/swagger-ui/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/swagger-resources/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.GET, "/v3/api-docs/**"), | ||
new woowacourse.touroot.global.auth.dto.HttpRequestInfo(HttpMethod.OPTIONS, "/**") |
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.
좋은지적 감사합니다리
바로 반영 완료 굿
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.
로깅 작업 고생하셨습니다! 문제 없어보이네요!
backend/.gitignore
Outdated
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.
EOL!
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.
EOL!
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.
좋은 지적 감삼다
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.
로그 파일이 저장되는 경로를 나중에 팀 문서에 공유 부탁드립니다!
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.
넵! 도커에서는 어디에 저장될지 예상이 안가서 한 번 돌려봐야될 것 같습니다
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.
하나만 확인해 주시면 approve 드릴게요!
backend/.gitignore
Outdated
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.
EOL!
request.setAttribute("memberId", memberId); | ||
request.setAttribute(MEMBER_ID_ATTRIBUTE, memberId); |
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.
레벨 1을 잊지 않으셨군요
log.warn("UNAUTHORIZED_EXCEPTION :: message = {}", message); | ||
|
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.
warn
로그 레벨 도입 좋습니다
if (memberId == null) { | ||
log.info("{} {} ({})", method, url, statusCode); | ||
filterChain.doFilter(request, response); | ||
return; | ||
} | ||
|
||
log.info("{} {} ({}) :: userId = {}", method, url, statusCode, memberId); | ||
filterChain.doFilter(request, response); |
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.
응답은 doFilter 호출 후에 로깅해야 하지 않나요?
한번 확인 부탁 드립니닷!!
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.
뿐만 아니라 에러 응답에 대한 로깅도 ExceptionHandler
말고 여기서 찍으면 어떨까 싶습니다!
여기서 응답 코드에 따라 로그 레벨을 구분해서 로깅하면 좋을 것 같아요!
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.
좋은 지적 감사합니다 낙낙! 200 응답만 확인해봤더니 미처 놓쳤네요
응답에 대한 로깅은 다른 팀원들과 얘기해본 결과
- GlobalExceptionHandler에서는 예외에 대한 정보를 로깅 -> 에외에 대한 정보가 한 곳에 모여있도록
- LoggingFilter에서는 응답에 대한 정보를 로깅
이렇게 역할을 주기로 결정했습니다!
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.
리뷰 반영 완료했습니다! 확인 부탁드려요!
backend/.gitignore
Outdated
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.
좋은 지적 감삼다
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.
넵! 도커에서는 어디에 저장될지 예상이 안가서 한 번 돌려봐야될 것 같습니다
if (memberId == null) { | ||
log.info("{} {} ({})", method, url, statusCode); | ||
filterChain.doFilter(request, response); | ||
return; | ||
} | ||
|
||
log.info("{} {} ({}) :: userId = {}", method, url, statusCode, memberId); | ||
filterChain.doFilter(request, response); |
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.
좋은 지적 감사합니다 낙낙! 200 응답만 확인해봤더니 미처 놓쳤네요
응답에 대한 로깅은 다른 팀원들과 얘기해본 결과
- GlobalExceptionHandler에서는 예외에 대한 정보를 로깅 -> 에외에 대한 정보가 한 곳에 모여있도록
- LoggingFilter에서는 응답에 대한 정보를 로깅
이렇게 역할을 주기로 결정했습니다!
* refactor: local/dev profile에서 show sql false로 변경 * feat: 401 exception logging 추가 및 attribute 이름 상수화 * feat: LoggingFilter 추가 * feat: loggback-spring.xml 작성 * fix: dev profile일 때 파일로 넣게 변경 * fix: logginFilter에서 userId memberId로 수정 * fix: HttpRequestInfo package 변경 * fix: HttpRequestInfo package 변경 * fix: .gitignore EOL 추가 * fix: logging 로직 dofilter 이후로 변경
✅ 작업 내용