feat: API 성능 로깅, 쿼리 별 메트릭 전송 추가 #602
Conversation
- traceId 기반 요청 추적 - 요청/응답 로깅 - CustomExceptionHandler와 중복 로깅 방지 - Actuator 엔드포인트 로깅 제외
- info, warn, error, api_perf 로 로그 파일 분리해서 관리
* feat: 데이터소스 프록시 의존성 및 config 파일 추가 * feat: 데이터 소스 프록시가 metric을 찍을 수 있도록 listener 클래스 추가 * feat: 요청 시 method, uri 정보를 listener에서 활용하기 위해 RequestContext 및 관련 interceptor 추가
- Time.builder 를 사용하면 매번 빌더를 생성하여 비효율적인 문제를 meterRegistry.timer 방식으로 해결
sukangpunch
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307 and
whqtker
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java (1)
51-55: Principal 타입 체크 추가를 고려해주세요.변경 내용:
- Authentication 객체에서 Principal을 SiteUserDetails로 캐스팅
- userId를 추출하여 request attribute에 저장
잠재적 이슈:
- Line 52에서
auth.getPrincipal()을SiteUserDetails로 무조건 캐스팅하면 다른 타입의 principal인 경우ClassCastException발생 가능일반적인 흐름에서는
TokenAuthentication이 항상SiteUserDetails를 반환하겠지만, 방어적 프로그래밍 관점에서 타입 체크를 추가하는 것을 권장합니다.🔎 방어적 타입 체크 추가 제안
private void extractIdFromAuthentication(HttpServletRequest request, Authentication auth) { + Object principal = auth.getPrincipal(); + if (!(principal instanceof SiteUserDetails)) { + return; + } - SiteUserDetails principal = (SiteUserDetails) auth.getPrincipal(); + SiteUserDetails userDetails = (SiteUserDetails) principal; - Long id = principal.getSiteUser().getId(); + Long id = userDetails.getSiteUser().getId(); request.setAttribute("userId", id); }src/main/java/com/example/solidconnection/common/interceptor/ApiPerformanceInterceptor.java (2)
17-19: 사용되지 않는 ROUTE_PATTERN_ATTRIBUTE 정리가 필요합니다.변경 내용:
- Line 17에서
ROUTE_PATTERN_ATTRIBUTE상수 정의- Line 38에서 해당 attribute를 request에 설정
이슈:
ROUTE_PATTERN_ATTRIBUTE가 저장되지만afterCompletion에서 읽혀지지 않습니다- Lines 58-59에서
request.getRequestURI()를 직접 사용- 향후 사용을 위해 준비된 것이거나 리팩터링 과정에서 남은 코드일 수 있습니다
코드 정리를 위해 사용하지 않는 경우 제거하거나, 사용할 계획이라면 주석으로 의도를 명시하는 것을 권장합니다.
Also applies to: 38-39
44-83: API 성능 로깅 로직이 효과적으로 구현되었습니다.변경 내용을 정리하면:
- 요청 시작부터 완료까지의 응답 시간 계산
- 3초(3000ms) 임계값을 기준으로 warn/info 레벨 분리
API_PERF로거와 표준 로거 두 곳에 동시 기록
API_PERF: 구조화된 형식으로 파싱 용이하게 기록- 표준 로거: 사람이 읽기 쉬운 형식으로 기록
구현이 올바르며,
logback-spring.xml의 분리된 로그 파일 전략과 잘 통합되어 있습니다.선택적 개선사항:
- Lines 63-66과 75-78, 그리고 68-70과 80-82에 중복된 로깅 코드가 있습니다
- 로깅 메서드를 별도로 추출하면 코드 중복을 줄이고 유지보수성을 높일 수 있습니다
🔎 로깅 중복 제거 리팩터링 제안
+ private void logPerformance(String method, String uri, long responseTime, int status, boolean isWarning) { + String logLevel = isWarning ? "WARN" : "INFO"; + String apiPerfMessage = "type=API_Performance method_type={} uri={} response_time={} status={}"; + String standardMessage = "[API Performance]: {} {} - {}ms [Status: {}]"; + + if (isWarning) { + API_PERF.warn(apiPerfMessage, method, uri, responseTime, status); + log.warn(standardMessage, method, uri, responseTime, status); + } else { + API_PERF.info(apiPerfMessage, method, uri, responseTime, status); + log.info(standardMessage, method, uri, responseTime, status); + } + } + @Override public void afterCompletion( HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex ) throws Exception { Long startTime = (Long) request.getAttribute(START_TIME_ATTRIBUTE); if(startTime == null) { return; } long responseTime = System.currentTimeMillis() - startTime; String uri = request.getRequestURI(); String method = request.getMethod(); int status = response.getStatus(); - if (responseTime > RESPONSE_TIME_THRESHOLD) { - API_PERF.warn( - "type=API_Performance method_type={} uri={} response_time={} status={}", - method, uri, responseTime, status - ); - - log.warn("[API Performance] {} {} - {}ms [Status: {}]", - method, uri, responseTime, status - ); - - return; - } - - API_PERF.info( - "type=API_Performance method_type={} uri={} response_time={} status={}", - method, uri, responseTime, status - ); - - log.info("[API Performance]: {} {} - {}ms [Status: {}]", - method, uri, responseTime, status - ); + boolean isWarning = responseTime > RESPONSE_TIME_THRESHOLD; + logPerformance(method, uri, responseTime, status, isWarning); }src/main/resources/logback-spring.xml (1)
14-28: 시간 기반 롤링 정책에 크기 제한 추가를 권장합니다.현재는 일별로만 로그 파일이 분리되는데, 트래픽이 많은 경우 하루 동안 로그 파일이 무제한으로 커질 수 있습니다. 다음과 같이 크기 제한을 추가하는 것을 권장합니다:
🔎 크기 기반 롤링 정책 추가 예시
INFO_FILE 앱더를 예로 들면:
<appender name="INFO_FILE" class="ch.qos.logback.core.rolling.RollingFileAppender"> <file>${LOG_PATH}/info/info.log</file> - <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy"> - <fileNamePattern>${LOG_PATH}/info/info.%d{yyyy-MM-dd}.log</fileNamePattern> + <rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy"> + <fileNamePattern>${LOG_PATH}/info/info.%d{yyyy-MM-dd}.%i.log</fileNamePattern> + <maxFileSize>100MB</maxFileSize> <maxHistory>7</maxHistory> + <totalSizeCap>3GB</totalSizeCap> </rollingPolicy>WARN_FILE, ERROR_FILE 앱더에도 동일하게 적용하시면 됩니다.
Also applies to: 31-45, 48-62
src/main/java/com/example/solidconnection/common/listener/QueryMetricsListener.java (1)
28-30: 배치 쿼리에서 첫 번째 쿼리만 사용됩니다.Line 29에서
queries.get(0)로 첫 번째 쿼리의 SQL만 가져와 타입을 판단합니다. 배치 쿼리의 경우 나머지 쿼리들의 타입 정보가 손실됩니다.대부분의 경우 배치 내 쿼리들이 동일한 타입일 것으로 예상되므로 현재 구현도 합리적이지만, 필요하다면 각 쿼리별로 별도 메트릭을 기록하는 것도 고려해볼 수 있습니다.
src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (1)
33-34: 사용자 컨텍스트 로깅이 효과적으로 추가되었습니다!모든 예외 핸들러에서
userId를 로깅하여 에러 발생 시 사용자 추적이 가능해졌습니다:
- 일관된 패턴: 6개 모든 핸들러에서 동일한 방식으로 userId 로깅
- 예외 핸들러별 적용:
- 커스텀 예외 (line 33-34)
- JSON 파싱 예외 (line 47-49)
- 입력값 검증 예외 (line 62, 70)
- 데이터 무결성 예외 (line 83-85)
- JWT 예외 (line 98-101)
- 서버 내부 예외 (line 114-117)
참고:
userId가null일 수 있지만 (인증되지 않은 요청), 로그에 "null"로 표시되는 것은 허용 가능하며 원본 예외를 숨기지 않으므로 현재 구현이 적절합니다.🔎 (선택사항) 가독성 개선 제안
로그 가독성을 약간 높이려면 다음과 같이 null 처리를 명시적으로 할 수 있습니다:
-Long userId = (Long) request.getAttribute("userId"); +Long userId = (Long) request.getAttribute("userId"); +String userIdStr = userId != null ? userId.toString() : "anonymous"; -log.error("커스텀 예외 발생 userId : {} msg: {}", userId, ex.getMessage()); +log.error("커스텀 예외 발생 userId : {} msg: {}", userIdStr, ex.getMessage());다만 현재 구현도 충분히 명확하므로 이는 선택적 개선사항입니다.
Also applies to: 47-49, 62-62, 70-70, 83-85, 98-101, 114-117
src/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.java (1)
36-43: 인터셉터 등록 순서 개선 제안현재 등록 순서는 기능상 문제없이 작동하고 있습니다. 다만 다음과 같은 이유로 순서 조정을 검토할 만한 가치가 있습니다:
현재 동작 분석:
ApiPerformanceInterceptor.preHandle()은 단순히 타이밍 정보만 기록하며, RequestContext에 의존하지 않습니다RequestContextInterceptor.preHandle()이 실행되는 시점이 컨트롤러 실행 전이므로, 쿼리 실행 시점에는 이미 RequestContext가 설정되어 있습니다개선 제안 이유:
- 의존성 설정 후 사용하는 논리적 순서로 정렬하면 코드의 의도가 더 명확해집니다
RequestContextInterceptor→ApiPerformanceInterceptor순서가 "컨텍스트 준비 → 성능 측정 시작"의 흐름을 더 직관적으로 나타냅니다등록 순서 변경 권장:
registry.addInterceptor(requestContextInterceptor) .addPathPatterns("/**"); registry.addInterceptor(apiPerformanceInterceptor) .addPathPatterns("/**");기능상 필수는 아니지만, 코드 가독성과 유지보수성 측면에서 순서 조정을 권장합니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
build.gradlesrc/main/java/com/example/solidconnection/common/config/datasource/DataSourceProxyConfig.javasrc/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.javasrc/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.javasrc/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.javasrc/main/java/com/example/solidconnection/common/interceptor/ApiPerformanceInterceptor.javasrc/main/java/com/example/solidconnection/common/interceptor/RequestContext.javasrc/main/java/com/example/solidconnection/common/interceptor/RequestContextHolder.javasrc/main/java/com/example/solidconnection/common/interceptor/RequestContextInterceptor.javasrc/main/java/com/example/solidconnection/common/listener/QueryMetricsListener.javasrc/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.javasrc/main/resources/logback-spring.xml
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.java (1)
src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (1)
Slf4j(23-123)
src/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.java (2)
src/main/java/com/example/solidconnection/common/listener/QueryMetricsListener.java (1)
RequiredArgsConstructor(15-53)src/main/java/com/example/solidconnection/common/config/datasource/DataSourceProxyConfig.java (1)
RequiredArgsConstructor(12-29)
src/main/java/com/example/solidconnection/common/interceptor/RequestContextInterceptor.java (1)
src/main/java/com/example/solidconnection/common/interceptor/RequestContextHolder.java (1)
RequestContextHolder(3-18)
src/main/java/com/example/solidconnection/common/listener/QueryMetricsListener.java (3)
src/main/java/com/example/solidconnection/common/interceptor/RequestContextHolder.java (1)
RequestContextHolder(3-18)src/main/java/com/example/solidconnection/common/config/datasource/DataSourceProxyConfig.java (1)
RequiredArgsConstructor(12-29)src/main/java/com/example/solidconnection/common/interceptor/RequestContextInterceptor.java (1)
Component(9-35)
src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java (1)
src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (19)
src/main/java/com/example/solidconnection/common/config/datasource/DataSourceProxyConfig.java (2)
12-29: 데이터소스 프록시 설정 잘 구현되었습니다.변경 내용을 정리하면:
@Primary어노테이션을 사용하여 프록시된 DataSource가 애플리케이션 전체에서 사용되도록 보장QueryMetricsListener를 생성자 주입으로 받아 프록시에 연결ProxyDataSourceBuilder를 사용하여 기존 DataSource를 래핑쿼리 메트릭 수집을 위한 구조가 명확하고 올바르게 구현되었습니다.
20-28: 연결 풀 설정이 프록시를 통해 올바르게 전달됩니다.검증 결과, 현재 구현이 완벽하게 올바른 패턴을 따르고 있습니다:
DataSourceProperties에서 설정이 먼저 적용됩니다
props.initializeDataSourceBuilder().build()는 application.yml의 모든 HikariCP 설정을 읽어 DataSource에 적용합니다프록시 래핑은 투명하게 동작합니다
- ProxyDataSourceBuilder(datasource-proxy 1.11.0)는 이미 완전히 구성된 DataSource를 감싼 투명한 데코레이터입니다
- 연결 풀의 설정을 변경하거나 손실하지 않습니다
Spring Boot 3.1.5 + HikariCP 기본 포함
- DataSource는 기본으로 HikariCP를 사용하며, 모든 풀 설정(maxPoolSize, minimumIdle, timeout 등)이 보존됩니다
코드는 Spring Boot 모범 사례를 정확히 따르고 있으므로 추가 조치가 필요하지 않습니다.
src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java (1)
38-38: 인증 성공 후 userId 추출 로직 위치가 적절합니다.변경 내용:
- 인증이 성공하고 SecurityContext에 설정된 직후에 userId를 추출하여 request attribute로 설정
- 이를 통해 HttpLoggingFilter, CustomExceptionHandler 등 다운스트림 컴포넌트에서 userId 활용 가능
요청 추적 및 로깅 인프라와 잘 통합되었습니다.
src/main/java/com/example/solidconnection/common/interceptor/RequestContextInterceptor.java (2)
12-25: 요청 컨텍스트 초기화 로직이 잘 구현되었습니다.변경 내용을 정리하면:
preHandle에서 HTTP method와 best matching path를 캡처RequestContext객체를 생성하여RequestContextHolder의 ThreadLocal에 저장- 이를 통해
QueryMetricsListener가 쿼리 메트릭에 HTTP 컨텍스트를 태깅 가능참고사항:
- Line 19의
bestMatchPath는 핸들러 매핑이 없는 경우(정적 리소스, 에러 등) null일 수 있습니다QueryMetricsListener에서 null 체크 후 "-"로 대체하므로 안전하게 처리됩니다
27-34: ThreadLocal 정리가 올바르게 구현되었습니다.변경 내용:
afterCompletion에서RequestContextHolder.clear()호출- 요청 처리 완료 후 ThreadLocal 값을 제거하여 메모리 누수 방지
중요:
afterCompletion은 예외 발생 시에도 호출되므로 ThreadLocal 정리에 적합한 위치입니다- ThreadLocal 사용 시 반드시 필요한 정리 로직이 정확하게 구현되었습니다
src/main/java/com/example/solidconnection/common/interceptor/ApiPerformanceInterceptor.java (1)
23-42: 요청 시작 시점 기록 로직이 적절합니다.변경 내용을 정리하면:
- 요청 시작 시간(
startTime)을 기록하여 나중에 응답 시간 계산- Request URI와 best matching pattern을 attribute에 저장
afterCompletion에서 성능 메트릭 계산에 사용구현이 명확하고 올바르게 작동합니다.
src/main/java/com/example/solidconnection/common/interceptor/RequestContext.java (1)
1-16: LGTM! 잘 설계된 불변 컨텍스트 객체입니다.HTTP 메서드와 경로를 담는 단순한 불변 데이터 홀더로, ThreadLocal 환경에서 안전하게 사용할 수 있도록 잘 설계되었습니다.
src/main/resources/logback-spring.xml (2)
6-7: 구조화된 로그 패턴으로 파싱이 용이합니다.
traceId를 포함한 키-값 형식의 로그 패턴은 로그 수집 시스템(예: Loki, ELK)에서 파싱하기 좋은 형태입니다.
65-79: API 성능 로그 분리가 잘 구성되었습니다.
additivity=false설정으로 중복 로깅을 방지하고, 별도 파일로 분리하여 성능 분석이 용이하도록 잘 구성되었습니다.PR 목표에서 언급한 대로 Alloy 설정 변경이 필요합니다. 다음 항목을 확인해주세요:
/var/log/spring디렉터리 및 하위 폴더(info/,warn/,error/,api-perf/) 생성 및 권한 설정- Alloy 설정 파일에서 새로운 로그 경로 반영
- 디스크 용량 모니터링 설정 (로그 4개 스트림 * 7일 보관)
src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.java (3)
85-94: userId 로깅이 개인정보 보호 정책을 준수하는지 확인이 필요합니다.응답 로그에
userId를 기록하고 있습니다. 다음 사항을 검토해주세요:
- 규정 준수: GDPR, CCPA 등 적용 가능한 개인정보 보호 규정에서 userId 로깅을 허용하는지 확인
- 로그 보관 정책: 7일간 보관되는 로그에 userId가 포함되어도 문제없는지 확인
- 접근 통제:
/var/log/spring디렉터리에 대한 접근 권한이 적절히 제한되어 있는지 확인userId가 민감 정보로 분류되지 않는다면 문제없지만, 조직의 정책을 확인하는 것이 좋습니다.
40-54: traceId 관리 및 예외 안전성이 우수합니다.다음 항목들이 잘 구현되어 있습니다:
- traceId 생성: 16자 hex 문자열로 충분한 고유성 보장
- 제외 경로 처리: Actuator 엔드포인트는 로깅을 건너뛰지만 traceId는 유지하여 하위 레이어 로깅 지원
- MDC 정리: finally 블록에서
MDC.clear()호출로 ThreadLocal 누수 방지- 중복 로깅 방지:
exceptionHandlerLogged속성으로 CustomExceptionHandler와의 중복 방지Also applies to: 56-69
102-117: URL 디코딩 오류 처리가 안전하게 구현되었습니다.잘못된 URL 인코딩으로 인한
IllegalArgumentException을 catch하여 원본 쿼리 스트링을 반환하는 방식이 적절합니다.src/main/java/com/example/solidconnection/common/listener/QueryMetricsListener.java (2)
44-52: SQL 타입 분류 로직이 적절합니다.간단한 prefix 매칭으로 주요 SQL 타입을 분류하고 있으며, 메트릭 수집 목적으로는 충분합니다. null 처리도 안전하게 구현되어 있습니다.
27-42: 해당 우려사항들은 코드에서 실제 문제가 아닙니다.원본 리뷰의 주요 지적사항들을 검증한 결과, 다음과 같은 사실을 확인했습니다:
경로 카디널리티 위험은 존재하지 않습니다
bestMatchPath는 Spring Framework의HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE에서 가져온 값으로,/users/{id}같은 패턴입니다- 실제 요청 경로(
/users/123같은 값)가 아니므로 카디널리티가 제한됩니다- 메트릭 저장소에 미치는 영향은 미미합니다
타이머 인스턴스 캐싱 최적화는 불필요합니다
- Micrometer는 메서드 이름과 태그 조합(Meter.Id)으로 타이머를 내부적으로 캐싱합니다
meterRegistry.timer()를 동일한 태그로 여러 번 호출해도 캐시된 인스턴스를 반환합니다- 별도의
Map기반 캐시는 필요하지 않습니다현재 구현은 성능 및 카디널리티 측면에서 안전합니다.
Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/common/interceptor/RequestContextHolder.java (1)
4-17: ThreadLocal 정리는 안전하게 이루어집니다.검증 결과, 현재 구현은 메모리 누수 없이 안전하게 작동합니다:
정리 보장 ✓
RequestContextInterceptor의afterCompletion()메서드는 Spring의HandlerInterceptor계약에 따라 요청 처리 완료 시 항상 호출됩니다- 예외 발생 시에도 보장되므로
clear()가 반드시 실행됩니다비동기 작업 처리 ✓
@Async메서드들(UpdateViewCountScheduler,FileUploadService등)은 별도 스레드에서 실행되지만,RequestContextHolder를 사용하지 않습니다RequestContextHolder.getContext()는QueryMetricsListener에서만 호출되며, 이는 요청 스레드 내에서 작동하는 Hibernate 쿼리 리스너입니다- 따라서 비동기 작업과의 상호작용이 없어 ThreadLocal 누수 위험이 없습니다
인터셉터 등록 ✓
RequestContextInterceptor가WebMvcConfig에서addPathPatterns("/**")로 등록되어 모든 경로를 포괄합니다src/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.java (2)
3-5: 관찰성 컴포넌트 의존성 주입이 적절합니다!새로운 필터와 인터셉터들이 깔끔하게 추가되었습니다:
- HttpLoggingFilter - 요청/응답 로깅
- ApiPerformanceInterceptor - API 성능 측정
- RequestContextInterceptor - 요청 컨텍스트 전파
생성자 주입 방식으로 의존성이 명확하게 관리되고 있어 좋습니다.
Also applies to: 10-11, 13-13, 15-15, 24-26
45-51: 로깅 필터 등록이 적절합니다!
HttpLoggingFilter를HIGHEST_PRECEDENCE로 등록한 것이 좋습니다:
- 우선순위 설정: 가장 높은 우선순위로 설정되어 모든 요청/응답을 빠짐없이 로깅할 수 있습니다
- 명시적 등록:
FilterRegistrationBean을 사용하여 필터 설정을 명확하게 제어하고 있습니다- traceId 기반 추적: PR 목표에 맞게 요청 시작부터 종료까지 추적이 가능하도록 구성되었습니다
src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (2)
28-31: 예외 핸들러에 HttpServletRequest 추가가 일관되게 적용되었습니다!모든 예외 핸들러에
HttpServletRequest가 추가되어 요청 컨텍스트에 접근할 수 있게 되었습니다:
- handleCustomException (line 28-31)
- handleInvalidFormatException (line 42-45)
- handleValidationExceptions (line 57-60)
- handleDataIntegrityViolationException (line 78-81)
- handleJwtException (line 93-96)
- handleOtherException (line 109-112)
Spring의
@ExceptionHandler가 서블릿 객체 주입을 지원하므로 이 방식은 적절하며, 일관된 패턴으로 구현되어 유지보수가 용이합니다.Also applies to: 42-45, 57-60, 78-81, 93-96, 109-112
32-32: 중복 로깅 방지 플래그가 잘 구현되었습니다!모든 예외 핸들러에서
exceptionHandlerLogged플래그를 설정하여HttpLoggingFilter와의 협력이 잘 이루어지고 있습니다:
- 목적: 예외 발생 시 응답이 이미 로깅되었음을 필터에 알려 중복 로깅을 방지
- 일관성: 6개 모든 핸들러에 동일하게 적용
- 통합: PR의 다른 변경사항(HttpLoggingFilter)과 깔끔하게 연동
이 패턴은 분산된 로깅 로직 간의 조율을 명확하게 하여 좋은 설계입니다.
Also applies to: 46-46, 61-61, 82-82, 97-97, 113-113
| // Database Proxy | ||
| implementation 'net.ttddyy:datasource-proxy:1.11.0' | ||
| implementation 'net.ttddyy.observation:datasource-micrometer:1.2.0' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
datasource-proxy net.ttddyy latest version Maven Central 2025
💡 Result:
Latest Maven Central version: 1.11.0 (released July 14, 2025).
Maven dependency:
net.ttddyy
datasource-proxy
1.11.0
Sources: [Maven Repository (1.11.0)] [1], [javadoc.io (1.11.0)] [2], [Maven Central index/metadata] [3].
References
- https://mvnrepository.com/artifact/net.ttddyy/datasource-proxy/1.11.0
- https://javadoc.io/doc/net.ttddyy/datasource-proxy/1.11.0
- https://maven-eu.nuxeo.org/nexus/content/groups/public/net/ttddyy/datasource-proxy/
🌐 Web query:
datasource-micrometer net.ttddyy.observation latest version Maven Central 2025
💡 Result:
The latest Maven Central version is net.ttddyy.observation:datasource-micrometer: 2.0.1. [1][2]
Sources:
- mvnrepository entry for datasource-micrometer 2.0.1. [1]
- Official docs (installation snippet showing version 2.0.1). [2]
🌐 Web query:
datasource-proxy 1.11.0 security vulnerability CVE
💡 Result:
I found no public CVE that specifically affects the net.ttddyy datasource‑proxy 1.11.0 library itself. Maven listings for datasource‑proxy 1.11.0 show reported vulnerabilities coming from its dependencies (examples listed there include CVE-2025-68161, CVE-2024-38819, CVE-2023-6378), not a direct CVE for datasource‑proxy. [1]
Note: There is a separate, unrelated Grafana "datasource proxy" advisory (CVE-2025-3454) that affects Grafana’s server API — not the net.ttddyy datasource‑proxy Java library. Do not confuse the two. [2]
Sources:
- Maven / mvnrepository entry for net.ttddyy:datasource-proxy 1.11.0 (vulnerability list). [1]
- Grafana security advisory (CVE-2025-3454) — unrelated Grafana product issue. [2]
References:
[1] https://mvnrepository.com/artifact/net.ttddyy/datasource-proxy/1.11.0
[2] https://grafana.com/security/security-advisories/cve-2025-3454/
🌐 Web query:
datasource-micrometer 1.2.0 security vulnerability CVE
💡 Result:
I found no public CVE specifically affecting net.ttddyy.observation:datasource-micrometer version 1.2.0. The Maven listing for that artifact/version shows no CVE entry for the artifact itself. [1]
There are unrelated CVEs for other metric libraries (for example CVE-2020-1960 affecting Apache Flink's JMX reporter), but those do not apply to datasource-micrometer. [2][3]
If you want, I can:
- scan OSS advisories (GitHub Security Advisories, NVD) for any recent updates, or
- check your SBOM/dependency tree for transitive vulnerabilities that might affect your build. Which would you prefer?
Sources:
[1] mvnrepository — net.ttddyy.observation:datasource-micrometer 1.2.0.
[2] Micrometer security policy / advisories.
[3] NVD / GitLab advisory — CVE-2020-1960 (example of an unrelated metrics CVE).
데이터소스 프록시 의존성이 추가되었습니다. datasource-micrometer 버전 업그레이드를 권장합니다.
변경 내용:
datasource-proxy:1.11.0- 최신 버전이며, 쿼리 인터셉션을 위한 핵심 프록시 라이브러리입니다.datasource-micrometer:1.2.0- 최신 버전은 2.0.1이므로 업그레이드를 고려하세요. Micrometer와의 통합을 위한 라이브러리입니다.
DataSourceProxyConfig 및 QueryMetricsListener와 연동되어 쿼리별 메트릭 수집을 지원합니다. 현재 datasource-micrometer는 1.2.0 사용 중이지만, 2.0.1이 최신 버전으로 이용 가능합니다. 특정 보안 취약점은 발견되지 않았으나, 최신 버전으로 업그레이드하면 개선 사항과 안정성을 함께 얻을 수 있습니다.
🤖 Prompt for AI Agents
In build.gradle around lines 72 to 74, the datasource-micrometer dependency is
pinned to 1.2.0 but should be upgraded to the current 2.0.1; update the
implementation coordinate from
'net.ttddyy.observation:datasource-micrometer:1.2.0' to
'net.ttddyy.observation:datasource-micrometer:2.0.1' and then run a quick build
and tests to verify compatibility (adjust any import or API usages in
DataSourceProxyConfig/QueryMetricsListener if the newer API changed).
There was a problem hiding this comment.
@whqtker 정확한 지적 감사합니다! 제 분석이 잘못되었네요.
말씀하신 내용 정리:
-
중복 의존성 제거 필요
datasource-micrometer가 내부적으로datasource-proxy를 포함하고 있어, Line 72의implementation 'net.ttddyy:datasource-proxy:1.11.0'라인은 제거해도 됩니다.
-
버전 1.2.0이 적절함
- 현재 프로젝트가 Spring Boot 3.1.5를 사용 중이므로,
datasource-micrometer:1.2.0이 올바른 버전입니다. - 2.x 버전은 Spring Boot 4.x용이므로 업그레이드하면 안 됩니다.
- 현재 프로젝트가 Spring Boot 3.1.5를 사용 중이므로,
참고 자료 제공해주셔서 이해가 명확해졌습니다. 중복 의존성을 제거하면 의존성 관리가 더 깔끔해질 것 같습니다!
✏️ Learnings added
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 602
File: build.gradle:72-74
Timestamp: 2026-01-02T08:56:21.107Z
Learning: In the solid-connect-server project using Spring Boot 3.1.5, datasource-micrometer 1.2.x is the correct version. Version 2.x is for Spring Boot 4.x and should not be used with Spring Boot 3.x.
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 602
File: build.gradle:72-74
Timestamp: 2026-01-02T08:56:21.107Z
Learning: The datasource-micrometer dependency (net.ttddyy.observation:datasource-micrometer) internally includes datasource-proxy as a transitive dependency, so explicit datasource-proxy dependency is redundant and should be removed to avoid duplication.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.java
Outdated
Show resolved
Hide resolved
- principal 조회 예외를 막기 위해 siteUserDetailsService given 추가
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java (2)
src/main/java/com/example/solidconnection/security/authentication/TokenAuthentication.java (1)
TokenAuthentication(7-42)src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java (3)
4-5: 테스트에 필요한 import 추가가 적절합니다.새로운 테스트 설정과 mocking을 위해 필요한 import들이 올바르게 추가되었습니다.
Also applies to: 11-11, 13-14
41-42: 테스트 픽스처 사용이 좋습니다.
SiteUserFixture를 활용하여 테스트 데이터를 생성하는 것은 재사용성과 일관성 측면에서 좋은 접근입니다.
56-60: Mock 설정이 적절합니다.
setUp에서SiteUserDetailsService의 mock을 구성하여 모든 테스트에서 일관된 사용자 정보를 제공하도록 한 것이 좋습니다.anyString()을 사용한 것은 이 테스트 범위에서는 적절합니다.
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java
Show resolved
Hide resolved
* test: 중복되는 테스트 제거 * refactor: 사용하지 않는 필드 제거
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.java (1)
3-3: 사용되지 않는 ObjectMapper import를 제거해주세요.
ObjectMapper클래스를 import하고 있지만, 코드 내에서 사용되지 않습니다.🔎 제안하는 수정
-import com.fasterxml.jackson.databind.ObjectMapper;
🧹 Nitpick comments (1)
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java (1)
77-77: 이전 리뷰 피드백이 잘 반영되었습니다.중복 테스트를 제거하고 기존 테스트를 확장하여 다음 항목들을 검증하도록 개선했습니다:
- SecurityContext에 인증 정보가 저장되는지 확인
- Request 속성에 userId가 설정되는지 확인
테스트 메서드명도 검증 내용을 정확히 반영하도록 업데이트되었습니다.
선택 사항: 토큰 생성 메서드 파라미터화 고려
Line 96의
createTokenWithExpiration메서드에서 subject를 하드코딩("1")하고 있는데, 이를 파라미터로 받도록 수정하면expectedUserId와의 관계가 더 명확해질 수 있습니다.🔎 선택적 리팩토링 제안
@Test void 토큰이_있으면_컨텍스트에_저장하고_userId를_request에_설정한다() throws Exception { // given Long expectedUserId = 1L; Date validExpiration = new Date(System.currentTimeMillis() + 1000); - String token = createTokenWithExpiration(validExpiration); + String token = createTokenWithExpiration(String.valueOf(expectedUserId), validExpiration); request = createRequestWithToken(token); // when tokenAuthenticationFilter.doFilterInternal(request, response, filterChain); // then assertThat(SecurityContextHolder.getContext().getAuthentication()) .isExactlyInstanceOf(TokenAuthentication.class); assertThat(request.getAttribute("userId")).isEqualTo(expectedUserId); then(filterChain).should().doFilter(request, response); } - private String createTokenWithExpiration(Date expiration) { + private String createTokenWithExpiration(String subject, Date expiration) { return Jwts.builder() - .setSubject("1") + .setSubject(subject) .setIssuedAt(new Date()) .setExpiration(expiration) .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) .compact(); }Also applies to: 79-79, 90-90
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.javasrc/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java (2)
src/main/java/com/example/solidconnection/security/authentication/TokenAuthentication.java (1)
TokenAuthentication(7-42)src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.java (1)
src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (1)
Slf4j(23-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/test/java/com/example/solidconnection/security/filter/TokenAuthenticationFilterTest.java (3)
4-5: 추가된 import 문들이 적절합니다.Mockito의
anyString(),given()메서드와 함께SiteUserDetails,SiteUser,SiteUserFixture를 추가하여 향상된 테스트 설정을 지원합니다. 모든 import가 실제로 사용되고 있어 불필요한 의존성이 없습니다.Also applies to: 11-11, 13-14
41-42: Fixture 활용이 좋습니다.
SiteUserFixture를 Autowire하여 테스트 데이터 생성을 표준화하고 일관성을 확보했습니다. 테스트 유지보수성이 향상됩니다.
56-60: Mock 설정이 정확합니다.
SiteUserDetailsService를 적절히 모킹하여 토큰 인증 흐름을 테스트 가능하게 만들었습니다. 생성된 사용자(ID=1)가 토큰의 subject("1") 및 예상 userId(1L)와 일관되게 정렬되어 있습니다.src/main/java/com/example/solidconnection/common/filter/HttpLoggingFilter.java (2)
32-67: 필터 로직이 잘 구현되어 있습니다.다음과 같은 흐름이 명확하고 안전하게 처리되고 있습니다:
- traceId 생성 및 MDC 설정 - 모든 요청에 추적 ID 부여
- 제외 경로 처리 -
/actuator/**같은 경로는 로깅 제외하되 traceId는 유지 (하위 레이어 로그에 활용)- 조건부 응답 로깅 - 예외 핸들러에서 이미 로깅했으면 중복 로깅 방지
- MDC 정리 - finally 블록에서 확실한 정리로 메모리 누수 방지
특히
exceptionHandlerLogged속성을 활용한 중복 로깅 방지 로직이CustomExceptionHandler와 잘 협력하고 있습니다.
69-115: 헬퍼 메서드들이 안전하게 구현되어 있습니다.각 메서드가 명확한 단일 책임을 가지고 있으며, 특히 다음 부분들이 잘 처리되었습니다:
decodeQuery메서드 (lines 100-109) - URL 디코딩 실패 시 원본 쿼리 문자열 반환하는 방어적 프로그래밍generateTraceId메서드 (lines 79-81) - UUID 기반 16자 추적 ID 생성buildDecodedRequestUri메서드 (lines 111-115) - 쿼리 문자열 유무를 안전하게 처리
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
정말 고생 많으셨습니다 ㅠㅠ 궁금한 점 등 리뷰 남겼습니다 ~!
| try { | ||
| filterChain.doFilter(request, response); | ||
|
|
||
| Boolean alreadyExceptionLogging = (Boolean) request.getAttribute("exceptionHandlerLogged"); |
There was a problem hiding this comment.
CustomExceptionHandler에서의 로그와 HttpLoggingFilter에서의 로그 형태가 서로 다른 목적을 가지고 있다고 생각해서 개인적으로 둘 다 있으면 좋을 거 같긴 합니다 ..! 음 근게 이건 다른 분들의 의견도 궁금하긴 하네요
There was a problem hiding this comment.
추가로 exceptionHandlerLogged를 상수로 분리하면 좋을 거 같은데 ... CustomExceptionHandler에도 사용되어서 조금 까다롭네요 ㅠㅠ
| ) { | ||
| request.setAttribute("exceptionHandlerLogged", true); | ||
| Long userId = (Long) request.getAttribute("userId"); | ||
| log.error("커스텀 예외 발생 userId : {} msg: {}", userId, ex.getMessage()); |
There was a problem hiding this comment.
예외 로그에 userId를 추가로 받도록 하신 이유가 있나요 ?
|
|
||
| if(bestMatchingPattern instanceof String s) { | ||
| request.setAttribute(ROUTE_PATTERN_ATTRIBUTE, s); | ||
| } |
There was a problem hiding this comment.
제가 확인한 바로는 routePattern 속성이 저장은 되나 사용되지 않는 거 같은데, 그렇다면 제거해도 무방할 듯 합니다 !
| String method = request.getMethod(); | ||
| int status = response.getStatus(); | ||
|
|
||
| if (responseTime > RESPONSE_TIME_THRESHOLD) { |
There was a problem hiding this comment.
개인적으론 if-else 구조가 로직을 이해하는 데 더 좋을 거 같습니다 !
| .addPathPatterns("/**"); | ||
|
|
||
| registry.addInterceptor(requestContextInterceptor) | ||
| .addPathPatterns("/**"); |
There was a problem hiding this comment.
필터에서 /actuator 경로에 대해 처리하지 않도록 설정되었고, 필터가 최우선적으로 실행되긴 하지만 인터셉터 등록 시에도 명시적으로 /actuator 경로를 무시하도록 하는 것이 좋을 거 같습니다 !
2 participants
관련 이슈
작업 내용
1. 로깅 필터 추가
- traceId 와 request response 로깅을 통해 , api의 시작과 끝에 대한 로그를 추적하기 쉽게 하기 위해 추가하였습니다.
2. Api 수행시간 interceptor
- api 별 수행시간을 로그로 남기기 위해 추가하였습니다.
3. 데이터 소스 프록시 추가 및 쿼리 별 수행시간 메트릭 저장 기능 추가
- 각 api 에서 발생하는 쿼리들의 수행 시간을 메트릭으로 전송합니다.
- 로그로 남기기엔 너무 용량이 커질 듯 하여 메트릭만 전송합니다.
4. 로그 파일 전략 수정
- 로그 폴더를 info, warn, error, api_perf 로 관리하여 로그 파일의 복잡도를 낮추는 방향으로 수정 해 보았습니다.
특이 사항
log 파일 저장 구조가 변경되어 alloy 파일의 수정이 필요합니다.
쿼리 메트릭을 대시 보드에서 찍기 위한 설정이 필요합니다.
리뷰 요구사항 (선택)