Conversation
Walkthrough이번 변경 사항은 마라톤 대회 신청 프로세스의 추적, 모니터링, 상태 관리, 그리고 관측성(Observability) 강화를 중심으로 다양한 계층에 걸쳐 이루어졌습니다. Sequence Diagram(s)sequenceDiagram
participant Client
participant CompetitionApplicationFacade
participant ApplicationSessionRepository
participant SagaServiceImpl
participant SagaContextHolder
participant Competition
participant SagaStateRepository
participant Metrics (Micrometer)
Client->>CompetitionApplicationFacade: applyForCompetition(appDto)
CompetitionApplicationFacade->>ApplicationSessionRepository: findByCompetitionAndParticipant()
alt 세션 없음
ApplicationSessionRepository-->>CompetitionApplicationFacade: null
CompetitionApplicationFacade->>ApplicationSessionRepository: saveSession(newSession)
CompetitionApplicationFacade->>Metrics: applicationStartCounter.increment()
else 세션 있음
ApplicationSessionRepository-->>CompetitionApplicationFacade: session
end
CompetitionApplicationFacade->>SagaServiceImpl: processCompleteApplication(appDto)
SagaServiceImpl->>SagaContextHolder: setCurrentSagaId(sagaId)
SagaServiceImpl->>Competition: processApplicationStep(...)
Competition->>SagaStateRepository: saveSagaState()
SagaServiceImpl->>SagaContextHolder: clear()
SagaServiceImpl-->>CompetitionApplicationFacade: stepResult
CompetitionApplicationFacade->>ApplicationSessionRepository: saveSession(updateSession)
CompetitionApplicationFacade->>Metrics: 단계별 counter/timer 기록
alt 모든 단계 완료
CompetitionApplicationFacade->>Metrics: applicationCompleteCounter.increment()
CompetitionApplicationFacade->>Metrics: applicationTotalTimeTimer.record()
else 예외 발생
CompetitionApplicationFacade->>Metrics: applicationFailCounter.increment()
end
CompetitionApplicationFacade-->>Client: stepResult
sequenceDiagram
participant SagaEventConsumer
participant SagaStateRepository
participant SagaState
SagaEventConsumer->>SagaStateRepository: getSagaState(sagaId)
SagaStateRepository-->>SagaEventConsumer: sagaState
SagaEventConsumer->>SagaState: update/markStepAsCompleted/setNextStep
SagaState-->>SagaEventConsumer: (상태 변경 및 로그)
SagaEventConsumer->>SagaStateRepository: saveSagaState(sagaState)
alt 결제 실패
SagaEventConsumer->>SagaState: markAsFailed()
SagaEventConsumer->>SagaStateRepository: saveSagaState(sagaState)
else 결제 성공
SagaEventConsumer->>SagaState: markStepAsCompleted()
SagaEventConsumer->>SagaStateRepository: saveSagaState(sagaState)
end
sequenceDiagram
participant MetricsConfig
participant MeterRegistry
participant Application
Application->>MetricsConfig: (빈 등록 시)
MetricsConfig->>MeterRegistry: counter/timer 생성 요청
MeterRegistry-->>MetricsConfig: Counter/Timer 인스턴스 반환
MetricsConfig-->>Application: Counter/Timer 빈 제공
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
singingsandhill
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (2)
competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/model/SagaState.java (1)
31-33: 🛠️ Refactor suggestion
completedSteps를List에서Set으로 변경 권장
markStepAsCompleted()에서 중복 여부를 매번contains로 확인하고 있습니다.
completedSteps를HashSet‧LinkedHashSet으로 선언하면
- O(1) 중복 체크
- 코드 간결화 (
contains검사/중복 추가 로직 불필요)- private List<SagaStep> completedSteps = new ArrayList<>(); + private Set<SagaStep> completedSteps = new LinkedHashSet<>();그리고
markStepAsCompleted를 다음과 같이 단순화할 수 있습니다.- if (!this.completedSteps.contains(step)) { - this.completedSteps.add(step); - } + this.completedSteps.add(step);Also applies to: 72-75
competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/messaging/SagaEventConsumer.java (1)
55-101: 🧹 Nitpick (assertive)🛠️ Refactor suggestion
동시 처리 시 데이터 경합 가능성 – 분산 락/트랜잭션 검토 필요
processApplicationEvent는 동일sagaId에 대한 여러 Kafka 메시지가 거의 동시 도착할 수 있습니다.
현재 구현은
- Redis 에서 상태 조회
- Java 객체 수정
- 다시 Redis 저장
을 여러 스레드가 병렬로 수행할 때 최종 상태가 꼬일 가능성이 있습니다.
해결 방안 예시
SETNX기반 분산 락(예: Redisson)으로 saga 단위 동시 실행 제한- Redis
WATCH/MULTI/EXEC트랜잭션 사용- Spring Transaction + Lua Script 로 CAS 구현
경합이 빈번한 환경이라면 위 방안을 검토해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
.gitignore(1 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/application/context/SagaContextHolder.java(1 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/application/facade/CompetitionApplicationFacade.java(5 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/application/service/SagaServiceImpl.java(4 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/model/ApplicationSession.java(1 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/model/Competition.java(6 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/model/SagaState.java(2 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/repository/ApplicationSessionRepository.java(1 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/config/MetricsConfig.java(1 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/config/RedisConfig.java(2 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/messaging/SagaEventConsumer.java(5 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/messaging/SagaOrchestratorImpl.java(1 hunks)competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/persistence/SagaStateRepository.java(3 hunks)competition-service/src/main/resources/application.properties(1 hunks)competition-service/src/main/resources/logback-spring.xml(1 hunks)grafana/grafana-dashboard.json(1 hunks)prometheus.yml(0 hunks)
💤 Files with no reviewable changes (1)
- prometheus.yml
🔇 Additional comments (31)
.gitignore (1)
44-44: 로그 파일 무시 패턴이 추가되었습니다.기존에는
logs/competition-service.log파일만 무시했지만, 이제logs/competition-service.*패턴을 추가하여 모든 관련 로그 파일을 무시하도록 했습니다. 이는 롤링 파일 정책에 의해 생성되는 날짜별 로그 파일들도 함께 무시하게 되어 좋은 변경입니다.competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/config/RedisConfig.java (3)
10-10: ApplicationSession 임포트가 추가되었습니다.신규 추가된
ApplicationSession클래스를 위한 임포트가 추가되었습니다. 이는 아래에서 구성하는 Redis 템플릿과 함께 사용됩니다.
42-50: String 기반 Redis 템플릿이 추가되었습니다.문자열 키-값 쌍을 저장하기 위한
sagaStringRedisTemplate빈이 추가되었습니다. 이는 간단한 인덱스나 조회를 위한 목적으로 보이며, 적절하게StringRedisSerializer로 구성되어 있습니다.이 템플릿은 Saga 관련 문자열 데이터를 효율적으로 저장하고 검색하는 데 유용할 것입니다. 특히 대회 신청 과정의 상태 관리와 모니터링에 도움이 될 것입니다.
52-67: ApplicationSession을 위한 Redis 템플릿이 추가되었습니다.
ApplicationSession객체를 저장하기 위한applicationSessionRedisTemplate빈이 추가되었습니다. 이는 복잡한 객체를 JSON으로 변환하여 저장하기 위해Jackson2JsonRedisSerializer를 적절히 사용하고 있습니다.이 구현은 다음과 같은 좋은 접근 방식을 사용하고 있습니다:
- 적절한 직렬화 도구 사용
- 의존성 주입 적절히 활용
- 기존 ObjectMapper 빈 재사용
- 명확한 빈 이름 지정
이는 PR 목표인 대회 도메인 애플리케이션 지표 수집을 위한 세션 관리 구현의 일부로 적절합니다.
competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/messaging/SagaOrchestratorImpl.java (1)
282-282: 코드 변경 확인되었습니다.파일 끝의 개행 문자 제거와 같은 사소한 변경만 확인됩니다. 기능적 변경사항은 없습니다.
competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/model/Competition.java (6)
14-14: SagaContextHolder 의존성 추가 확인애플리케이션 지표 수집을 위한 Saga 컨텍스트 관리 클래스 의존성이 추가되었습니다.
188-190: Saga ID 관리 방식 개선이전에는 각 이벤트마다 새로운 Saga ID를 생성했지만, 이제는 ThreadLocal을 통해 중앙 집중식으로 관리하는 방식으로 변경되었습니다. 이는 신청 프로세스 전반에 걸쳐 일관된 Saga ID를 유지하여 추적성을 향상시킵니다.
197-198: 이벤트 발행 조건 강화이벤트 퍼블리셔와 saga ID가 모두 null이 아닌 경우에만 이벤트를 발행하도록 조건이 강화되었습니다. 이는 이벤트 추적과 관측성을 향상시키는 중요한 변경사항입니다.
213-214: 기념품 선택 이벤트 발행 조건 강화이벤트 퍼블리셔와 saga ID가 모두 null이 아닌 경우에만 이벤트를 발행하도록 조건이 강화되었습니다. 일관된 조건 적용을 통해 이벤트 추적 및 메트릭 수집의 정확성을 보장합니다.
229-230: 배송지 이벤트 발행 조건 강화이벤트 퍼블리셔와 saga ID가 모두 null이 아닌 경우에만 이벤트를 발행하도록 조건이 강화되었습니다. 다른 단계들과 일관된 방식으로 적용되었습니다.
259-260: 결제 단계 이벤트 발행 조건 강화결제 완료 이벤트 발행에도 동일하게 이벤트 퍼블리셔와 saga ID 검증 로직이 적용되었습니다. 모든 단계에서 일관된 조건 적용을 통해 메트릭 수집의 완전성을 확보합니다.
competition-service/src/main/java/com/_42195km/msa/competitionservice/application/context/SagaContextHolder.java (1)
1-31: ThreadLocal을 활용한 SagaContextHolder 구현Saga ID를 ThreadLocal 변수로 관리하는 새로운 컴포넌트가 추가되었습니다. 이는 HTTP 요청이나 메시지 처리 과정에서 Saga ID를 일관되게 전파하고 관리하기 위한 중요한 구현입니다. 각 스레드마다 독립적인 컨텍스트를 유지하므로 동시성 문제를 방지할 수 있습니다.
다음 부분들이 잘 구현되었습니다:
- Saga ID 설정, 조회, 제거를 위한 정적 메서드 제공
- ThreadLocal을 사용한 스레드 안전성 확보
- 명확한 Javadoc 주석으로 사용 방법 문서화
competition-service/src/main/java/com/_42195km/msa/competitionservice/application/service/SagaServiceImpl.java (4)
8-8: SagaContextHolder 를 사용하여 스레드 로컬 컨텍스트 관리 추가Saga ID를 스레드 로컬에서 관리할 수 있는 SagaContextHolder를 가져오는 import가 추가되었습니다. 이 변경은 대회 신청 메트릭 수집에 중요한 컨텍스트 관리를 지원합니다.
62-63: Saga ID를 스레드 로컬 컨텍스트에 설정SagaContextHolder를 통해 현재 Saga ID를 스레드 로컬에 설정합니다. 이를 통해 도메인 이벤트 처리 시 일관된 Saga ID를 유지할 수 있습니다.
65-70: Saga 상태 검증 로직 강화Saga 상태가 null인 경우에 대한 예외 처리와 로깅이 추가되었습니다. 이는 애플리케이션의 안정성을 높이고 문제 추적을 용이하게 합니다.
139-142: 스레드 로컬 자원 정리try-finally 블록을 사용하여 메서드 실행 후 항상 스레드 로컬 컨텍스트를 정리합니다. 이는 메모리 누수 방지와 스레드 안전성 확보에 필수적입니다.
competition-service/src/main/java/com/_42195km/msa/competitionservice/domain/model/ApplicationSession.java (4)
9-25: 주요 세션 속성 정의 및 직렬화 지원ApplicationSession 클래스는 대회 신청 과정의 상태를 추적하기 위한 핵심 도메인 모델로, 각 단계별 타임스탬프와 상태 정보를 포함합니다. Serializable 구현으로 Redis에 저장 가능하도록 했습니다.
27-34: 세션 생성 팩토리 메서드 구현정적 팩토리 메서드 패턴을 사용하여 세션 생성 로직을 캡슐화했습니다. 이는 세션 ID 생성 및 초기 상태 설정의 일관성을 보장합니다.
36-70: 단계별 상태 변경 메서드 구현각 신청 단계 완료 및 실패 처리를 위한 메서드들이 명확하게 정의되어 있습니다. 이 메서드들은 적절한 타임스탬프를 설정하여 단계 진행 상황을 기록합니다.
68-92: 단계별 소요 시간 계산 메서드각 단계 간 소요 시간 계산 메서드를 제공하여 성능 지표 수집을 용이하게 합니다. null 체크가 잘 구현되어 있어 예외 발생을 방지합니다.
grafana/grafana-dashboard.json (6)
24-30: 대시보드 기본 설정그라파나 대시보드 설정에 대회 신청 프로세스 모니터링에 적합한 기본 설정이 잘 구성되어 있습니다. 실시간 모니터링과 분석을 위한 준비가 되어 있습니다.
42-118: 대회 신청 성공률 패널 구성대시보드 첫 섹션에 신청 성공률을 시각화하는 게이지 및 시계열 패널이 잘 구성되어 있습니다. 성공률에 대한 임계값 설정과 시간별 추이 분석이 가능합니다.
269-472: 단계별 완료율 및 전환율 패널신청 단계별 완료율과 전환율을 시각화하는 패널들이 적절하게 구성되어 있습니다. 사용자 여정에서 이탈이 발생하는 지점을 식별하는 데 유용합니다.
473-717: 처리 시간 모니터링 패널처리 시간 분포 및 단계별 소요 시간을 분석할 수 있는 패널들이 적절히 구성되어 있습니다. 백분위수 기반 성능 분석으로 성능 병목 현상을 식별할 수 있습니다.
718-894: 오류 분석 패널로그 및 오류 발생 추이를 분석할 수 있는 패널들이 포함되어 있습니다. 다양한 유형의 오류(Saga, 비즈니스, 시스템 오류 등)를 추적하여 문제 해결에 도움이 됩니다.
896-917: 대시보드 리프레시 및 태그 설정10초 간격의 리프레시 설정과 적절한 태그 지정으로 모니터링 편의성이 향상되었습니다. 기본 6시간 시간 범위는 실시간 모니터링에 적합합니다.
competition-service/src/main/java/com/_42195km/msa/competitionservice/infrastructure/config/MetricsConfig.java (5)
10-17: 메트릭 설정 클래스 구성MeterRegistry를 주입받는 설정 클래스로 메트릭 수집을 위한 기반을 잘 구성했습니다. Spring의 설정 클래스 패턴을 적절히 활용하고 있습니다.
19-70: 대회 신청 단계별 카운터 메트릭 정의대회 신청의 시작, 완료, 실패 및 각 단계별 완료 횟수를 추적하는 카운터를 빈으로 등록했습니다. 이는 PR 목적인 어플리케이션 메트릭 수집에 직접 기여합니다.
72-80: 전체 신청 프로세스 타이머 설정전체 신청 과정의 소요 시간을 측정하는 타이머를 구성했습니다. 백분위수 히스토그램을 발행하도록 설정하여 성능 분석에 필요한 상세 데이터를 제공합니다.
82-113: 단계별 소요 시간 타이머 설정각 단계별 소요 시간을 측정하는 타이머들이 잘 정의되어 있습니다. 이를 통해 어느 단계에서 지연이 발생하는지 파악하여 사용자 경험 개선에 활용할 수 있습니다.
1-114: 전반적인 메트릭 설계대회 신청 프로세스의 전체 메트릭 계층을 RED 방법론(요청율, 오류율, 지속시간)에 따라 체계적으로 설계했습니다. 이는 관측성을 크게 향상시키고 그라파나 대시보드와 잘 통합됩니다.
| eureka.client.service-url.defaultZone= ${EUREKA_SERVER} | ||
|
|
||
| spring.jpa.hibernate.ddl-auto=update | ||
| spring.jpa.hibernate.ddl-auto=create |
There was a problem hiding this comment.
spring.jpa.hibernate.ddl-auto 설정이 create로 변경되었습니다.
spring.jpa.hibernate.ddl-auto=create 설정은 애플리케이션이 시작될 때마다 데이터베이스 스키마를 새로 생성합니다. 이는 데이터 손실을 초래할 수 있으며, 특히 운영 환경에서는 위험합니다.
이 설정은 개발이나 테스트 환경에서만 사용하고, 운영 환경에서는 다음 옵션 중 하나를 권장합니다:
-spring.jpa.hibernate.ddl-auto=create
+spring.jpa.hibernate.ddl-auto=validate또는 배포 환경에 따라 설정을 다르게 적용하는 것을 고려하세요:
# 개발 환경
spring.jpa.hibernate.ddl-auto=update
# 운영 환경
spring.jpa.hibernate.ddl-auto=validate| <!-- Loki 서버 URL 설정 --> | ||
| <http> | ||
| <url>http://loki:3100/loki/api/v1/push</url> | ||
| <url>http://localhost:3100/loki/api/v1/push</url> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Loki 서버 주소가 localhost로 변경되었습니다.
Loki 서버 URL이 http://loki:3100/loki/api/v1/push에서 http://localhost:3100/loki/api/v1/push로 변경되었습니다. 이는 로컬 개발 환경에서는 작동할 수 있지만, 컨테이너화된 환경(Docker, Kubernetes 등)에서는 문제가 될 수 있습니다.
환경에 따라 유연하게 설정할 수 있도록 환경 변수를 사용하는 것이 좋습니다:
- <url>http://localhost:3100/loki/api/v1/push</url>
+ <url>${LOKI_URL:http://loki:3100/loki/api/v1/push}</url>그리고 application.properties 파일에 추가:
# Loki 설정
loki.url=http://loki:3100/loki/api/v1/push
📝 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.
| <url>http://localhost:3100/loki/api/v1/push</url> | |
| <url>${LOKI_URL:http://loki:3100/loki/api/v1/push}</url> |
| package com._42195km.msa.competitionservice.domain.repository; | ||
|
|
||
| import java.util.UUID; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| import org.springframework.data.redis.core.RedisTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import com._42195km.msa.competitionservice.domain.model.ApplicationSession; | ||
|
|
||
| @Repository | ||
| public class ApplicationSessionRepository { | ||
| private static final String SESSION_KEY_PREFIX = "app_session:"; | ||
| private static final long SESSION_TTL_HOURS = 24; | ||
|
|
||
| private final RedisTemplate<String, ApplicationSession> redisTemplate; | ||
|
|
||
| public ApplicationSessionRepository(RedisTemplate<String, ApplicationSession> redisTemplate) { | ||
| this.redisTemplate = redisTemplate; | ||
| } | ||
|
|
||
| public void saveSession(ApplicationSession session) { | ||
| String key = SESSION_KEY_PREFIX + session.getSessionId(); | ||
| redisTemplate.opsForValue().set(key, session); | ||
| redisTemplate.expire(key, SESSION_TTL_HOURS, TimeUnit.HOURS); | ||
| } | ||
|
|
||
| public ApplicationSession getSession(String sessionId) { | ||
| String key = SESSION_KEY_PREFIX + sessionId; | ||
| return redisTemplate.opsForValue().get(key); | ||
| } | ||
|
|
||
| public ApplicationSession findByCompetitionAndParticipant(UUID competitionId, UUID participantId) { | ||
| // TODO : 보조 인덱스나 검색을 사용 구현 | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redis 기반 ApplicationSession 저장소 구현
대회 신청 세션을 Redis에 저장하고 관리하는 새로운 저장소가 구현되었습니다. 24시간 TTL을 가진 세션 데이터를 효율적으로 관리하는 기능을 제공합니다.
잘 구현된 부분:
- 명확한 키 접두사와 TTL 설정
- 세션 저장 및 조회 기능
개선 필요 사항:
findByCompetitionAndParticipant메서드는 현재 구현되지 않고 null을 반환합니다. TODO 주석에 명시된 대로 보조 인덱스나 검색 기능을 빠른 시일 내에 구현해주세요.- Redis 작업 시 발생할 수 있는 예외 처리가 없습니다. 네트워크 지연이나 연결 실패와 같은 문제가 발생할 경우를 대비한 예외 처리를 추가하는 것이 좋겠습니다.
public ApplicationSession getSession(String sessionId) {
String key = SESSION_KEY_PREFIX + sessionId;
- return redisTemplate.opsForValue().get(key);
+ try {
+ return redisTemplate.opsForValue().get(key);
+ } catch (Exception e) {
+ // 로깅 및 예외 처리
+ log.error("세션 조회 중 오류 발생. 세션 ID: " + sessionId, e);
+ return null;
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| public void updateEligibilityStatus(String eligibilityStatus) { | ||
| log.debug("Updating eligibility status: {} for saga: {}", eligibilityStatus, this.sagaId); | ||
| this.eligibilityStatus = eligibilityStatus; | ||
| this.updatedAt = LocalDateTime.now(); | ||
| } | ||
|
|
||
| public void updateEligibilityReason(String eligibilityReason) { | ||
| log.debug("Updating eligibility reason: {} for saga: {}", eligibilityReason, this.sagaId); | ||
| this.eligibilityReason = eligibilityReason; | ||
| this.updatedAt = LocalDateTime.now(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
updatedAt 갱신 방식의 불일치
updateEligibilityStatus / updateEligibilityReason 두 메서드는 updateModifiedTime() 대신 직접 updatedAt 값을 설정하고 있습니다.
다른 update 메서드와 일관성을 유지하려면 updateModifiedTime() 호출로 통일하는 것이 좋습니다.
- this.eligibilityStatus = eligibilityStatus;
- this.updatedAt = LocalDateTime.now();
+ this.eligibilityStatus = eligibilityStatus;
+ updateModifiedTime();같은 변경을 updateEligibilityReason에도 적용해 주세요.
📝 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.
| public void updateEligibilityStatus(String eligibilityStatus) { | |
| log.debug("Updating eligibility status: {} for saga: {}", eligibilityStatus, this.sagaId); | |
| this.eligibilityStatus = eligibilityStatus; | |
| this.updatedAt = LocalDateTime.now(); | |
| } | |
| public void updateEligibilityReason(String eligibilityReason) { | |
| log.debug("Updating eligibility reason: {} for saga: {}", eligibilityReason, this.sagaId); | |
| this.eligibilityReason = eligibilityReason; | |
| this.updatedAt = LocalDateTime.now(); | |
| } | |
| public void updateEligibilityStatus(String eligibilityStatus) { | |
| log.debug("Updating eligibility status: {} for saga: {}", eligibilityStatus, this.sagaId); | |
| this.eligibilityStatus = eligibilityStatus; | |
| updateModifiedTime(); | |
| } | |
| public void updateEligibilityReason(String eligibilityReason) { | |
| log.debug("Updating eligibility reason: {} for saga: {}", eligibilityReason, this.sagaId); | |
| this.eligibilityReason = eligibilityReason; | |
| updateModifiedTime(); | |
| } |
| import com._42195km.msa.competitionservice.domain.model.SagaStatus; | ||
| import com._42195km.msa.competitionservice.infrastructure.persistence.ParticipantDetailRepositoryImpl; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
사용되지 않는 SagaStatus import 삭제
현재 파일 내에서 SagaStatus 타입을 참조하지 않습니다.
불필요한 import 는 제거하여 가독성과 빌드 속도를 개선하세요.
| if (!"SUCCESS".equals(event.getPaymentStatus())) { | ||
| log.error("Payment not successful for saga: {}", event.getSagaId()); | ||
| sagaState.markAsFailed(); | ||
| sagaStateRepository.saveSagaState(sagaState); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
하드코딩된 "SUCCESS" 문자열 → Enum 사용 권장
결제 상태를 문자열로 비교하면 오타·대소문자 오류에 취약합니다.
PaymentStatus.SUCCESS 같은 enum / 상수로 치환하여 타입 안전성을 확보하세요.
-if (!"SUCCESS".equals(event.getPaymentStatus())) {
+if (event.getPaymentStatus() != PaymentStatus.SUCCESS) {(enum 정의가 없다면 새로 추가)
Committable suggestion skipped: line range outside the PR's diff.
| // 활성 상태인 경우 인덱스 저장 (문자열 템플릿 사용) | ||
| if (state.getStatus() == SagaStatus.STARTED || state.getStatus() == SagaStatus.IN_PROGRESS) { | ||
| String indexKey = "sagaIndex:" + state.getCompetitionId() + ":" + state.getParticipantId(); | ||
| sagaStringRedisTemplate.opsForValue().set(indexKey, state.getSagaId()); | ||
| sagaStringRedisTemplate.expire(indexKey, SAGA_TTL_HOURS, TimeUnit.HOURS); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Saga 완료·실패 시 인덱스 즉시 삭제 고려
지금은 STARTED/IN_PROGRESS 에만 인덱스를 쓰고,
완료 후엔 나중에 조회될 때 Lazy 삭제합니다.
- 즉시 삭제하지 않으면 같은 참가자·대회로 신규 Saga 시작 시 “활성 Saga 존재” 오탐 가능
- TTL 24 h 동안 메모리를 불필요하게 사용
saveSagaState 내부에서 COMPLETED/FAILED/COMPENSATED 상태로 전환될 때 해당 인덱스 키를 즉시 delete하도록 보완하는 방안을 권장합니다.
| public CompetitionApplicationFacade( | ||
| CompetitionService competitionService, | ||
| ParticipantService participantService, | ||
| SagaService sagaService, | ||
| ApplicationSessionRepository sessionRepository, | ||
| @Qualifier("applicationStartCounter") Counter applicationStartCounter, | ||
| @Qualifier("applicationCompleteCounter") Counter applicationCompleteCounter, | ||
| @Qualifier("applicationFailCounter") Counter applicationFailCounter, | ||
| @Qualifier("termsAgreementCounter") Counter termsAgreementCounter, | ||
| @Qualifier("souvenirSelectionCounter") Counter souvenirSelectionCounter, | ||
| @Qualifier("shippingAddressCounter") Counter shippingAddressCounter, | ||
| @Qualifier("paymentCounter") Counter paymentCounter, | ||
| @Qualifier("applicationTotalTimeTimer") Timer applicationTotalTimeTimer, | ||
| @Qualifier("termsStepTimeTimer") Timer termsStepTimeTimer, | ||
| @Qualifier("souvenirStepTimeTimer") Timer souvenirStepTimeTimer, | ||
| @Qualifier("shippingStepTimeTimer") Timer shippingStepTimeTimer, | ||
| @Qualifier("paymentStepTimeTimer") Timer paymentStepTimeTimer | ||
| ) { | ||
| this.competitionService = competitionService; | ||
| this.participantService = participantService; | ||
| this.sagaService = sagaService; | ||
| this.sessionRepository = sessionRepository; | ||
| this.applicationStartCounter = applicationStartCounter; | ||
| this.applicationCompleteCounter = applicationCompleteCounter; | ||
| this.applicationFailCounter = applicationFailCounter; | ||
| this.termsAgreementCounter = termsAgreementCounter; | ||
| this.souvenirSelectionCounter = souvenirSelectionCounter; | ||
| this.shippingAddressCounter = shippingAddressCounter; | ||
| this.paymentCounter = paymentCounter; | ||
| this.applicationTotalTimeTimer = applicationTotalTimeTimer; | ||
| this.termsStepTimeTimer = termsStepTimeTimer; | ||
| this.souvenirStepTimeTimer = souvenirStepTimeTimer; | ||
| this.shippingStepTimeTimer = shippingStepTimeTimer; | ||
| this.paymentStepTimeTimer = paymentStepTimeTimer; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
메트릭 Bean 주입의 가독성 및 유지보수성 개선 제안
현재 생성자에 10개 이상의 Counter, Timer가 개별 주입되고 있어 서명과 DI 설정이 장황합니다.
빈 수가 늘어날 경우 생성자가 급격히 비대해지므로, 전용 DTO/컨테이너(MetricsHolder)로 래핑하거나
Map<String, Counter> 형태로 주입받아 이름으로 조회하도록 리팩터링을 고려해 보세요.
장점
- 생성자 간결화 및 테스트 용이성 향상
- 메트릭 추가‧삭제 시 파사드 코드 변경 최소화
- Bean Qualifier 남발로 인한 오타‧중복 위험 감소
선택 사항이지만 장기적으로 운영 서비스의 가독성을 크게 높일 수 있습니다.
| // 현재 단계 결정 | ||
| ApplicationStep currentStep = determineCurrentStep(appDto); | ||
|
|
||
| // 서비스 호출하여 실제 처리 | ||
| String response = sagaService.processCompleteApplication(appDto); | ||
|
|
||
| // 결과에 따라 세션 업데이트 및 메트릭 기록 | ||
| updateSessionAndMetrics(session, currentStep, appDto, response); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
사용되지 않는 매개변수 제거 또는 활용
currentStep과 response를 updateSessionAndMetrics 인자로 전달하지만 내부에서 전혀 사용되지 않습니다.
경고를 유발할 뿐 아니라 향후 유지보수 시 혼란을 초래하므로 제거하거나 실제 로깅/메트릭에 활용하세요.
-ApplicationStep currentStep = determineCurrentStep(appDto);
-...
-updateSessionAndMetrics(session, currentStep, appDto, response);
+// currentStep가 불필요하다면 아래처럼 단순화
+updateSessionAndMetrics(session, appDto);-private void updateSessionAndMetrics(ApplicationSession session, ApplicationStep currentStep,
- CompleteAppDto appDto, String response) {
+private void updateSessionAndMetrics(ApplicationSession session, CompleteAppDto appDto) {불필요한 인자를 제거하면 코드가 간결해지고 의도가 명확해집니다.
Also applies to: 217-220
| // 1. 약관 동의 단계 (항상 처음부터 확인) | ||
| if (appDto.getTermsAgreed() != null && appDto.getTermsAgreed()) { | ||
| if (session.getTermsAgreedTime() == null) { | ||
| session.completeTermsAgreement(); | ||
| termsAgreementCounter.increment(); | ||
| termsStepTimeTimer.record(session.getTermsStepTimeMillis(), TimeUnit.MILLISECONDS); | ||
| log.info("약관 동의 단계 완료: sessionId={}, 소요시간={}ms", | ||
| session.getSessionId(), session.getTermsStepTimeMillis()); | ||
| } | ||
|
|
||
| // 2. 약관 동의가 완료된 후에만 기념품 선택 단계 처리 | ||
| if (appDto.getSouvenirSelection() != null && session.getTermsAgreedTime() != null) { | ||
| if (session.getSouvenirSelectedTime() == null) { | ||
| session.completeSouvenirSelection(); | ||
| souvenirSelectionCounter.increment(); | ||
| souvenirStepTimeTimer.record(session.getSouvenirStepTimeMillis(), TimeUnit.MILLISECONDS); | ||
| log.info("기념품 선택 단계 완료: sessionId={}, 소요시간={}ms", | ||
| session.getSessionId(), session.getSouvenirStepTimeMillis()); | ||
| } | ||
|
|
||
| // 3. 기념품 선택이 완료된 후에만 배송지 입력 단계 처리 | ||
| if (appDto.getShippingAddress() != null && session.getSouvenirSelectedTime() != null) { | ||
| if (session.getShippingEnteredTime() == null) { | ||
| session.completeShippingAddress(); | ||
| shippingAddressCounter.increment(); | ||
| shippingStepTimeTimer.record(session.getShippingStepTimeMillis(), TimeUnit.MILLISECONDS); | ||
| log.info("배송지 입력 단계 완료: sessionId={}, 소요시간={}ms", | ||
| session.getSessionId(), session.getShippingStepTimeMillis()); | ||
| } | ||
|
|
||
| // 4. 배송지 입력이 완료된 후에만 결제 완료 단계 처리 | ||
| if (appDto.getPaymentStatus() != null && "SUCCESS".equals(appDto.getPaymentStatus()) | ||
| && session.getShippingEnteredTime() != null) { | ||
| if (session.getPaymentCompletedTime() == null) { | ||
| session.completePayment(); | ||
| paymentCounter.increment(); | ||
| paymentStepTimeTimer.record(session.getPaymentStepTimeMillis(), TimeUnit.MILLISECONDS); | ||
|
|
||
| // 모든 단계가 완료되었는지 확인하여 신청 완료 처리 | ||
| if (isAllStepsCompleted(session)) { | ||
| session.complete(); | ||
| applicationCompleteCounter.increment(); | ||
| applicationTotalTimeTimer.record(session.getTotalTimeMillis(), TimeUnit.MILLISECONDS); | ||
| log.info("대회 신청 프로세스 완료: sessionId={}, 총 소요시간={}ms", | ||
| session.getSessionId(), session.getTotalTimeMillis()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
후속 단계가 처리되지 않는 논리적 결함 수정 필요
updateSessionAndMetrics 메서드의 최상위 조건문이 termsAgreed 체크로 감싸져 있어,
약관 동의 이후 단계(기념품 선택, 배송지 입력, 결제 완료)가 요청 DTO에 termsAgreed 필드가 다시 전달되지 않으면 절대 실행되지 않습니다.
프론트엔드가 단계별로 필요한 필드만 전송할 가능성이 높기 때문에, 실제 운영 시 후속 단계가 진행되지 않는 치명적 버그가 발생합니다.
-// 1. 약관 동의 단계 (항상 처음부터 확인)
-if (appDto.getTermsAgreed() != null && appDto.getTermsAgreed()) {
- ...
-}
+// 1. 약관 동의 단계
+if (appDto.getTermsAgreed() != null && appDto.getTermsAgreed()
+ && session.getTermsAgreedTime() == null) {
+ session.completeTermsAgreement();
+ ...
+}
+
+// 2. 기념품 선택 단계 (선행 단계 완료 여부만 확인)
+if (appDto.getSouvenirSelection() != null
+ && session.getTermsAgreedTime() != null
+ && session.getSouvenirSelectedTime() == null) {
+ ...
+}
+
+// 3. 배송지 입력 단계
+if (appDto.getShippingAddress() != null
+ && session.getSouvenirSelectedTime() != null
+ && session.getShippingEnteredTime() == null) {
+ ...
+}
+
+// 4. 결제 완료 단계
+if ("SUCCESS".equals(appDto.getPaymentStatus())
+ && session.getShippingEnteredTime() != null
+ && session.getPaymentCompletedTime() == null) {
+ ...
+}위와 같이 각 단계별로 독립적인 if 블록을 두고, 선행 단계 완료 여부만 확인하도록 수정해야 합니다.
그렇지 않으면 메트릭 누락 및 사용자 불만이 발생할 수 있습니다.
📝 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.
| // 1. 약관 동의 단계 (항상 처음부터 확인) | |
| if (appDto.getTermsAgreed() != null && appDto.getTermsAgreed()) { | |
| if (session.getTermsAgreedTime() == null) { | |
| session.completeTermsAgreement(); | |
| termsAgreementCounter.increment(); | |
| termsStepTimeTimer.record(session.getTermsStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("약관 동의 단계 완료: sessionId={}, 소요시간={}ms", | |
| session.getSessionId(), session.getTermsStepTimeMillis()); | |
| } | |
| // 2. 약관 동의가 완료된 후에만 기념품 선택 단계 처리 | |
| if (appDto.getSouvenirSelection() != null && session.getTermsAgreedTime() != null) { | |
| if (session.getSouvenirSelectedTime() == null) { | |
| session.completeSouvenirSelection(); | |
| souvenirSelectionCounter.increment(); | |
| souvenirStepTimeTimer.record(session.getSouvenirStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("기념품 선택 단계 완료: sessionId={}, 소요시간={}ms", | |
| session.getSessionId(), session.getSouvenirStepTimeMillis()); | |
| } | |
| // 3. 기념품 선택이 완료된 후에만 배송지 입력 단계 처리 | |
| if (appDto.getShippingAddress() != null && session.getSouvenirSelectedTime() != null) { | |
| if (session.getShippingEnteredTime() == null) { | |
| session.completeShippingAddress(); | |
| shippingAddressCounter.increment(); | |
| shippingStepTimeTimer.record(session.getShippingStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("배송지 입력 단계 완료: sessionId={}, 소요시간={}ms", | |
| session.getSessionId(), session.getShippingStepTimeMillis()); | |
| } | |
| // 4. 배송지 입력이 완료된 후에만 결제 완료 단계 처리 | |
| if (appDto.getPaymentStatus() != null && "SUCCESS".equals(appDto.getPaymentStatus()) | |
| && session.getShippingEnteredTime() != null) { | |
| if (session.getPaymentCompletedTime() == null) { | |
| session.completePayment(); | |
| paymentCounter.increment(); | |
| paymentStepTimeTimer.record(session.getPaymentStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| // 모든 단계가 완료되었는지 확인하여 신청 완료 처리 | |
| if (isAllStepsCompleted(session)) { | |
| session.complete(); | |
| applicationCompleteCounter.increment(); | |
| applicationTotalTimeTimer.record(session.getTotalTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("대회 신청 프로세스 완료: sessionId={}, 총 소요시간={}ms", | |
| session.getSessionId(), session.getTotalTimeMillis()); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| // 1. 약관 동의 단계 | |
| if (appDto.getTermsAgreed() != null | |
| && appDto.getTermsAgreed() | |
| && session.getTermsAgreedTime() == null) { | |
| session.completeTermsAgreement(); | |
| termsAgreementCounter.increment(); | |
| termsStepTimeTimer.record(session.getTermsStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("약관 동의 단계 완료: sessionId={}, 소요시간={}ms", | |
| session.getSessionId(), session.getTermsStepTimeMillis()); | |
| } | |
| // 2. 기념품 선택 단계 (선행 단계 완료 여부만 확인) | |
| if (appDto.getSouvenirSelection() != null | |
| && session.getTermsAgreedTime() != null | |
| && session.getSouvenirSelectedTime() == null) { | |
| session.completeSouvenirSelection(); | |
| souvenirSelectionCounter.increment(); | |
| souvenirStepTimeTimer.record(session.getSouvenirStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("기념품 선택 단계 완료: sessionId={}, 소요시간={}ms", | |
| session.getSessionId(), session.getSouvenirStepTimeMillis()); | |
| } | |
| // 3. 배송지 입력 단계 | |
| if (appDto.getShippingAddress() != null | |
| && session.getSouvenirSelectedTime() != null | |
| && session.getShippingEnteredTime() == null) { | |
| session.completeShippingAddress(); | |
| shippingAddressCounter.increment(); | |
| shippingStepTimeTimer.record(session.getShippingStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("배송지 입력 단계 완료: sessionId={}, 소요시간={}ms", | |
| session.getSessionId(), session.getShippingStepTimeMillis()); | |
| } | |
| // 4. 결제 완료 단계 | |
| if ("SUCCESS".equals(appDto.getPaymentStatus()) | |
| && session.getShippingEnteredTime() != null | |
| && session.getPaymentCompletedTime() == null) { | |
| session.completePayment(); | |
| paymentCounter.increment(); | |
| paymentStepTimeTimer.record(session.getPaymentStepTimeMillis(), TimeUnit.MILLISECONDS); | |
| // 모든 단계가 완료되었는지 확인하여 신청 완료 처리 | |
| if (isAllStepsCompleted(session)) { | |
| session.complete(); | |
| applicationCompleteCounter.increment(); | |
| applicationTotalTimeTimer.record(session.getTotalTimeMillis(), TimeUnit.MILLISECONDS); | |
| log.info("대회 신청 프로세스 완료: sessionId={}, 총 소요시간={}ms", | |
| session.getSessionId(), session.getTotalTimeMillis()); | |
| } | |
| } |
3 participants
관련 이슈
변경 타입
변경 내용
as-is
to-be
코멘트
Summary by CodeRabbit
신규 기능
버그 수정
문서화
설정
스타일/리팩터링