feat: Spring AOP를 활용한 MDC 로깅 아키텍처 구축 #96
Conversation
…pis 모듈에서 사용 가능하도록 추가
…t 및 Pointcut 설정 추가
…름도 로깅할 수 있도록 kotlinOptions.javaParameters = true 옵션 추가
- HTTP 요청 시 traceId, client IP, 요청 정보, 사용자 ID를 MDC에 저장 - JWT 인증 사용자의 subject를 userId로 기록, 인증되지 않은 경우 "GUEST"로 처리 - 요청 처리 완료 후 MDC 클리어하여 다음 요청에 영향 없음 - OncePerRequestFilter 상속으로 각 요청마다 한 번만 실행
…Filter 이후에 배치하여 JWT 인증 후 사용자 정보 MDC에 기록하도록 구현
WalkthroughAOP 기반 로깅 및 MDC 전파 인프라가 추가되어 InfraBaseConfigGroup에 AOP가 포함되고 AopConfig, LoggingAopProperties, Controller/Service 애스펙트, 공통 포인트컷과 NoLogging 애노테이션, MDC 필터·TaskDecorator, Log4j2 설정 및 관련 빌드·의존성 변경이 적용되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as Security FilterChain
participant Bearer as BearerTokenAuthenticationFilter
participant MDCF as MdcLoggingFilter
participant Controller
participant CAspect as ControllerLoggingAspect
participant Service
participant SAspect as ServiceLoggingAspect
Client->>Gateway: HTTP Request
Gateway->>Bearer: 인증 처리
Bearer-->>Gateway: 인증 결과
Gateway->>MDCF: MdcLoggingFilter 실행 (MDC 초기화)
MDCF-->>Gateway: proceed
Gateway->>Controller: 요청 전달
CAspect->>Controller: Around (시작 로그)
Controller->>Service: 서비스 호출
SAspect->>Service: Around (서비스 시작 로그)
Service-->>SAspect: 처리 결과
SAspect-->>Controller: 서비스 성공 로그
Controller-->>CAspect: 응답
CAspect-->>Gateway: 응답 반환
Gateway-->>Client: HTTP Response
note right of MDCF: finally { MDC.clear() }
sequenceDiagram
participant Caller as Thread A
participant Exec as TaskExecutor (MdcTaskDecorator)
participant Task as Thread B (Async)
Caller->>Exec: submit(runnable with MDC)
Exec->>Task: decorate -> wrapper restores MDC
Task-->>Task: run() (MDC available)
Task-->>Task: finally { restore previous/clear MDC }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🔭 Outside diff range comments (3)
infra/src/main/kotlin/org/yapp/infra/config/internal/async/AsyncConfig.kt (1)
43-53: 무제한 큐 기본값으로 인한 메모리 위험 및 확장성 부족ThreadPoolTaskExecutor 기본 queueCapacity가 매우 커서, 트래픽 스파이크 시 큐가 과도하게 쌓일 수 있고, corePoolSize=3에서 확장되지 않을 수 있습니다. maxPoolSize/queueCapacity/keepAliveSeconds/Rejected 정책을 명시적으로 설정하는 것을 권장합니다.
@Bean(name = ["taskExecutor"]) fun taskExecutor(): Executor { val executor = ThreadPoolTaskExecutor() executor.corePoolSize = 3 + executor.maxPoolSize = 6 + executor.setQueueCapacity(1000) + executor.setKeepAliveSeconds(60) executor.setThreadNamePrefix("async-") executor.setWaitForTasksToCompleteOnShutdown(true) executor.setAwaitTerminationSeconds(30) executor.setTaskDecorator(MdcTaskDecorator()) + // 큐가 가득 찼을 때 호출 스레드가 직접 실행하여 백프레셔 유도 + executor.setRejectedExecutionHandler(java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy()) executor.initialize() return executor }실 운영값은 프로파일/프로퍼티로 외부화하는 것도 고려해 주세요.
infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt (2)
1-11: 애스펙트 빈 스캐닝/등록 누락 — 현 상태로는 AOP 로그가 나오지 않을 수 있습니다현재
AopConfig는 프로퍼티 바인딩만 활성화하고 애스펙트 빈을 등록/스캔하지 않습니다. 애스펙트가@Component로 등록되어도 해당 패키지가 스캔 범위에 포함되지 않으면 빈이 생성되지 않습니다. 다음과 같이@EnableAspectJAutoProxy및@ComponentScan(애스펙트 패키지 기준)을 추가하는 것을 권장합니다.package org.yapp.infra.config.internal.aop import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.ComponentScan +import org.springframework.context.annotation.EnableAspectJAutoProxy import org.yapp.infra.InfraBaseConfig import org.yapp.infra.aop.properties.LoggingAopProperties +import org.yapp.infra.aop.aspect.ControllerLoggingAspect +import org.yapp.infra.aop.aspect.ServiceLoggingAspect -@Configuration -@EnableConfigurationProperties(LoggingAopProperties::class) -class AopConfig : InfraBaseConfig +@Configuration +@EnableConfigurationProperties(LoggingAopProperties::class) +@EnableAspectJAutoProxy(proxyTargetClass = true) +@ComponentScan(basePackageClasses = [ControllerLoggingAspect::class, ServiceLoggingAspect::class]) +class AopConfig : InfraBaseConfig참고:
spring-boot-starter-aop의존성이 없으면 위 설정이 있어도 프록시가 활성화되지 않습니다(별도 코멘트 참고).
1-11: kotlin-spring/all-open 플러그인 미적용 확인 — 조치 필요build*.gradle* 파일을 검색한 결과 kotlin-spring / all-open 플러그인이 발견되지 않았습니다. Kotlin은 클래스/메서드가 기본적으로 final이라 Spring AOP(CGLIB) 프록시가 생성되지 않을 수 있습니다. 아래 중 하나로 조치하세요.
주의할 점:
- infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt — 현재 선언:
class AopConfig : InfraBaseConfig(@configuration) — 프록시 생성 실패 가능성- build*.gradle* — 플러그인 미검출 (검색 결과)
권장 수정 (선택 중 하나):
- Gradle에 kotlin-spring 플러그인 적용
- Kotlin DSL (build.gradle.kts) 예시:
plugins { kotlin("plugin.spring") }- Groovy DSL (build.gradle) 예시:
plugins { id 'org.jetbrains.kotlin.plugin.spring' }- 또는 클래스/프록시 대상 메서드를 수동으로 open으로 변경:
변경 전:변경 후:class AopConfig : InfraBaseConfigopen class AopConfig : InfraBaseConfig- (대안) 인터페이스 기반 프록시 사용 또는 @configuration(proxyBeanMethods = false) 등으로 CGLIB 서브클래싱 요구를 제거
위 중 하나를 적용한 후 빌드/실행 환경에서 AOP 동작을 확인해 주세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (19)
apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt(1 hunks)build.gradle.kts(2 hunks)buildSrc/src/main/kotlin/Dependencies.kt(1 hunks)domain/build.gradle.kts(1 hunks)domain/src/main/kotlin/org/yapp/domain/config/CoreDomainConfig.kt(0 hunks)gateway/src/main/kotlin/org/yapp/gateway/filter/MdcLoggingFilter.kt(1 hunks)gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt(2 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/annotation/NoLogging.kt(1 hunks)infra/build.gradle.kts(1 hunks)infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/aop/pointcut/CommonPointcuts.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/aop/properties/LoggingAopProperties.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/async/AsyncConfig.kt(3 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/async/MdcTaskDecorator.kt(1 hunks)infra/src/main/resources/application-persistence.yml(1 hunks)infra/src/main/resources/log4j2-spring.xml(1 hunks)
💤 Files with no reviewable changes (1)
- domain/src/main/kotlin/org/yapp/domain/config/CoreDomainConfig.kt
🧰 Additional context used
🪛 detekt (1.23.8)
global-utils/src/main/kotlin/org/yapp/globalutils/annotation/NoLogging.kt
[warning] 5-5: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt
[warning] 37-37: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 38-38: Do not rethrow a caught exception of the same type.
(detekt.exceptions.RethrowCaughtException)
🔇 Additional comments (5)
infra/src/main/kotlin/org/yapp/infra/config/internal/async/AsyncConfig.kt (1)
14-16: 확인 — kotlin-spring 플러그인(=all-open)이 적용되어 있습니다.root의 build.gradle.kts에서 subprojects { apply(plugin = Plugins.Kotlin.SPRING) }로 모든 서브프로젝트에 플러그인이 적용되어 있어 infra 모듈의 @configuration 클래스가 CGLIB 프록시 대상이므로 추가 조치 불필요합니다.
- 확인 위치:
- build.gradle.kts — subprojects { apply(plugin = Plugins.Kotlin.SPRING) } (약 build.gradle.kts:47)
- build.gradle.kts — plugins.withId(Plugins.Kotlin.ALLOPEN) { ... } (약 build.gradle.kts:65)
- buildSrc/src/main/kotlin/Plugins.kt — ALLOPEN / SPRING 상수 정의
- infra/build.gradle.kts — 별도 적용 없음(서브프로젝트 적용으로 상속)
apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt (1)
16-16: AOP 인프라 그룹 활성화 LGTM
@EnableInfraBaseConfig에InfraBaseConfigGroup.AOP추가로 AOP 설정이 정상적으로 로딩 대상에 포함됩니다.infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt (1)
5-5: AOP 그룹 매핑 추가 적절합니다
AOP(AopConfig::class.java)추가 및 관련 import가 일관되고 명확합니다. 상위 구성에서 해당 그룹을 활성화하면 이 설정이 즉시 반영될 수 있습니다.Also applies to: 19-21
infra/build.gradle.kts (1)
26-26: 테스트 의존성 정리는 무해합니다테스트 의존성 위치 변경/정리는 동작에 영향이 없습니다.
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt (1)
24-29: 프로퍼티 게이트 + NoLogging 배제 포인트컷 구성 좋습니다컨트롤러 로깅을 프로퍼티로 게이팅하고, 공통 포인트컷과
noLogging()을 결합해 배제하는 설계가 명확합니다.
| cache: | ||
| type: simple | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
기본 프로파일에서 simple 캐시 활성화는 운영 환경에 비추천
spring.cache.type: simple은 메모리 기반(ConcurrentMap)으로 만료/통제 기능이 부족해 운영 환경에서 메모리 사용량 증가 및 예측 불가 동작을 유발할 수 있습니다. dev/test 프로파일에서만 사용하도록 이동하는 것을 권장합니다.
dev/test 프로파일로 이동(예시, 파일의 dev/test 블록에 추가):
# dev
---
spring:
config:
activate:
on-profile: dev
cache:
type: simple
# test
---
spring:
config:
activate:
on-profile: test
cache:
type: simple🤖 Prompt for AI Agents
infra/src/main/resources/application-persistence.yml lines 27-29: the file
enables spring.cache.type: simple in the default profile — move that setting out
of the default and into dev and test specific profile sections so simple
(ConcurrentMap) cache is only active for non-prod environments; remove the
current default cache.type entry and add equivalent spring.cache.type: simple
under two profile blocks activated on-profile: dev and on-profile: test, leaving
production/default to use a production-grade cache or no explicit cache.type.
| logging: | ||
| aop: | ||
| service: | ||
| enabled: true | ||
| max-log-length: 1000 | ||
| controller: | ||
| enabled: true | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LoggingAopProperties 바인딩(prefix) 및 설정 파일 로딩 여부 확인 필요
logging.aop.*키들이@ConfigurationProperties("logging.aop")로 선언된LoggingAopProperties에 정상 바인딩되는지 확인해 주세요.- 이 파일(
application-persistence.yml)이 실제 실행 애플리케이션 환경에서 로딩되는지도 점검 필요합니다. 별도의spring.config.import또는 상위application.yml에서의 include가 없다면 이 설정이 반영되지 않을 수 있습니다.
다음 스크립트로 확인해 보세요:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) LoggingAopProperties 선언/프리픽스 확인"
rg -n "class\\s+LoggingAopProperties" -A 5 || true
rg -n "@ConfigurationProperties\\(\"[^\"]+\"\\)" -A 2 || true
echo
echo "2) 해당 YAML이 로딩되는지 확인 (spring.config.import, 파일 참조 등)"
rg -n "application-persistence\\.yml|spring\\.config\\.import" -A 2 || trueLength of output: 1134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) 모든 LoggingAopProperties 참조 검색"
rg -n "LoggingAopProperties" -S || true
echo
echo "2) @ConfigurationProperties / @ConstructorBinding / EnableConfigurationProperties / ConfigurationPropertiesScan 검색"
rg -n "@ConfigurationProperties|@ConstructorBinding|@EnableConfigurationProperties|@ConfigurationPropertiesScan" -S || true
echo
echo "3) @SpringBootApplication 위치 검색 (메인 애플리케이션 클래스 확인용)"
rg -n "@SpringBootApplication" -S || true
echo
echo "4) spring.config.import 및 application-persistence.yml 직접 참조 검색"
rg -n "spring\\.config\\.import|application-persistence\\.yml" -S || true
echo
echo "5) application-*.yml 파일들 목록 (파일명이 프로파일용인지 확인)"
rg -n "application-[a-zA-Z0-9_-]+\\.yml" -S || true
echo
echo "6) 'persistence' 키워드(프로파일로 사용중인지 확인) 전체 검색"
rg -n "persistence" -S || trueLength of output: 6223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- infra/src/main/resources/application-persistence.yml ----"
sed -n '1,240p' infra/src/main/resources/application-persistence.yml || true
echo
echo "---- infra/src/main/kotlin/org/yapp/infra/aop/properties/LoggingAopProperties.kt ----"
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/aop/properties/LoggingAopProperties.kt || true
echo
echo "---- infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt ----"
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt || true
echo
echo "---- apis/src/main/resources/application.yml ----"
sed -n '1,240p' apis/src/main/resources/application.yml || true
echo
echo "---- admin/src/main/resources/application.yml ----"
sed -n '1,240p' admin/src/main/resources/application.yml || true
echo
echo "---- batch/src/main/resources/application.yml ----"
sed -n '1,240p' batch/src/main/resources/application.yml || trueLength of output: 6997
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- apis/src/main/kotlin/org/yapp/apis/ApisApplication.kt ----"
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/ApisApplication.kt || true
echo
echo "---- admin/src/main/kotlin/org/yapp/admin/AdminApplication.kt ----"
sed -n '1,240p' admin/src/main/kotlin/org/yapp/admin/AdminApplication.kt || true
echo
echo "---- batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt ----"
sed -n '1,240p' batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt || true
echo
echo "---- infra/src/main/kotlin/org/yapp/infra/InfraBaseConfig.kt ----"
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/InfraBaseConfig.kt || true
echo
echo "---- infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt ----"
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt || true
echo
echo "---- apis/build.gradle.kts ----"
sed -n '1,240p' apis/build.gradle.kts || true
echo
echo "---- settings.gradle.kts ----"
sed -n '1,240p' settings.gradle.kts || trueLength of output: 4603
logging.aop 바인딩 및 application-persistence.yml 로딩 확인 결과
간단결론: LoggingAopProperties 바인딩 설정은 올바르게 되어 있으나, application-persistence.yml은 명시적 import가 없으면 persistence 프로파일이 활성화될 때만 로드됩니다. 런타임에 persistence 프로파일이 활성화되지 않으면 해당 설정이 반영되지 않습니다.
주의가 필요한 위치:
- infra/src/main/kotlin/org/yapp/infra/aop/properties/LoggingAopProperties.kt — @ConfigurationProperties(prefix = "logging.aop")
- infra/src/main/kotlin/org/yapp/infra/config/internal/aop/AopConfig.kt — @EnableConfigurationProperties(LoggingAopProperties::class)
- infra/src/main/resources/application-persistence.yml — logging.aop 설정 포함 (존재 확인됨)
- apis/src/main/resources/application.yml, admin/src/main/resources/application.yml, batch/src/main/resources/application.yml — spring.profiles.group에 'persistence' 포함(프로파일 그룹으로만 설정됨)
권장 조치(선택):
- 항상 적용되길 원하면 application-persistence.yml 내용을 상위 application.yml로 옮기거나 상위 application.yml에 spring.config.import: classpath:application-persistence.yml 추가.
- 특정 상황에서만 적용되길 원하면 런타임에 spring.profiles.active에 'persistence'(또는 'dev' 등 그룹을 통해 활성화되는 프로파일)를 확실히 설정.
🤖 Prompt for AI Agents
infra/src/main/resources/application-persistence.yml lines 30-37: logging.aop
entries live only in application-persistence.yml which is loaded only when the
'persistence' profile is active, so the logging.aop properties won't be applied
unless that profile is enabled at runtime; to fix, either move the logging.aop
settings into the top-level application.yml or add spring.config.import:
classpath:application-persistence.yml to the top-level application.yml so the
file is always imported, or ensure the 'persistence' profile (or a profile group
that includes it) is set in spring.profiles.active at runtime depending on
whether you want the settings always or conditionally applied.
| %d{yyyy-MM-dd HH:mm:ss.SSS} %-5level [%thread] [%X{traceId}] [%X{userId}] [%X{clientIp}] [%X{requestInfo}] %logger{36} - %msg%n | ||
| </Property> | ||
|
|
||
| <Property name="CONSOLE_LOG_PATTERN"> | ||
| %d{yyyy-MM-dd HH:mm:ss.SSS} %highlight{%-5level}{FATAL=red blink, ERROR=red, WARN=yellow bold, INFO=green, DEBUG=green bold, TRACE=blue} %style{[%thread]}{cyan} %style{[%X{traceId}]}{green} %style{[%X{userId}]}{magenta} %style{[%X{clientIp}]}{blue} %style{[%X{requestInfo}]}{yellow} %style{%logger{36}}{bright,white} - %msg%n | ||
| </Property> |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
userId 노출 수준 검토 필요(PII/개인정보 고려)
MDC에 userId를 그대로 노출하면 로그 집계/전송 구간에서 PII 이슈가 발생할 수 있습니다. 가명화/마스킹(예: 해시) 또는 적절한 레벨/샘플링을 검토해 주세요.
필요하시면 마스킹 유틸/패턴(예: %X{userHash})로 대체하는 방안을 제안드릴 수 있습니다.
userId 로그 마스킹 필요 — PII(개인정보) 유출 위험
MDC에 원본 userId가 그대로 노출되어 있습니다. 로그 집계/전송 과정에서 PII 유출 가능성이 있으므로 가명화(해시)/마스킹 또는 로그 레벨·샘플링 적용을 권장합니다.
-
대상 파일
- infra/src/main/resources/log4j2-spring.xml (스니펫: 7–12행) — %X{userId} 사용
-
제안 변경 (단순 교체 예시: 원본 userId → userHash)
- 기존
%d{yyyy-MM-dd HH:mm:ss.SSS} %-5level [%thread] [%X{traceId}] [%X{userId}] [%X{clientIp}] [%X{requestInfo}] %logger{36} - %msg%n ... %d{yyyy-MM-dd HH:mm:ss.SSS} %highlight{%-5level}{...} %style{[%thread]}{cyan} %style{[%X{traceId}]}{green} %style{[%X{userId}]}{magenta} %style{[%X{clientIp}]}{blue} %style{[%X{requestInfo}]}{yellow} %style{%logger{36}}{bright,white} - %msg%n - 제안
%d{yyyy-MM-dd HH:mm:ss.SSS} %-5level [%thread] [%X{traceId}] [%X{userHash}] [%X{clientIp}] [%X{requestInfo}] %logger{36} - %msg%n ... %d{yyyy-MM-dd HH:mm:ss.SSS} %highlight{%-5level}{...} %style{[%thread]}{cyan} %style{[%X{traceId}]}{green} %style{[%X{userHash}]}{magenta} %style{[%X{clientIp}]}{blue} %style{[%X{requestInfo}]}{yellow} %style{%logger{36}}{bright,white} - %msg%n
- 기존
-
권장 조치
- 애플리케이션에서 MDC에 원본 대신 userHash를 넣도록 변경(예: SHA‑256 해시 또는 영구 가명 ID).
- 로그 수집/전송 파이프라인(ELK/Splunk 등)에서 원본 userId가 저장/전송되지 않는지 확인.
- 필요 시 마스킹 유틸/설정 예시(%X{userHash} 사용) 제공 가능합니다.
🤖 Prompt for AI Agents
In infra/src/main/resources/log4j2-spring.xml around lines 7 to 12, the log
pattern currently emits raw userId via %X{userId}; replace occurrences of
%X{userId} with %X{userHash} in both patterns and ensure the application
populates MDC with a pseudonymous value (e.g., SHA‑256 hash or persistent
pseudonym) instead of the raw userId; also verify downstream logging/ingest
pipelines do not persist original userId and update any docs/configs that
reference the MDC key.
| <Appenders> | ||
| <!-- 콘솔 출력 - 색상 활성화 --> | ||
| <Console name="Console" target="SYSTEM_OUT"> | ||
| <PatternLayout pattern="${CONSOLE_LOG_PATTERN}" disableAnsi="false"/> | ||
| </Console> | ||
|
|
||
| <!-- 파일 출력 - 색상 없음 --> | ||
| <RollingFile name="FileAppender" fileName="logs/application.log" | ||
| filePattern="logs/application-%d{yyyy-MM-dd}-%i.log.gz"> | ||
| <PatternLayout pattern="${FILE_LOG_PATTERN}"/> | ||
| <Policies> | ||
| <TimeBasedTriggeringPolicy interval="1"/> | ||
| <SizeBasedTriggeringPolicy size="100MB"/> | ||
| </Policies> | ||
| <DefaultRolloverStrategy max="10"/> | ||
| </RollingFile> | ||
| </Appenders> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
비동기 로깅으로 API 스레드 차단 최소화 제안
파일/콘솔 출력은 I/O 블로킹을 유발할 수 있습니다. AsyncAppender를 추가해 로깅이 애플리케이션 스레드를 덜 블로킹하도록 개선하는 것을 권장합니다.
<Appenders>
<!-- 콘솔 출력 - 색상 활성화 -->
<Console name="Console" target="SYSTEM_OUT">
<PatternLayout pattern="${CONSOLE_LOG_PATTERN}" disableAnsi="false"/>
</Console>
<!-- 파일 출력 - 색상 없음 -->
<RollingFile name="FileAppender" fileName="logs/application.log"
filePattern="logs/application-%d{yyyy-MM-dd}-%i.log.gz">
<PatternLayout pattern="${FILE_LOG_PATTERN}"/>
<Policies>
<TimeBasedTriggeringPolicy interval="1"/>
<SizeBasedTriggeringPolicy size="100MB"/>
</Policies>
<DefaultRolloverStrategy max="10"/>
</RollingFile>
+
+ <!-- 비동기 Appender 래핑 -->
+ <AsyncAppender name="AsyncConsole" bufferSize="2048" blocking="true">
+ <AppenderRef ref="Console"/>
+ </AsyncAppender>
+ <AsyncAppender name="AsyncFile" bufferSize="8192" blocking="true">
+ <AppenderRef ref="FileAppender"/>
+ </AsyncAppender>
</Appenders>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In infra/src/main/resources/log4j2-spring.xml around lines 15 to 31, the current
Console and RollingFile appenders are synchronous; wrap them in an AsyncAppender
and point the root logger to that AsyncAppender to reduce blocking on API
threads. Add an AsyncAppender element that references the existing "Console" and
"FileAppender" (set a sensible bufferSize, e.g., 1024, and
includeLocation="false"), then replace or update the root logger appender-ref to
use the AsyncAppender name instead of the synchronous appenders.
| <Logger name="org.yapp" level="INFO" additivity="false"> | ||
| <AppenderRef ref="Console"/> | ||
| <AppenderRef ref="FileAppender"/> | ||
| </Logger> | ||
|
|
||
| <!-- 루트 로거 --> | ||
| <Root level="INFO"> | ||
| <AppenderRef ref="Console"/> | ||
| <AppenderRef ref="FileAppender"/> | ||
| </Root> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
AsyncAppender 사용 시 로거 참조 교체
위에서 비동기 래퍼를 추가하면 로거에서 AsyncAppender를 참조하도록 변경하세요.
<Logger name="org.yapp" level="INFO" additivity="false">
- <AppenderRef ref="Console"/>
- <AppenderRef ref="FileAppender"/>
+ <AppenderRef ref="AsyncConsole"/>
+ <AppenderRef ref="AsyncFile"/>
</Logger>
<!-- 루트 로거 -->
<Root level="INFO">
- <AppenderRef ref="Console"/>
- <AppenderRef ref="FileAppender"/>
+ <AppenderRef ref="AsyncConsole"/>
+ <AppenderRef ref="AsyncFile"/>
</Root>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Logger name="org.yapp" level="INFO" additivity="false"> | |
| <AppenderRef ref="Console"/> | |
| <AppenderRef ref="FileAppender"/> | |
| </Logger> | |
| <!-- 루트 로거 --> | |
| <Root level="INFO"> | |
| <AppenderRef ref="Console"/> | |
| <AppenderRef ref="FileAppender"/> | |
| </Root> | |
| <Logger name="org.yapp" level="INFO" additivity="false"> | |
| <AppenderRef ref="AsyncConsole"/> | |
| <AppenderRef ref="AsyncFile"/> | |
| </Logger> | |
| <!-- 루트 로거 --> | |
| <Root level="INFO"> | |
| <AppenderRef ref="AsyncConsole"/> | |
| <AppenderRef ref="AsyncFile"/> | |
| </Root> |
🤖 Prompt for AI Agents
In infra/src/main/resources/log4j2-spring.xml around lines 35 to 44, after
adding the AsyncAppender wrapper you must update the Logger and Root entries to
reference that AsyncAppender rather than the Console and FileAppender directly;
replace the two <AppenderRef ref="Console"/> and <AppenderRef
ref="FileAppender"/> lines in both the org.yapp Logger and the Root logger with
a single <AppenderRef ref="YourAsyncAppenderName"/> (use the actual name of the
AsyncAppender you defined) so logging uses the asynchronous wrapper.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt (2)
32-38: 불필요한 Throwable 캐치/재던지기 제거 (detekt 경고 대응)전역 예외 처리기로 위임한다면 여기서
Throwable을 잡아 동일하게 재던지기는 불필요합니다. detekt의 TooGenericExceptionCaught/RethrowCaughtException 경고도 사라집니다.- try { - val result = joinPoint.proceed() - logRequestSuccess(startTime) - return result - } catch (e: Throwable) { - throw e - } + val result = joinPoint.proceed() + logRequestSuccess(startTime) + return result
50-54: MDC에 이미 있는 traceId/userId를 메시지 파라미터에서 중복 제거로그 패턴에서
%X{traceId} %X{userId}를 찍고 있다면 메시지 파라미터의 동일 값은 중복입니다. 메시지를 간결하게 유지하세요.- log.info( - "[API-REQ] {} {} | Controller: {}.{} | Start At: {} | TraceId: {} | UserId: {}", - httpMethod, uri, className, methodName, startTime, - MDC.get("traceId"), MDC.get("userId") - ) + log.info( + "[API-REQ] {} {} | Controller: {}.{} | Start At: {}", + httpMethod, uri, className, methodName, startTime + )gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt (1)
58-58: MDC 필터는 BearerTokenAuthenticationFilter '이전'에 배치해야 401/403 등 인증 예외 로그에도 traceId가 찍힙니다현재 addFilterAfter로 등록되어 있어 인증 실패 시 EntryPoint/AccessDeniedHandler 로그에 MDC가 초기화되지 않을 수 있습니다. Bearer 토큰 검증보다 앞단에서 MDC를 세팅하도록 순서를 조정해 주세요.
아래처럼 변경 권장:
- .addFilterAfter(mdcLoggingFilter, BearerTokenAuthenticationFilter::class.java) + .addFilterBefore(mdcLoggingFilter, BearerTokenAuthenticationFilter::class.java)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt(1 hunks)
⏰ 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-validation
🔇 Additional comments (2)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt (1)
23-27: 구성 플래그로 로깅 온/오프 처리 LGTM비활성화 시 즉시 proceed()만 호출하는 분기 처리가 명확하고 과도한 오버헤드를 피합니다.
gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt (1)
18-18: hasRole('ADMIN') 매칭 문제 없음 — 수정 불필요간단 설명: JwtConfig.kt에서 authority prefix를 빈 문자열로 설정했지만(authoritiesConverter.setAuthorityPrefix(NO_AUTHORITY_PREFIX)), 토큰 생성 시 roles에 Role.key("ROLE_ADMIN") 형태로 저장됩니다(JwtTokenProvider.kt: roles.map { it.key }). Role enum도 "ROLE_ADMIN"으로 정의되어 있어(global-utils/.../Role.kt) 최종 GrantedAuthority는 "ROLE_ADMIN"이며 hasRole("ADMIN")과 매칭됩니다.
참고 위치:
- gateway/src/main/kotlin/org/yapp/gateway/config/JwtConfig.kt — authorities 설정 (setAuthoritiesClaimName, setAuthorityPrefix)
- gateway/src/main/kotlin/org/yapp/gateway/jwt/JwtTokenProvider.kt — roles.map { it.key }로 토큰에 "ROLE_*" 저장
- global-utils/src/main/kotlin/org/yapp/globalutils/auth/Role.kt — ADMIN("ROLE_ADMIN")
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt
Show resolved
Hide resolved
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
infra/build.gradle.kts (1)
3-26: spring-boot-starter-aop 의존성 추가 필요AOP 기반 로깅이 정상 동작하려면 런타임 클래스패스에 AOP 스타터가 반드시 포함되어야 합니다. 현 모듈의 infra/build.gradle.kts dependencies 블록에 아래와 같이 의존성을 추가해주세요. (상수화된 값이 있다면 해당 상수를 사용하세요.)
- 파일: infra/build.gradle.kts
- 위치:
dependencies { … }블록dependencies { implementation(project(Dependencies.Projects.GLOBAL_UTILS)) implementation(project(Dependencies.Projects.DOMAIN)) implementation(Dependencies.Spring.BOOT_STARTER_WEB) implementation(Dependencies.Spring.BOOT_STARTER_DATA_JPA) implementation(Dependencies.Spring.BOOT_STARTER_DATA_REDIS) implementation(Dependencies.Spring.KOTLIN_REFLECT) + implementation("org.springframework.boot:spring-boot-starter-aop") implementation(Dependencies.RestClient.HTTP_CLIENT5) implementation(Dependencies.RestClient.HTTP_CORE5) implementation(Dependencies.Database.MYSQL_CONNECTOR) implementation(Dependencies.Flyway.MYSQL) implementation(Dependencies.QueryDsl.JPA) kapt(Dependencies.QueryDsl.APT) testImplementation(Dependencies.Spring.BOOT_STARTER_TEST) testImplementation(Dependencies.TestContainers.MYSQL) testImplementation(Dependencies.TestContainers.JUNIT_JUPITER) testImplementation(Dependencies.TestContainers.REDIS) }
♻️ Duplicate comments (12)
infra/build.gradle.kts (1)
20-23: ConfigurationProperties 메타데이터 프로세서 누락
@ConfigurationProperties바인딩 지원 및 IDE 힌트를 위해 configuration-processor 추가를 권장드립니다. 과거 코멘트와 동일 제안입니다.다음 변경을 권장합니다:
implementation(Dependencies.QueryDsl.JPA) kapt(Dependencies.QueryDsl.APT) + kapt("org.springframework.boot:spring-boot-configuration-processor") testImplementation(Dependencies.Spring.BOOT_STARTER_TEST)build.gradle.kts (1)
63-65: 모든 서브모듈에 starter-log4j2 주입은 과함 — 실행 모듈로 한정 권장라이브러리 모듈까지 구현체를 주입하면 결합도와 빌드 시간이 증가합니다. 실행 모듈(예: apis, gateway 등) 한정 혹은 SLF4J API만 노출하는 구성이 일반적입니다.
아래처럼 실행 모듈 화이트리스트로 한정하는 방식을 고려해 주세요(예시는 모듈명은 실제에 맞게 조정 필요):
- dependencies { - implementation(Dependencies.Spring.STARTER_LOG4J2) - } + // 실행 모듈에만 로깅 구현체 주입 (필요에 맞게 목록 조정) + val appModules = setOf("apis", "gateway") + if (name in appModules) { + dependencies { + add("implementation", Dependencies.Spring.STARTER_LOG4J2) + } + }global-utils/src/main/kotlin/org/yapp/globalutils/annotation/NoLogging.kt (1)
3-5: 클래스 레벨 미대상으로 타입(@nologging 클래스) 매칭 불가 — CommonPointcuts와 불일치현재 @target 이 FUNCTION 만 포함되어, 클래스 레벨에 @nologging 을 부착할 수 없습니다. CommonPointcuts.noLogging()이 타입 애노테이션도 제외 대상으로 가정하는 만큼, CLASS 를 추가해야 합니다.
적용 제안:
-@Target(AnnotationTarget.FUNCTION) +@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) @Retention(AnnotationRetention.RUNTIME) annotation class NoLogginggateway/src/main/kotlin/org/yapp/gateway/filter/MdcLoggingFilter.kt (2)
34-38: 응답 헤더에 traceId(X-Request-ID) 반영하여 상호 추적성 확보클라이언트가 서버가 최종 확정한 traceId를 응답 헤더로 받을 수 있도록 설정하는 것을 권장합니다.
try { filterChain.doFilter(request, response) } finally { + // 요청-응답 상호 추적을 위해 traceId 반환 + response.setHeader(TRACE_ID_HEADER, traceId) MDC.clear() }
50-50: requestInfo에 쿼리스트링 포함 권장정밀한 요청 식별을 위해 queryString을 포함하세요.
- MDC.put(REQUEST_INFO_KEY, "${request.method} ${request.requestURI}") + val query = request.queryString?.let { "?$it" } ?: "" + MDC.put(REQUEST_INFO_KEY, "${request.method} ${request.requestURI}$query")infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt (3)
31-37: 불필요한 Throwable 캐치/재던지기 제거 — detekt 경고 해소 및 전역 예외 핸들러 일관성 유지스택트레이스 로깅은 전역 핸들러에 위임한다는 정책이라면 여기서의 try/catch 후 동일 예외 재던지기는 불필요합니다.
- try { - val result = joinPoint.proceed() - logRequestSuccess(startTime) - return result - } catch (e: Throwable) { - throw e - } + val result = joinPoint.proceed() + logRequestSuccess(startTime) + return result
52-55: 실행 시간 계산 일관화 및 라벨 명확화(총 소요 시간으로 표시)endTime을 캡처한 시각과 Duration 계산 기준을 일치시키고, 컨트롤러 레벨은 “Logic” 보다는 “Total” 표현이 적합합니다.
- private fun logRequestSuccess(startTime: Instant) { - val executionTimeMs = getExecutionTimeMs(startTime) - log.info("[API-RES] Logic Duration: {}ms", executionTimeMs) - } + private fun logRequestSuccess(startTime: Instant) { + val endTime = Instant.now() + val executionTimeMs = getExecutionTimeMs(startTime, endTime) + log.info("[API-RES] Total Duration: {}ms", executionTimeMs) + } @@ - private fun getExecutionTimeMs(startTime: Instant): Long = - Duration.between(startTime, Instant.now()).toMillis() + private fun getExecutionTimeMs(startTime: Instant, endTime: Instant): Long = + Duration.between(startTime, endTime).toMillis()Also applies to: 60-61
22-23: 예외 발생 “존재”만 기록하는 @AfterThrowing 추가 제안(스택트레이스는 전역 핸들러 위임)성공 로그만 남기되 실패 시점 가시성을 확보하려면 @AfterThrowing 어드바이스로 존재만 기록하는 경고 로그를 추가하는 방안을 권장합니다.
예시(파일 내 적절한 위치에 추가):
import org.aspectj.lang.JoinPoint import org.aspectj.lang.annotation.AfterThrowing @AfterThrowing( pointcut = "org.yapp.infra.aop.pointcut.CommonPointcuts.controller() && !org.yapp.infra.aop.pointcut.CommonPointcuts.noLogging()", throwing = "ex" ) fun logControllerError(joinPoint: JoinPoint, ex: Throwable) { if (!properties.controller.enabled) return val signature = joinPoint.signature val className = signature.declaringType.simpleName val methodName = signature.name val request = getCurrentRequest() val httpMethod = request?.method ?: "UNKNOWN" val uri = request?.requestURI ?: "UNKNOWN" // 스택트레이스 없이 실패 “존재”만 기록 log.warn("[API-ERR] {} {} | Controller: {}.{} | Error occurred", httpMethod, uri, className, methodName) }infra/src/main/resources/log4j2-spring.xml (3)
7-7: userId 직접 노출은 PII 위험 — 패턴을 userHash로 교체MDC의 원본 userId 노출은 개인정보 이슈를 유발할 수 있습니다. 가명화 키(userHash)로 대체하는 것을 권장합니다. MdcLoggingFilter에서 userHash를 채우는 변경과 함께 적용해 주세요.
- %d{yyyy-MM-dd HH:mm:ss.SSSSSS} %-5level [%thread] [%X{traceId}] [%X{userId}] [%X{clientIp}] [%X{requestInfo}] %logger{36} - %msg%n + %d{yyyy-MM-dd HH:mm:ss.SSSSSS} %-5level [%thread] [%X{traceId}] [%X{userHash}] [%X{clientIp}] [%X{requestInfo}] %logger{36} - %msg%n @@ - %d{yyyy-MM-dd HH:mm:ss.SSSSSS} %highlight{%-5level}{FATAL=red blink, ERROR=red, WARN=yellow bold, INFO=green, DEBUG=green bold, TRACE=blue} %style{[%thread]}{cyan} %style{[%X{traceId}]}{green} %style{[%X{userId}]}{magenta} %style{[%X{clientIp}]}{blue} %style{[%X{requestInfo}]}{yellow} %style{%logger{36}}{bright,white} - %msg%n + %d{yyyy-MM-dd HH:mm:ss.SSSSSS} %highlight{%-5level}{FATAL=red blink, ERROR=red, WARN=yellow bold, INFO=green, DEBUG=green bold, TRACE=blue} %style{[%thread]}{cyan} %style{[%X{traceId}]}{green} %style{[%X{userHash}]}{magenta} %style{[%X{clientIp}]}{blue} %style{[%X{requestInfo}]}{yellow} %style{%logger{36}}{bright,white} - %msg%nAlso applies to: 11-11
15-31: 로깅 비동기화로 API 스레드 차단 최소화 — AsyncAppender 추가 제안Console/RollingFile 동기 출력은 I/O로 인해 요청 스레드를 차단할 수 있습니다. AsyncAppender 래핑을 권장합니다.
<Appenders> <!-- 콘솔 출력 - 색상 활성화 --> <Console name="Console" target="SYSTEM_OUT"> <PatternLayout pattern="${CONSOLE_LOG_PATTERN}" disableAnsi="false"/> </Console> @@ </RollingFile> + + <!-- 비동기 Appender 래핑 --> + <AsyncAppender name="AsyncConsole" bufferSize="2048" blocking="true"> + <AppenderRef ref="Console"/> + </AsyncAppender> + <AsyncAppender name="AsyncFile" bufferSize="8192" blocking="true"> + <AppenderRef ref="FileAppender"/> + </AsyncAppender> </Appenders>
35-44: AsyncAppender 사용 시 로거 참조 교체 필요AsyncAppender를 추가했다면 Logger/Root가 해당 래퍼를 참조하도록 바꿔주세요.
<Logger name="org.yapp" level="INFO" additivity="false"> - <AppenderRef ref="Console"/> - <AppenderRef ref="FileAppender"/> + <AppenderRef ref="AsyncConsole"/> + <AppenderRef ref="AsyncFile"/> </Logger> @@ <Root level="INFO"> - <AppenderRef ref="Console"/> - <AppenderRef ref="FileAppender"/> + <AppenderRef ref="AsyncConsole"/> + <AppenderRef ref="AsyncFile"/> </Root>infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt (1)
37-44: 서비스 실패 시 [SVC-FAILED] 로그를 남기고, Throwable 대신 Exception만 캐치하세요실패 사실/시간을 남기지 않아 디버깅 가시성이 떨어집니다. 또한 Throwable 캐치는 과도합니다(Detekt: TooGenericExceptionCaught, RethrowCaughtException). 실패 로그를 남기고 Exception만 캐치하도록 변경하는 것을 권장합니다.
try { val result = joinPoint.proceed() logServiceSuccess(signature, startTime, result) return result - } catch (e: Throwable) { - throw e + } catch (e: Exception) { + logServiceFailure(signature, startTime, e) + throw e }아래 메서드를 추가해 주세요(스택트레이스는 GlobalExceptionHandler에 위임되므로 타입/소요 시간만 기록).
private fun logServiceFailure(signature: MethodSignature, startTime: Instant, throwable: Throwable) { val className = signature.declaringType.simpleName val methodName = signature.name val duration = Duration.between(startTime, Instant.now()).toMillis() log.error( "[SVC-FAILED] {}.{} | Exception: {} | Duration: {}ms", className, methodName, throwable.javaClass.simpleName, duration ) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apis/build.gradle.kts(1 hunks)build.gradle.kts(2 hunks)buildSrc/src/main/kotlin/Plugins.kt(1 hunks)gateway/src/main/kotlin/org/yapp/gateway/filter/MdcLoggingFilter.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/annotation/NoLogging.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt(2 hunks)infra/build.gradle.kts(1 hunks)infra/src/main/kotlin/org/yapp/infra/aop/aspect/ControllerLoggingAspect.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/async/MdcTaskDecorator.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt(1 hunks)infra/src/main/resources/log4j2-spring.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-16T06:56:28.923Z
Learnt from: hoonyworld
PR: YAPP-Github/Reed-Server#96
File: gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt:0-0
Timestamp: 2025-08-16T06:56:28.923Z
Learning: In Spring Security filter chain, MdcLoggingFilter should be placed after BearerTokenAuthenticationFilter because it depends on SecurityContextHolder to extract userId from the authenticated principal. Placing it before authentication filters would result in null or anonymous authentication context.
Applied to files:
gateway/src/main/kotlin/org/yapp/gateway/filter/MdcLoggingFilter.kt
🪛 detekt (1.23.8)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt
[warning] 41-41: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 109-109: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 42-42: Do not rethrow a caught exception of the same type.
(detekt.exceptions.RethrowCaughtException)
[warning] 109-109: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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-validation
🔇 Additional comments (14)
global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt (1)
12-12: 올바른 예외 타입 import 추가
MethodArgumentTypeMismatchException처리용 import가 적절히 추가되었습니다. 아래 핸들러에서만 사용되며 불필요한 의존성도 없습니다.buildSrc/src/main/kotlin/Plugins.kt (1)
14-14: KAPT 플러그인 상수 추가 적절
Plugins.Kotlin.KAPT상수 추가로apply(plugin = Plugins.Kotlin.KAPT)를 안전하게 참조할 수 있게 되어 일관성이 좋아졌습니다. 내부Short.KAPT와의 역할 구분도 명확합니다.apis/build.gradle.kts (1)
25-25: annotationProcessor → kapt 전환 완료
apis 모듈에 Java 소스가 존재하지 않아kapt(Dependencies.Spring.CONFIGURATION_PROCESSOR)만으로 메타데이터가 정상 생성됩니다. 추가적인annotationProcessor설정은 필요 없습니다.infra/build.gradle.kts (3)
9-10: kotlin-reflect 추가 적절리플렉션이 필요한 프록시/AOP 환경에서 요구되는 의존성으로 적절합니다.
20-20: QueryDSL APT를 kapt로 선언한 점 적합Kotlin 기준으로 APT는 kapt로 선언하는 게 맞습니다.
16-18: Flyway/QueryDSL JPA 의존성 유지 여부 확인 필요
현재 infra/build.gradle.kts 16–18라인에 Flyway.MYSQL과 QueryDsl.JPA 의존성이 여전히 선언되어 있습니다. AI 요약에는 ‘제거’로 표기되어 있으나 실제 코드와 불일치합니다.
- infra/build.gradle.kts:16 implementation(Dependencies.Flyway.MYSQL)
- infra/build.gradle.kts:18 implementation(Dependencies.QueryDsl.JPA)
제거가 의도된 것인지, 아니면 요약을 업데이트해야 하는지 확인 부탁드립니다.
build.gradle.kts (2)
59-61: starter-logging 전역 exclude 분리 적용 👍이전 피드백대로 dependencies 블록 밖에서 전역 exclude로 선언되어 충돌 가능성을 줄였습니다.
83-83: javaParameters = true 설정 적절리플렉션 기반 프레임워크/DI에서 파라미터 이름 유지에 유용합니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt (1)
12-12: 검증 완료: 구 파라미터명(named-arg) 호출 없음 — 승인
구 파라미터명(bookIsbn13List,bookIsbns)을 사용하는 named argument 호출이나 식별자가 코드베이스에서 발견되지 않았습니다. 바이너리 호환성에 영향이 없으므로 변경을 승인합니다.infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
45-49: 파라미터명(bookIsbn13s) 변경 — 구현부와 JPA 레이어 일치의미 일관성이 좋아졌습니다. 다만 코틀린 named argument 호출부가 있다면 이번 변경에 맞춰 파라미터명을 함께 갱신해야 합니다.
infra/src/main/kotlin/org/yapp/infra/config/internal/async/MdcTaskDecorator.kt (1)
11-19: MDC 전파·복원 로직 적절 — 잔존 MDC 누수 방지 처리 완료부모 MDC가 null일 때 clear, 실행 후 이전 컨텍스트 복원까지 정확히 처리되어 스레드풀 재사용 시 누수를 방지합니다. LGTM.
gateway/src/main/kotlin/org/yapp/gateway/filter/MdcLoggingFilter.kt (1)
13-14: 중복 등록 없음: MdcLoggingFilter는 @component로만 등록되어 있습니다
- SecurityConfig 내
addFilterAfter/addFilterBefore/addFilterAt호출이 없습니다.FilterRegistrationBean을 통한 전역 등록 설정도 없습니다.따라서 서블릿 체인과 보안 체인에서 두 번 실행될 우려는 없습니다.
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt (2)
54-67: 성공 로그의 수행 시간 측정과 결과 마스킹 경로는 합리적입니다Duration.between으로 ms 단위 측정, 결과 값도 formatValue 경로로 통일되어 있어 일관성 좋습니다.
69-78: Kotlin 파라미터명 노출 보장 확인 필요
런타임에 Kotlin 메서드의 실제 파라미터명을 얻으려면 Gradle 설정에서kotlinOptions.javaParameters = true또는 JavaCompile 단계에-parameters옵션이 반드시 포함되어야 합니다. 레포지토리의 모든*.gradle.kts(또는build.gradle) 파일에서 다음 항목을 확인해 주세요:
kotlinOptions블록에javaParameters = true설정 여부freeCompilerArgs(또는options.compilerArgs)에"-parameters"추가 여부예시:
tasks.withType<KotlinCompile> { kotlinOptions { javaParameters = true freeCompilerArgs += listOf("-Xjsr305=strict", "-parameters") } }해당 설정이 누락되어 있으면 빌드 스크립트에 추가해 주세요.
| implementation(Dependencies.BouncyCastle.BC_PKIX) | ||
|
|
||
| annotationProcessor(Dependencies.Spring.CONFIGURATION_PROCESSOR) | ||
| kapt(Dependencies.Spring.CONFIGURATION_PROCESSOR) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
AOP 스타터 의존성 존재 여부 확인 필요
본 PR의 핵심인 AOP(애스펙트) 로깅이 실제 런타임에서 동작하려면 실행 모듈(예: apis)에 spring-boot-starter-aop가 필요합니다. 현재 제공된 빌드 파일들에서는 확인되지 않습니다.
아래 스크립트로 리포지토리 전체에서 AOP 스타터 추가 여부를 점검해 주세요.
apis 모듈에 추가가 필요하다면 다음 예시처럼 선언해 주세요(프로젝트 상수로 관리 중이면 해당 상수를 사용하세요).
dependencies {
implementation("org.springframework.boot:spring-boot-starter-aop")
}🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[Search] spring-boot-starter-aop 사용 여부"
rg -n -S 'spring-boot-starter-aop|STARTER_AOP|BOOT_STARTER_AOP' -g 'build*.gradle*' || echo "AOP starter 미검출"Length of output: 233
apis 모듈에 AOP 스타터 의존성 추가 필요
스크립트 결과, 전체 build*.gradle* 파일에서 spring-boot-starter-aop 의존성이 미검출되었습니다. AOP(애스펙트) 기반 로깅이 런타임에서 동작하려면 apis/build.gradle.kts의 dependencies 블록에 아래 의존성을 추가해주세요.
- 파일:
apis/build.gradle.kts - 위치:
dependencies { … }블록 내부
dependencies {
implementation("org.springframework.boot:spring-boot-starter-aop")
// …기존 의존성
}🤖 Prompt for AI Agents
In apis/build.gradle.kts around line 25, the project is missing the Spring AOP
starter dependency required for AOP-based runtime logging; open the dependencies
{ … } block in that file and add the Spring Boot AOP starter as an
implementation dependency
(implementation("org.springframework.boot:spring-boot-starter-aop")) alongside
the existing dependencies so AOP aspects are available at runtime.
| apply(plugin = Plugins.Kotlin.JPA) | ||
| apply(plugin = Plugins.Kotlin.JVM) | ||
| apply(plugin = Plugins.JACOCO) | ||
| apply(plugin = Plugins.Kotlin.KAPT) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
KAPT 플러그인 서브모듈 적용 OK — 루트 plugins 블록은 apply false 권장
현재 루트 plugins { kotlin("kapt") version ... }로 루트에도 플러그인이 적용되고, 서브모듈에 다시 apply하고 있습니다. 루트는 apply false로 선언하고 서브모듈에서만 apply(plugin = Plugins.Kotlin.KAPT) 하는 패턴이 더 깔끔합니다.
아래와 같이 변경을 고려해 주세요(루트 plugins 블록):
plugins {
id(Plugins.SPRING_BOOT) version Versions.SPRING_BOOT
id(Plugins.SPRING_DEPENDENCY_MANAGEMENT) version Versions.SPRING_DEPENDENCY_MANAGEMENT
kotlin(Plugins.Kotlin.Short.KAPT) version Versions.KOTLIN apply false
kotlin(Plugins.Kotlin.Short.JVM) version Versions.KOTLIN apply false
kotlin(Plugins.Kotlin.Short.SPRING) version Versions.KOTLIN apply false
kotlin(Plugins.Kotlin.Short.JPA) version Versions.KOTLIN apply false
id(Plugins.DETEKT) version Versions.DETEKT
id(Plugins.JACOCO)
id(Plugins.SONAR_QUBE) version Versions.SONAR_QUBE
}🤖 Prompt for AI Agents
build.gradle.kts around line 51: 현재 루트 plugins 블록에서 kotlin("kapt")가 전역으로 적용되어 있고
서브모듈에서도 apply(plugin = Plugins.Kotlin.KAPT)을 사용하고 있으니, 루트 plugins 블록에서 해당 KAPT(및
다른 kotlin 플러그인들)를 version 선언 후 apply false로 변경하고 서브모듈에서만 apply(plugin =
Plugins.Kotlin.KAPT) 하도록 변경하세요; 구체적으로 루트의 plugins { ... }에 kotlin
kapt/jvm/spring/jpa 플러그인을 version과 함께 apply false로 선언하고 루트에서의 직접 적용을 제거한 뒤 각
서브모듈 build.gradle.kts에서는 기존대로 apply(plugin = Plugins.Kotlin.KAPT) 를 유지해 빌드스크립트의
플러그인 적용 범위를 서브모듈로 한정하세요.
| val userId = resolveUserId() | ||
| MDC.put(USER_ID_KEY, userId ?: DEFAULT_GUEST_USER) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
PII 최소화: 원본 userId 대신 가명(userHash) 기록 권장
로그에 원본 userId를 직접 노출하면 PII 이슈가 발생할 수 있습니다. 원본을 유지해야 한다면 별도 키(userHash)에 해시를 함께 기록하고, log4j2 패턴은 userHash로 바꾸는 것이 안전합니다. log4j2-spring.xml에 대한 코멘트도 함께 확인 바랍니다.
- val userId = resolveUserId()
- MDC.put(USER_ID_KEY, userId ?: DEFAULT_GUEST_USER)
+ val userId = resolveUserId() ?: DEFAULT_GUEST_USER
+ MDC.put(USER_ID_KEY, userId) // 필요 시 유지
+ MDC.put("userHash", hashSha256(userId)) // 로그 패턴은 userHash 사용 권장추가 함수(파일 내 임의 위치, 예: 클래스 하단)에 정의해 주세요:
// import 필요: java.security.MessageDigest
private fun hashSha256(input: String): String {
val md = java.security.MessageDigest.getInstance("SHA-256")
val bytes = md.digest(input.toByteArray(Charsets.UTF_8))
val hex = StringBuilder(bytes.size * 2)
for (b in bytes) {
val i = b.toInt() and 0xff
if (i < 0x10) hex.append('0')
hex.append(i.toString(16))
}
return hex.toString()
}| return when (val principal = authentication.principal) { | ||
| is Jwt -> principal.subject | ||
| else -> principal?.toString() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
principal.toString()는 과다/민감 정보 노출 위험 — authentication.name 사용 권장
toString()은 객체 상세를 포함할 수 있어 로그 과다/PII 위험이 큽니다. 일반 케이스는 authentication.name 이 안전하고 일관적입니다.
- return when (val principal = authentication.principal) {
- is Jwt -> principal.subject
- else -> principal?.toString()
- }
+ return when (val principal = authentication.principal) {
+ is Jwt -> principal.subject
+ else -> authentication.name
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return when (val principal = authentication.principal) { | |
| is Jwt -> principal.subject | |
| else -> principal?.toString() | |
| } | |
| return when (val principal = authentication.principal) { | |
| is Jwt -> principal.subject | |
| else -> authentication.name | |
| } |
🤖 Prompt for AI Agents
gateway/src/main/kotlin/org/yapp/gateway/filter/MdcLoggingFilter.kt around lines
73 to 76: the code returns principal?.toString() which can expose excessive or
sensitive details in logs; change the fallback to use authentication.name (a
safer, consistent identifier) instead of calling toString() on the principal,
i.e. return principal.subject for Jwt and otherwise return authentication.name
so logs do not leak PII.
| /** | ||
| * 메서드 파라미터 타입 변환 실패 처리 | ||
| * | ||
| * 주로 @RequestParam, @PathVariable 등에서 클라이언트가 잘못된 타입의 값을 전달했을 때 발생합니다. | ||
| * 예: 문자열을 int 타입으로 변환 시도 | ||
| */ | ||
| @ExceptionHandler(MethodArgumentTypeMismatchException::class) | ||
| protected fun handleMethodArgumentTypeMismatch(ex: MethodArgumentTypeMismatchException): ResponseEntity<ErrorResponse> { | ||
| val commonErrorCode = CommonErrorCode.INVALID_REQUEST | ||
|
|
||
| log.warn { "Method argument type mismatch: ${ex.name}, value: ${ex.value}, requiredType: ${ex.requiredType}" } | ||
|
|
||
| val error = ErrorResponse.builder() | ||
| .status(commonErrorCode.getHttpStatus().value()) | ||
| .message("Invalid value '${ex.value}' for parameter '${ex.name}'. Expected type: ${ex.requiredType?.simpleName}") | ||
| .code(commonErrorCode.getCode()) | ||
| .build() | ||
|
|
||
| return ResponseEntity(error, commonErrorCode.getHttpStatus()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
민감정보/대용량 값의 로그·응답 반영 가능성 — 값 마스킹/절단 및 널-세이프 처리 권장
- 현재
ex.value가 로그와 클라이언트 응답 메시지에 그대로 반영됩니다. PathVariable/RequestParam에 토큰, 이메일 등 PII가 포함될 경우 노출 위험이 있습니다. PR에서 제시한 “민감 정보 마스킹” 원칙과도 상충합니다. ex.requiredType는 null 가능성이 있어 메시지에null이 출력될 수 있습니다. 기본값을 두는 편이 안전합니다.- 로그에서는
requiredType전체 객체가 출력되어"class java.lang.Integer"와 같은 노이즈가 생길 수 있으므로simpleName사용을 권장합니다.
아래와 같이 값 마스킹/절단 및 널-세이프 처리를 적용해 주세요.
@@
- log.warn { "Method argument type mismatch: ${ex.name}, value: ${ex.value}, requiredType: ${ex.requiredType}" }
+ // 민감 값 마스킹/절단 및 널-세이프 타입명 구성
+ val valueStr = ex.value?.toString()?.let { if (it.length > 100) it.take(100) + "..." else it } ?: "null"
+ val requiredTypeName = ex.requiredType?.simpleName ?: "Unknown"
+ log.warn { "Method argument type mismatch: name=${ex.name}, value=$valueStr, requiredType=$requiredTypeName" }
@@
- .message("Invalid value '${ex.value}' for parameter '${ex.name}'. Expected type: ${ex.requiredType?.simpleName}")
+ .message("Invalid value '$valueStr' for parameter '${ex.name}'. Expected type: $requiredTypeName")위 수정으로
- 로그/응답에 과도한 원문 값 노출을 줄이고,
- 타입명이 null이거나 장황하게 출력되는 문제를 방지합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 메서드 파라미터 타입 변환 실패 처리 | |
| * | |
| * 주로 @RequestParam, @PathVariable 등에서 클라이언트가 잘못된 타입의 값을 전달했을 때 발생합니다. | |
| * 예: 문자열을 int 타입으로 변환 시도 | |
| */ | |
| @ExceptionHandler(MethodArgumentTypeMismatchException::class) | |
| protected fun handleMethodArgumentTypeMismatch(ex: MethodArgumentTypeMismatchException): ResponseEntity<ErrorResponse> { | |
| val commonErrorCode = CommonErrorCode.INVALID_REQUEST | |
| log.warn { "Method argument type mismatch: ${ex.name}, value: ${ex.value}, requiredType: ${ex.requiredType}" } | |
| val error = ErrorResponse.builder() | |
| .status(commonErrorCode.getHttpStatus().value()) | |
| .message("Invalid value '${ex.value}' for parameter '${ex.name}'. Expected type: ${ex.requiredType?.simpleName}") | |
| .code(commonErrorCode.getCode()) | |
| .build() | |
| return ResponseEntity(error, commonErrorCode.getHttpStatus()) | |
| } | |
| /** | |
| * 메서드 파라미터 타입 변환 실패 처리 | |
| * | |
| * 주로 @RequestParam, @PathVariable 등에서 클라이언트가 잘못된 타입의 값을 전달했을 때 발생합니다. | |
| * 예: 문자열을 int 타입으로 변환 시도 | |
| */ | |
| @ExceptionHandler(MethodArgumentTypeMismatchException::class) | |
| protected fun handleMethodArgumentTypeMismatch(ex: MethodArgumentTypeMismatchException): ResponseEntity<ErrorResponse> { | |
| val commonErrorCode = CommonErrorCode.INVALID_REQUEST | |
| // 민감 값 마스킹/절단 및 널-세이프 타입명 구성 | |
| val valueStr = ex.value | |
| ?.toString() | |
| ?.let { if (it.length > 100) it.take(100) + "..." else it } | |
| ?: "null" | |
| val requiredTypeName = ex.requiredType?.simpleName ?: "Unknown" | |
| log.warn { "Method argument type mismatch: name=${ex.name}, value=$valueStr, requiredType=$requiredTypeName" } | |
| val error = ErrorResponse.builder() | |
| .status(commonErrorCode.getHttpStatus().value()) | |
| .message("Invalid value '$valueStr' for parameter '${ex.name}'. Expected type: $requiredTypeName") | |
| .code(commonErrorCode.getCode()) | |
| .build() | |
| return ResponseEntity(error, commonErrorCode.getHttpStatus()) | |
| } |
🤖 Prompt for AI Agents
In
global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt
around lines 163 to 182, ex.value is currently logged and returned raw and
ex.requiredType can be null or verbose; change to create a safe, maskedValue by
null-checking ex.value, converting to string, masking common sensitive patterns
(emails/tokens) and truncating long values (e.g., to 100 chars with "…"), then
use that maskedValue in both the log and the ErrorResponse message; for the type
use ex.requiredType?.simpleName ?: "Unknown" to avoid nulls and noisy class
names; update the log to include maskedValue and simpleName-only type.
| private fun formatValue(obj: Any?): String { | ||
| return when (obj) { | ||
| null -> "null" | ||
| is Unit -> "void" | ||
| is String -> "\"$obj\"" | ||
| is Number, is Boolean -> obj.toString() | ||
| is UUID -> obj.toString() | ||
| is Collection<*> -> "[${obj.size} items]" | ||
| else -> maskMapLikeObject(obj) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
배열/프리미티브 배열/바이너리 데이터는 내용 대신 크기만 기록하세요
현재 Collection만 크기 정보를 기록합니다. 배열/바이너리도 동일 처리하여 로그 폭주/PII 유출을 줄이세요.
private fun formatValue(obj: Any?): String {
return when (obj) {
null -> "null"
is Unit -> "void"
is String -> "\"$obj\""
is Number, is Boolean -> obj.toString()
is UUID -> obj.toString()
- is Collection<*> -> "[${obj.size} items]"
+ is Collection<*> -> "[${obj.size} items]"
+ is ByteArray -> "[${obj.size} bytes]"
+ is IntArray -> "[${obj.size} items]"
+ is LongArray -> "[${obj.size} items]"
+ is ShortArray -> "[${obj.size} items]"
+ is FloatArray -> "[${obj.size} items]"
+ is DoubleArray -> "[${obj.size} items]"
+ is CharArray -> "[${obj.size} chars]"
+ is BooleanArray -> "[${obj.size} items]"
+ is Array<*> -> "[${obj.size} items]"
else -> maskMapLikeObject(obj)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun formatValue(obj: Any?): String { | |
| return when (obj) { | |
| null -> "null" | |
| is Unit -> "void" | |
| is String -> "\"$obj\"" | |
| is Number, is Boolean -> obj.toString() | |
| is UUID -> obj.toString() | |
| is Collection<*> -> "[${obj.size} items]" | |
| else -> maskMapLikeObject(obj) | |
| } | |
| } | |
| private fun formatValue(obj: Any?): String { | |
| return when (obj) { | |
| null -> "null" | |
| is Unit -> "void" | |
| is String -> "\"$obj\"" | |
| is Number, is Boolean -> obj.toString() | |
| is UUID -> obj.toString() | |
| is Collection<*> -> "[${obj.size} items]" | |
| is ByteArray -> "[${obj.size} bytes]" | |
| is IntArray -> "[${obj.size} items]" | |
| is LongArray -> "[${obj.size} items]" | |
| is ShortArray -> "[${obj.size} items]" | |
| is FloatArray -> "[${obj.size} items]" | |
| is DoubleArray -> "[${obj.size} items]" | |
| is CharArray -> "[${obj.size} chars]" | |
| is BooleanArray -> "[${obj.size} items]" | |
| is Array<*> -> "[${obj.size} items]" | |
| else -> maskMapLikeObject(obj) | |
| } | |
| } |
🤖 Prompt for AI Agents
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt around
lines 86 to 96: the formatter currently treats only Collection as size-only, so
arrays and primitive arrays (including ByteArray/binary) still get expanded and
may leak PII or flood logs; update the when branch to detect Array<*>, ByteArray
and all Kotlin primitive arrays (IntArray, LongArray, ShortArray, CharArray,
BooleanArray, FloatArray, DoubleArray) and return a size-only placeholder like
"[N items]" (for ByteArray use length), leaving other branches unchanged; ensure
Array<*> uses obj.size and primitive arrays use their size properties so
binary/primitive arrays are logged by length not contents.
| override fun findAllByUserIdAndBookIsbn13In( | ||
| userId: UUID, | ||
| bookIsbns: List<String> | ||
| bookIsbn13s: List<String> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
List → Collection으로 타입 완화(구현부) 제안
레포지토리 인터페이스와 함께 Collection<String>으로 완화하면 API 일관성과 사용 편의성이 향상됩니다.
적용 예:
- bookIsbn13s: List<String>
+ bookIsbn13s: Collection<String>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bookIsbn13s: List<String> | |
| bookIsbn13s: Collection<String> |
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt
around line 47, the method parameter is declared as List<String>; relax it to
Collection<String> to match the repository interface and improve API
consistency. Change the parameter type to Collection<String>, update any
internal usages that require List (e.g., call .toList() where ordering/index
access is needed), and adjust any overridden method signature to exactly match
the interface; ensure compilation by updating calls and imports if necessary.
| return jpaUserBookRepository.findAllByUserIdAndBookIsbn13In(userId, bookIsbn13s) | ||
| .map { it.toDomain() } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
빈 컬렉션 입력 시 DB 호출 방지 및 잠재적 오류 예방
IN () 케이스는 JPA/Hibernate 설정에 따라 예외가 발생하거나(빈 컬렉션 미허용) 쓸모없는 쿼리를 유발할 수 있습니다. 입력이 비어있으면 조기 반환하도록 권장합니다.
아래처럼 조기 반환을 추가해 주세요:
): List<UserBook> {
- return jpaUserBookRepository.findAllByUserIdAndBookIsbn13In(userId, bookIsbn13s)
+ if (bookIsbn13s.isEmpty()) return emptyList()
+ return jpaUserBookRepository.findAllByUserIdAndBookIsbn13In(userId, bookIsbn13s)
.map { it.toDomain() }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return jpaUserBookRepository.findAllByUserIdAndBookIsbn13In(userId, bookIsbn13s) | |
| .map { it.toDomain() } | |
| } | |
| ): List<UserBook> { | |
| if (bookIsbn13s.isEmpty()) return emptyList() | |
| return jpaUserBookRepository.findAllByUserIdAndBookIsbn13In(userId, bookIsbn13s) | |
| .map { it.toDomain() } | |
| } |
🤖 Prompt for AI Agents
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt
around lines 49-51: Add an early-return guard for an empty bookIsbn13s
collection to avoid calling JPA with IN () — if the input collection is
null/empty, return an empty list immediately; otherwise proceed to call
jpaUserBookRepository.findAllByUserIdAndBookIsbn13In(userId, bookIsbn13s).map {
it.toDomain() }.
| fun existsByIdAndUserId(id: UUID, userId: UUID): Boolean | ||
| fun findAllByUserId(userId: UUID): List<UserBookEntity> | ||
| fun findAllByUserIdAndBookIsbn13In(userId: UUID, bookIsbn13List: List<String>): List<UserBookEntity> | ||
| fun findAllByUserIdAndBookIsbn13In(userId: UUID, bookIsbn13s: List<String>): List<UserBookEntity> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
List 대신 Collection으로 파라미터 타입 완화 제안
Spring Data JPA의 In 조건은 Collection이면 충분합니다. API 유연성과 호출 시 불필요한 복사를 줄일 수 있습니다.
아래와 같이 변경을 제안합니다:
- fun findAllByUserIdAndBookIsbn13In(userId: UUID, bookIsbn13s: List<String>): List<UserBookEntity>
+ fun findAllByUserIdAndBookIsbn13In(userId: UUID, bookIsbn13s: Collection<String>): List<UserBookEntity>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun findAllByUserIdAndBookIsbn13In(userId: UUID, bookIsbn13s: List<String>): List<UserBookEntity> | |
| fun findAllByUserIdAndBookIsbn13In( | |
| userId: UUID, | |
| bookIsbn13s: Collection<String> | |
| ): List<UserBookEntity> |
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt
around line 12, change the method signature parameter type from List<String> to
Collection<String> to relax the API for Spring Data JPA's "In" condition; update
the declaration to use Collection<String> and adjust any callers if they rely on
List-specific methods (prefer passing their existing collections as-is) so no
unnecessary copies are made.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (2)
42-45: 에러 로그에 요청 객체 전체를 포함하지 마세요 (PII/보안 이슈)
request전체를 메시지에 포함하면 민감 필드가 노출될 수 있습니다. AOP에서 파라미터 로깅/마스킹이 이미 수행되므로, 여기서는 원인과 컨텍스트 최소 정보만 로그로 남기는 것이 안전합니다.다음처럼 메시지에서
request제거를 제안합니다:- log.error(exception) { "Failed to call Aladin search API for request: '$request'" } + log.error(exception) { "Failed to call Aladin search API" }가능하다면 BookException에 원인(cause)을 함께 전달하는 생성자를 사용해 스택트레이스 보존도 고려해 주세요.
90-93: KotlinLogging 예외 로깅 사용법 수정 필요 — 스택트레이스가 출력되지 않을 수 있음현재
log.error("message", exception)형태는 SLF4J의 vararg 치환으로 처리되어 예외가 Throwable로 인식되지 않을 수 있습니다. KotlinLogging에서는log.error(exception) { "message" }형태를 사용하세요.- log.error("Failed to call Aladin lookup API for request: '$request'", exception) + // 메시지에 request 전체 포함은 지양 + log.error(exception) { "Failed to call Aladin lookup API" }또한 위와 동일하게
request전체를 메시지에 포함하지 않도록 조정해 주세요.
♻️ Duplicate comments (4)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt (4)
33-51: 예외 시 [SVC-FAILED] 로그가 누락되어 있고 detekt 경고가 발생합니다 — runCatching 기반으로 리팩터링하세요현재
catch (e: Throwable) { throw e }는 detekt의 RethrowCaughtException/TooGenericExceptionCaught 지적 대상이며, 실패 로그도 남지 않습니다. PR 목표(예외 사실 감지, 상세 로깅은 전역 핸들러 위임)에 맞춰 실패 사실과 수행 시간을 로그하세요.@Around("org.yapp.infra.aop.pointcut.CommonPointcuts.serviceLayer() && !org.yapp.infra.aop.pointcut.CommonPointcuts.noLogging()") fun logService(joinPoint: ProceedingJoinPoint): Any? { if (!properties.service.enabled) { return joinPoint.proceed() } val signature = joinPoint.signature as MethodSignature val startTime = Instant.now() logServiceStart(signature, joinPoint.args) - try { - val result = joinPoint.proceed() - logServiceSuccess(signature, startTime, result) - return result - } catch (e: Throwable) { - throw e - } + val outcome = runCatching { joinPoint.proceed() } + outcome.onSuccess { result -> + logServiceSuccess(signature, startTime, result) + }.onFailure { throwable -> + logServiceFailure(signature, startTime, throwable) + } + return outcome.getOrThrow() }아래 헬퍼를 추가하세요(파일 내 적절한 위치):
private fun logServiceFailure(signature: MethodSignature, startTime: Instant, throwable: Throwable) { val className = signature.declaringType.simpleName val methodName = signature.name val duration = Duration.between(startTime, Instant.now()).toMillis() // 스택트레이스 출력은 전역 예외 처리기로 위임: 타입/소요시간만 기록 log.warn( "[SVC-FAILED] {}.{} | Exception: {} | Duration: {}ms", className, methodName, throwable.javaClass.simpleName, duration ) }이 방식은 detekt의 rethrow 경고도 피하면서 실패 로그를 일관되게 남깁니다.
53-59: [SVC-START] 파라미터 INFO 로깅은 토글 가능해야 합니다운영 환경에서 파라미터 전체를 INFO로 항상 남기는 것은 과도/민감할 수 있습니다.
properties.service.logParams(기본 false)로 토글하고, 비활성화 시 INFO에는 메서드 시작만, 파라미터는 DEBUG로만 남기도록 분기하세요.private fun logServiceStart(signature: MethodSignature, args: Array<Any?>) { val className = signature.declaringType.simpleName val methodName = signature.name val params = getArgumentsAsString(signature, args) - log.info("[SVC-START] {}.{} | Params: {}", className, methodName, truncateIfNeeded(params)) + if (properties.service.logParams) { + log.info("[SVC-START] {}.{} | Params: {}", className, methodName, truncateIfNeeded(params)) + } else { + log.info("[SVC-START] {}.{}", className, methodName) + if (log.isDebugEnabled) { + log.debug("[SVC-START] {}.{} | Params: {}", className, methodName, truncateIfNeeded(params)) + } + } }추가로 필요한 설정 바인딩 예시(다른 파일 변경):
// LoggingAopProperties.kt (예시) data class LoggingAopProperties( val service: Service = Service() ) { data class Service( val enabled: Boolean = true, val logParams: Boolean = false, val maxLogLength: Int = 2048, val sensitiveFields: List<String> = listOf("password", "accessToken") ) }
93-104: 배열/바이너리 데이터는 내용 대신 크기만 기록하세요현재 Collection만 크기 정보를 노출합니다. 배열/프리미티브 배열/바이너리는 크기만 출력하도록 보강해 로그 폭주와 PII 노출을 줄이세요.
private fun formatValue(obj: Any?): String { return when (obj) { null -> "null" is Unit -> "void" is String -> "\"$obj\"" is Number, is Boolean -> obj.toString() is UUID -> obj.toString() is Enum<*> -> obj.name - is Collection<*> -> "[${obj.size} items]" + is Collection<*> -> "[${obj.size} items]" + is ByteArray -> "[${obj.size} bytes]" + is IntArray -> "[${obj.size} items]" + is LongArray -> "[${obj.size} items]" + is ShortArray -> "[${obj.size} items]" + is FloatArray -> "[${obj.size} items]" + is DoubleArray -> "[${obj.size} items]" + is CharArray -> "[${obj.size} chars]" + is BooleanArray -> "[${obj.size} items]" + is Array<*> -> "[${obj.size} items]" else -> maskMapLikeObject(obj) } }
106-120: 중첩 필드 마스킹 누락/예외 처리 보강 필요 — 재귀 마스킹 + 안전한 예외 처리현재는 최상위 Map의 키만 검사해 중첩 구조의 민감 필드가 노출될 수 있고,
Exception만 포괄적으로 잡아 detekt 경고(TooGenericExceptionCaught/SwallowedException)도 발생합니다. 재귀 마스킹 및 디버그 로그를 남기는 방식으로 보강하세요.- private fun maskMapLikeObject(obj: Any): String { - return try { - val map: Map<*, *> = objectMapper.convertValue(obj, Map::class.java) - val maskedMap = map.mapValues { (key, value) -> - val keyStr = key.toString() - if (shouldMaskParameter(keyStr)) { - return@mapValues MASKING_TEXT - } - value - } - objectMapper.writeValueAsString(maskedMap) - } catch (e: Exception) { - "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" - } - } + @Suppress("TooGenericExceptionCaught", "SwallowedException") + private fun maskMapLikeObject(obj: Any): String { + return try { + val map: Map<*, *> = objectMapper.convertValue(obj, Map::class.java) + val masked = maskRecursively(map) + objectMapper.writeValueAsString(masked) + } catch (e: com.fasterxml.jackson.core.JsonProcessingException) { + if (log.isDebugEnabled) log.debug("JSON processing failed for {}: {}", obj.javaClass.name, e.message) + "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" + } catch (e: IllegalArgumentException) { + if (log.isDebugEnabled) log.debug("Conversion failed for {}: {}", obj.javaClass.name, e.message) + "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" + } catch (e: StackOverflowError) { + if (log.isDebugEnabled) log.debug("Stack overflow while converting {}; fallback to descriptor", obj.javaClass.name) + "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" + } catch (e: Exception) { + if (log.isDebugEnabled) log.debug("Unexpected error for {}: {}", obj.javaClass.name, e.message) + "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" + } + }헬퍼 추가(라인 범위 외 추가 코드):
private fun maskRecursively(value: Any?): Any? { return when (value) { null -> null is Map<*, *> -> value.entries.associate { (k, v) -> val keyStr = k?.toString() ?: "null" val maskedVal = if (shouldMaskParameter(keyStr)) MASKING_TEXT else maskRecursively(v) keyStr to maskedVal } is Collection<*> -> "[${value.size} items]" is Array<*> -> "[${value.size} items]" is ByteArray -> "[${value.size} bytes]" is String -> "\"$value\"" is Number, is Boolean, is UUID -> value.toString() else -> { try { val asMap: Map<*, *> = objectMapper.convertValue(value, Map::class.java) maskRecursively(asMap) } catch (_: Exception) { "${value.javaClass.simpleName}@${Integer.toHexString(value.hashCode())}" } } } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt(3 hunks)infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt
[warning] 48-48: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 117-117: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 49-49: Do not rethrow a caught exception of the same type.
(detekt.exceptions.RethrowCaughtException)
[warning] 117-117: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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-validation
🔇 Additional comments (3)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (3)
28-29: AOP 도입에 따른 중복/과도 로깅 제거 적절서비스 단에서의 안내성 로그를 주석 처리해 AOP 로그와의 중복을 줄인 점 좋습니다. 운영 시 노이즈 감소에 도움이 됩니다.
47-49: 대응 응답 전체 내용 로깅 제거 적절전체 응답을 그대로 남기는 로그는 성능/PII 관점에서 리스크가 큽니다. 주석 처리 방향에 동의합니다.
78-81: 필터링 이후 전체 응답 로깅 제거 적절후처리된 응답도 크기가 클 수 있어 INFO 로깅은 과도합니다. 주석 처리 유지에 동의합니다.
| private fun shouldMaskParameter(paramName: String): Boolean { | ||
| return isProdEnvironment && properties.service.sensitiveFields.any { sensitiveField -> | ||
| paramName.lowercase().contains(sensitiveField.lowercase()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
마스킹 여부를 프로필 하드코딩 대신 프로퍼티로 제어하는 방안 제안
prod 프로필 하드코딩은 환경 구성 변화에 취약합니다. properties.service.maskingEnabled(기본 true) 같은 플래그로 제어하거나, environment.acceptsProfiles(Profiles.of("prod")) 사용을 고려해 주세요.
가능한 변경 예:
- private fun shouldMaskParameter(paramName: String): Boolean {
- return isProdEnvironment && properties.service.sensitiveFields.any { sensitiveField ->
+ private fun shouldMaskParameter(paramName: String): Boolean {
+ return properties.service.maskingEnabled && properties.service.sensitiveFields.any { sensitiveField ->
paramName.lowercase().contains(sensitiveField.lowercase())
}
}추가로 Properties에 maskingEnabled: Boolean = true를 도입하세요.
Committable suggestion skipped: line range outside the PR's diff.
| private fun truncateIfNeeded(text: String): String { | ||
| return if (text.length > properties.service.maxLogLength) { | ||
| "${text.substring(0, properties.service.maxLogLength)}...[truncated]" | ||
| } else { | ||
| text | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
문자열 절단 구현 간소화 제안
Kotlin의 take(n) 사용이 간결합니다. 또한 maxLogLength <= 0일 때는 절단하지 않도록 방어로직을 권장합니다.
- private fun truncateIfNeeded(text: String): String {
- return if (text.length > properties.service.maxLogLength) {
- "${text.substring(0, properties.service.maxLogLength)}...[truncated]"
- } else {
- text
- }
- }
+ private fun truncateIfNeeded(text: String): String {
+ val limit = properties.service.maxLogLength
+ if (limit <= 0) return text
+ return if (text.length > limit) "${text.take(limit)}...[truncated]" else text
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun truncateIfNeeded(text: String): String { | |
| return if (text.length > properties.service.maxLogLength) { | |
| "${text.substring(0, properties.service.maxLogLength)}...[truncated]" | |
| } else { | |
| text | |
| } | |
| } | |
| private fun truncateIfNeeded(text: String): String { | |
| val limit = properties.service.maxLogLength | |
| if (limit <= 0) return text | |
| return if (text.length > limit) "${text.take(limit)}...[truncated]" else text | |
| } |
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt
around lines 122 to 128, the truncateIfNeeded implementation should be
simplified and hardened: add a defensive check that if
properties.service.maxLogLength <= 0 then return the original text (no
truncation), otherwise use Kotlin's String.take(n) to get the prefix and append
"...[truncated]" when the original length exceeds the limit; replace the current
substring-based logic with this take-based approach to make the code shorter and
safer.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt (6)
33-51: 예외 발생 사실을 [SVC-FAILED]로 기록하지 않아 목표와 불일치 + detekt 경고 발생현재 catch 블록에서 단순 재던지기만 하여 실패 사실과 소요 시간을 남기지 못합니다. PR 목적(예외 발생 사실은 감지하되 상세는 GlobalExceptionHandler 위임)과도 어긋납니다. 또한 detekt가 TooGenericExceptionCaught/RethrowCaughtException를 경고합니다. 실패 로그를 남기고, 의도적 패턴임을 @Suppress로 명시하세요.
다음 변경을 제안합니다:
@Around("org.yapp.infra.aop.pointcut.CommonPointcuts.serviceLayer() && !org.yapp.infra.aop.pointcut.CommonPointcuts.noLogging()") - fun logService(joinPoint: ProceedingJoinPoint): Any? { + @Suppress("TooGenericExceptionCaught", "RethrowCaughtException") + fun logService(joinPoint: ProceedingJoinPoint): Any? { @@ try { val result = joinPoint.proceed() logServiceSuccess(signature, startTime, result) return result } catch (e: Throwable) { - throw e + logServiceFailure(signature, startTime, e) // 상세 스택은 전역 예외 처리기에 위임 + throw e }추가 메서드(파일 내 임의 위치, 예: logServiceSuccess 바로 아래)에 다음을 추가하세요:
private fun logServiceFailure(signature: MethodSignature, startTime: Instant, throwable: Throwable) { val className = signature.declaringType.simpleName val methodName = signature.name val duration = Duration.between(startTime, Instant.now()).toMillis() log.warn( "[SVC-FAILED] {}.{} | Exception: {} | Duration: {}ms", className, methodName, throwable.javaClass.simpleName, duration ) }
53-59: 운영 환경에서 파라미터 INFO 로깅은 과도할 수 있음 — 프로퍼티로 토글 가능하게파라미터를 항상 INFO로 남기면 운영에서 과도/민감할 수 있습니다. properties.service.logParams(기본 false)로 제어하고, 꺼져있을 땐 INFO에는 메서드명만, 파라미터는 DEBUG에 내리세요.
private fun logServiceStart(signature: MethodSignature, args: Array<Any?>) { val className = signature.declaringType.simpleName val methodName = signature.name val params = getArgumentsAsString(signature, args) - log.info("[SVC-START] {}.{} | Params: {}", className, methodName, truncateIfNeeded(params)) + if (properties.service.logParams) { + log.info("[SVC-START] {}.{} | Params: {}", className, methodName, truncateIfNeeded(params)) + } else { + log.info("[SVC-START] {}.{}", className, methodName) + if (log.isDebugEnabled) { + log.debug("[SVC-START] {}.{} | Params: {}", className, methodName, truncateIfNeeded(params)) + } + } }추가로 LoggingAopProperties에 logParams: Boolean = false 필드를 도입해야 합니다.
87-91: 마스킹 활성 조건을 프로필 하드코딩 대신 프로퍼티로 제어prod 프로필 하드코딩은 환경 구성 변화에 취약합니다. properties.service.maskingEnabled(기본 true)로 제어하는 편이 유연합니다.
- private fun shouldMaskParameter(paramName: String): Boolean { - return isProdEnvironment && properties.service.sensitiveFields.any { sensitiveField -> - paramName.lowercase().contains(sensitiveField.lowercase()) - } - } + private fun shouldMaskParameter(paramName: String): Boolean { + return properties.service.maskingEnabled && + properties.service.sensitiveFields.any { sensitiveField -> + paramName.lowercase().contains(sensitiveField.lowercase()) + } + }참고: LoggingAopProperties에 maskingEnabled: Boolean = true 추가가 필요합니다.
93-105: 배열/프리미티브 배열/바이너리 데이터는 내용 대신 크기만 기록현재 Array<*>만 크기 표기이고 ByteArray/프리미티브 배열은 누락되어 과도한 로그(또는 불필요한 변환)를 유발할 수 있습니다. 모두 크기만 표기하도록 확장하세요.
private fun formatValue(obj: Any?): String { return when (obj) { null -> "null" is Unit -> "void" is String -> "\"$obj\"" is Number, is Boolean -> obj.toString() is UUID -> obj.toString() is Enum<*> -> obj.name is Collection<*> -> "[${obj.size} items]" - is Array<*> -> "${obj.javaClass.simpleName}[${obj.size}]" + is ByteArray -> "[${obj.size} bytes]" + is IntArray -> "[${obj.size} items]" + is LongArray -> "[${obj.size} items]" + is ShortArray -> "[${obj.size} items]" + is FloatArray -> "[${obj.size} items]" + is DoubleArray -> "[${obj.size} items]" + is CharArray -> "[${obj.size} chars]" + is BooleanArray -> "[${obj.size} items]" + is Array<*> -> "[${obj.size} items]" else -> maskMapLikeObject(obj) } }
123-129: 문자열 절단 방어 로직 누락(음수/0 길이) 및 구현 간소화 가능maxLogLength ≤ 0일 때는 절단하지 않도록 방어하고, take()로 간결하게 구현하세요.
- private fun truncateIfNeeded(text: String): String { - return if (text.length > properties.service.maxLogLength) { - "${text.substring(0, properties.service.maxLogLength)}...[truncated]" - } else { - text - } - } + private fun truncateIfNeeded(text: String): String { + val limit = properties.service.maxLogLength + if (limit <= 0) return text + return if (text.length > limit) "${text.take(limit)}...[truncated]" else text + }
19-21: 마스킹/파라미터 로깅 토글용 프로퍼티 미존재 확인 및 추가 필요현재
LoggingAopProperties.ServiceLoggingProperties에maskingEnabled및logParams가 정의되어 있지 않습니다. 아래 위치에 두 프로퍼티를 추가해주세요.
- infra/src/main/kotlin/org/yapp/infra/aop/properties/LoggingAopProperties.kt
data class ServiceLoggingProperties( val enabled: Boolean = true, val maxLogLength: Int = 1000, val sensitiveFields: Set<String> = setOf( "refreshToken", "oauthToken", "authorizationCode", "providerId", "accessToken" )
/** 로그 파라미터 출력 여부 토글 */val logParams: Boolean = true,/** 민감 필드 마스킹 여부 토글 */val maskingEnabled: Boolean = true
- (선택) ServiceLoggingAspect.kt에서 새 프로퍼티를 활용하는 로직 반영 여부 검토
위 사항 반영 후 다시 확인 부탁드립니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt
[warning] 48-48: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 118-118: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 49-49: Do not rethrow a caught exception of the same type.
(detekt.exceptions.RethrowCaughtException)
[warning] 118-118: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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-validation
🔇 Additional comments (1)
infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt (1)
61-74: 성공 로그 포맷과 시간 계산 로직은 적절합니다반환값 포맷팅 후 길이 절단, 수행 시간(ms) 기록이 일관되고 명확합니다.
| private fun maskMapLikeObject(obj: Any): String { | ||
| return try { | ||
| val map: Map<*, *> = objectMapper.convertValue(obj, Map::class.java) | ||
| val maskedMap = map.mapValues { (key, value) -> | ||
| val keyStr = key.toString() | ||
| if (shouldMaskParameter(keyStr)) { | ||
| return@mapValues MASKING_TEXT | ||
| } | ||
| value | ||
| } | ||
| objectMapper.writeValueAsString(maskedMap) | ||
| } catch (e: Exception) { | ||
| "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
객체 변환 중 순환 참조/에러로 인한 장애 가능성 — Throwable 처리 및 억제 주석 추가
convertValue()는 순환 참조 시 StackOverflowError가 발생할 수 있고, 현재 Exception만 캐치하여 프로세스가 중단될 여지가 있습니다. detekt의 TooGenericExceptionCaught/SwallowedException 경고도 함께 처리하세요.
- private fun maskMapLikeObject(obj: Any): String {
+ @Suppress("TooGenericExceptionCaught", "SwallowedException")
+ private fun maskMapLikeObject(obj: Any): String {
return try {
val map: Map<*, *> = objectMapper.convertValue(obj, Map::class.java)
val maskedMap = map.mapValues { (key, value) ->
val keyStr = key.toString()
if (shouldMaskParameter(keyStr)) {
return@mapValues MASKING_TEXT
}
value
}
objectMapper.writeValueAsString(maskedMap)
- } catch (e: Exception) {
- "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}"
+ } catch (e: Throwable) {
+ if (log.isDebugEnabled) {
+ log.debug("maskMapLikeObject: fallback for {} due to {}", obj.javaClass.name, e.javaClass.simpleName)
+ }
+ "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}"
}
}추가 개선(선택): 중첩 Map/객체에 대한 재귀 마스킹(maskRecursively)을 도입하면 내부 필드까지 일관되게 마스킹할 수 있습니다. 원하시면 보일러플레이트 포함해 패치 제안 드리겠습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun maskMapLikeObject(obj: Any): String { | |
| return try { | |
| val map: Map<*, *> = objectMapper.convertValue(obj, Map::class.java) | |
| val maskedMap = map.mapValues { (key, value) -> | |
| val keyStr = key.toString() | |
| if (shouldMaskParameter(keyStr)) { | |
| return@mapValues MASKING_TEXT | |
| } | |
| value | |
| } | |
| objectMapper.writeValueAsString(maskedMap) | |
| } catch (e: Exception) { | |
| "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" | |
| } | |
| } | |
| @Suppress("TooGenericExceptionCaught", "SwallowedException") | |
| private fun maskMapLikeObject(obj: Any): String { | |
| return try { | |
| val map: Map<*, *> = objectMapper.convertValue(obj, Map::class.java) | |
| val maskedMap = map.mapValues { (key, value) -> | |
| val keyStr = key.toString() | |
| if (shouldMaskParameter(keyStr)) { | |
| return@mapValues MASKING_TEXT | |
| } | |
| value | |
| } | |
| objectMapper.writeValueAsString(maskedMap) | |
| } catch (e: Throwable) { | |
| if (log.isDebugEnabled) { | |
| log.debug("maskMapLikeObject: fallback for {} due to {}", obj.javaClass.name, e.javaClass.simpleName) | |
| } | |
| "${obj.javaClass.simpleName}@${Integer.toHexString(obj.hashCode())}" | |
| } | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 118-118: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 118-118: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/aop/aspect/ServiceLoggingAspect.kt
around lines 107–121, change the error handling in maskMapLikeObject to catch
Throwable (to avoid crashing on StackOverflowError from convertValue) and add a
focused suppression annotation/comment for detekt (e.g.
@Suppress("TooGenericExceptionCaught","SwallowedException") on the method) with
a short justification; keep the current fallback return but ensure the catch
logs or documents the suppression, and optionally replace simple masking with a
recursive maskRecursively routine for nested maps/objects if deeper masking is
desired.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
Kotlin AOP + Spring AOP의 혼합 적용 전략
Spring AOP: 요청/응답 라이프사이클 로깅
Spring 프레임워크에 내장된 프록시 기반의 AOP를 활용하여, 애플리케이션의 핵심 경계(Boundary)에서 발생하는 공통 작업을 처리합니다.
(향후 계획) Kotlin 고차 함수: DB 트랜잭션 및 캐싱 처리
MDC를 이용한 분산 요청 추적
멀티스레드 환경에서 각기 다른 요청의 로그가 뒤섞이는 문제를 해결하기 위해 MDC(Mapped Diagnostic Context)를 도입했습니다.
MdcLoggingFilter: 웹 요청의 가장 앞단에서 실행되는 필터입니다.traceId: Nginx를 통해 들어오는 요청에서 X-Request-ID 헤더를 추출하거나, 헤더가 없으면 새로 생성한 고유 ID를 사용합니다. 이 ID는 해당 요청과 관련된 모든 로그를 하나로 묶어 추적할 수 있는 식별자입니다.userId,clientIp,requestInfo: 인증된 사용자 정보, IP 주소, HTTP 요청 정보 등을 추가하여 로그의 컨텍스트를 풍부하게 만듭니다.비동기 환경에서의 MDC 전파 검증 후 방안 마련
AS-IS
TO-BE
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신기능
개선
작업(Chores)