Skip to content

Conversation

@7zrv
Copy link
Collaborator

@7zrv 7zrv commented Jan 5, 2025

📌 과제 설명

로그 기록, 가시화를 위한 loki의 의존성 추가와
Logback, logging AOP의 리팩토링

👩‍💻 요구 사항과 구현 내용

  • log 파일 gitignore 설정 추가
  • Loki 의존성 추가
  • logback.xml에 Loki 설정 추가
  • logback.xml 리팩토링
  • LoggingAspect의 리팩토링과 각 책임에 따른 클래스 분리
  • 민감한 데이터의 마스킹 처리 메서드 생성
  • Log 공통 응답 객체 생성

✅ PR 포인트 & 궁금한 점

스크린샷 2025-01-05 오후 11 10 12 스크린샷 2025-01-05 오후 11 09 57

현재 위와 같은 형태로 로그를 기록하고 있습니다
패널 하나로 전체 로그를 볼지 도메인별 패널을 만들어 도메인별 로그를 볼 지 고민중입니다.
모두를 위한 편의 기능인 만큼 현재 정보 외에도 이런 정보가 있으면 좋을것 같다! 같은 자유로운 의견 주시면 감사하겠습니다

7zrv added 7 commits January 5, 2025 22:44
-root/logs 디렉토리의 ignore 설정
- 로그 저장, 시각화를 위한 loki 의존성 추가
- loki 관련 설정 추가
- 로그 패턴의 단일화 적용
- 파일 저장 appender의 설정 변경
- Extractor 클래스의 테스트 커버리지 예외 조건 추가
- 로그 기록 세분화
- 민감한 데이터 마스킹 처리
- 책임에 따른 클래스 분리
- 마스킹 테스트및 검증 완
- 로그 응답에 대한 Response 객체 생성
- ParameterExtractor의 생성자 코드 제거및 RequiredArgs 어노테이션 추가
- ResponseExtractor의 생성자 코드 제거및 RequiredArgs 어노테이션 추가
- SensitiveDataMasker의 불필요한 어노테이션 제거
@7zrv 7zrv self-assigned this Jan 5, 2025
@7zrv 7zrv removed the Feature label Jan 5, 2025
@7zrv 7zrv changed the title Feature/252 Logging 설정 추가및 리팩토링 Refactor/252 Logging 설정 추가및 리팩토링 Jan 5, 2025
- logback-spring.xml 파일의 마지막 라인에 개행 추가
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2025

Copy link
Collaborator

@ayoung-dev ayoung-dev left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 신기하네용

도메인별로 패널이 있는 것도 좋을 거 같아요!
추가되었으면 하는 부분은 계속 생각해보겠습니다

Copy link
Collaborator

@leebs0521 leebs0521 left a comment

Choose a reason for hiding this comment

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

Good

@7zrv 7zrv merged commit 7c3deae into main Jan 6, 2025
3 checks passed
@7zrv 7zrv deleted the feature/252-add-log branch January 6, 2025 07:00
Copy link
Collaborator

@m-a-king m-a-king left a comment

Choose a reason for hiding this comment

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

수고하셨습니다. 제가 스프링 AOP 개념이 약해서 완벽히 이해하기가 쉽지 않네요 ㅎㅎ

public HttpServletRequest getCurrentRequest() {
ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
if (attributes == null) {
throw new IllegalStateException("요청을 찾을수 없습니다.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

오타 발견했습니다 ㅎㅎ

throw new IllegalStateException("요청을 찾을수 없습니다.");
throw new IllegalStateException("요청을 찾을 수 없습니다.");

Comment on lines +41 to +51
public HttpStatus extractExceptionStatus(Exception e) {
if (e instanceof BadRequestException ||
e instanceof ImageUploadException ||
e instanceof DuplicateException ||
e instanceof MethodArgumentNotValidException) {
return HttpStatus.BAD_REQUEST;
} else if (e instanceof NoSuchElementException) {
return HttpStatus.NOT_FOUND;
}
return HttpStatus.INTERNAL_SERVER_ERROR;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

상위 커스텀 예외 클래스를 활용하는 방안도 있을 것 같습니다.
더 확장될 일이 없다면 지금도 좋아보입니다!

또, BadRequestException 예외가 너무 넓은 부분을 처리하지 않는지도 생각해볼 수 있을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

또, BadRequestException 예외가 너무 넓은 부분을 처리하지 않는지도 생각해볼 수 있을 것 같습니다.

BadReq와 여러 Exception을 포함해서 BadReq status 를 반환하는 것에서 찝찝함을 느껴서 코멘트했습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이제 확인했네요 죄송합니다
저도 어색하다고 생각한부분이라 To do 적어놓고 지금 작업 완료후에 바꿔보겠습니다

Comment on lines +14 to +21
private final Set<String> sensitiveFields = new HashSet<>(Arrays.asList(
"password", "token", "secret", "credential", "authorization",
"accessToken", "refreshToken"
));

public Object maskSensitiveData(String fieldName, Object value, ObjectMapper objectMapper) throws JsonProcessingException {
if (isSensitiveField(fieldName)) {
return "********";
Copy link
Collaborator

Choose a reason for hiding this comment

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

신기해요~~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants