slack logger에 access log 정보를 포함한다. #501 #530
Conversation
| @Bean | ||
| public ThreadLocal<ContentCachingRequestWrapper> requestStorage() { | ||
| return new ThreadLocal<>(); |
There was a problem hiding this comment.
아.. 빈을 쓰레드로컬처럼 사용하려고 했군요!!
그렇다면 차라리 빈의 스코프를 다르게 주면 어떨까요?
예를 들어 세션 스코프라던가?? 아마 세션 스코프도 스프링이 내부적으로 ThreadLocal 을 관리할텐데 ThreadLocal 도 잘 닫아줘야 하는데 그 부분을 스프링이 관리해준다면 관리 포인트가 하나 줄어들고 편해지니까요!!
확실한 부분이 아니라 테스트를 하긴 해야겠지만요.. 헤헤
There was a problem hiding this comment.
오 좋은 방법인것 같아요! 안그래도 ThreadLocal 관리하는 부분이 조금 애매했는데 시도해보겠습니다!
There was a problem hiding this comment.
@qhals321 session scope보다는 request scope를 이용하는게 좋을것같아서 변경했습니다!
| if (args[0] instanceof Exception) { | ||
| return true; | ||
| } | ||
| log.warn("[SlackAlarm] argument is not Exception"); |
There was a problem hiding this comment.
로그 찍는 부분을 위에 if 절에 위치하면 조금 더 흐름을 읽는데 편할 것 같은데 어떻게 생각하시나요~?
if (!validateHasOneArgument(args)) {
log.warn("[SlackAlarm] argument is not Exception");
return;
}
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋㅋ 사실 if (!(args[0] instanceof Exception)) 이런식의 로직을 짜기가 싫어서 저렇게 한건데, 저도 거슬리긴 했거든요... 역시나 여기서 같은 스맬을 느끼시는군요... 바꿀께요 흑
| import org.springframework.http.MediaType; | ||
| import org.springframework.web.reactive.function.client.WebClient; | ||
|
|
||
| public class SlackMessage { |
There was a problem hiding this comment.
빈으로 등록하지 않은 이유가 궁금하네요!
빈으로 등록하지 않았을 때는 도메인(순수 자바 코드)처럼 바라보게 되는데 이 객체의 역할은 외부랑도 통신해야하고 외부 라이브러리 관련이 많이 import 가 되어 있네요! 특히 스프링 부분이요! ObjectMapper 도 스프링에서 관리하고 있으니까요! 많은 부분이 스프링과 강한 결합이 있다고 생각이 드네요! 빈으로 등록해서 스프링이 관리하게 하고 send(String message) 를 통해 외부와 통신하게 하면 어떨까 의견을 내봅니다~
There was a problem hiding this comment.
빈 등록도 괜찮아보이네요! 객체간의 협력관계를 생각했을때 지금 설계가 괜찮은것 같아서 진행했는데 역시 성능의 트레이드 오프는 못이기는것같아요
|
|
||
| private static final String SLACK_LOGGER_WEBHOOK_URI = | ||
| System.getenv("SLACK_LOGGER_WEBHOOK_URI"); | ||
| private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); |
There was a problem hiding this comment.
이 부분은 Autowired 로 스프링 ObjectMapper 가져와도 괜찮을 거 같네요~!
bperhaps
force-pushed
the
refactor/slack-logger-with-information
branch
from
1695967
to
40e93c6
Compare
|
SonarCloud Quality Gate failed. |
* feat: 커스텀 slackAppender 뼈대 제작 * feat: ServletRequest 캐싱 로직 구현 * refactor: filter 전략 변경 * feat: Slack 알람 관련 interceptor 등록 * chore: webClient를 위한 webflux 의존성 추가. * feat: 슬랙으로 메세지 전송 기능 구현 * refactor: spring에서 제공해주는 wrapper로 변경 * test: 필요 없는 테스트 제거 * feat: AoP, ThreadLocal 기반으로 변경 * feat: Slack 알림 등록 * refactor: 기존 SlackAppender 제거 * refactor: 사용하지 않는 Appender 제거 * fix: SlackAlarm이 test에서도 전송되는 문제 해결 * refactor: SlackMessage 이름 변경 및 빈등록 * refactor: logger 위치 변경 * refactor: thread local 제거후 request scope 사용 * refactor: 잘못된 token에 대한 에러 핸들링 * refactor: 중복된 flayway 파일 제거 * refactor: objectmapper 주입받아 사용하도록 변경
너무 힘들었습니다.
일단 간단하게 정리하자면,
위와같이 @SlackAlarm 어노테이션을 등록하면 알람이 가도록 구현하였습니다.
AoP와 ThreadLocal을 이용하여 구현하였습니다.request scope 빈과 AoP를 이용했습니다.
기존의 Logback SalckAppender는 request 정보를 가져올 방법이 없어 제거하였습니다!
자세한 내용을 정리 완료되면 올리겠습니다.
close #501