-
Notifications
You must be signed in to change notification settings - Fork 0
[YS-571] feature: 공고 매칭 전송 시, 디스코드 알림 채널로 daily Job 정보 전송하도록 설정 #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the WalkthroughDiscord 통합을 다중 웹훅 구조로 마이그레이션하고, AlertGateway에 알림 채널과 sendNotify를 추가하며 Feign 클라이언트를 경로 기반 webhook(id/token) 호출로 변경했습니다. 스케줄러와 구현체에 알림 전송 및 MDC 기반 로깅/메트릭이 통합되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Job as SendMatchingEmailJob
participant Repo as Repository
participant ES as EmailService
participant Alert as AlertGateway
participant Impl as AlertGatewayImpl
participant FC as DiscordFeignClient
participant Discord as Discord API
Note over Job,Discord: 매칭 작업 실행 및 이메일 전송 루프
Job->>Repo: 매칭 조회/처리 (MDC 설정)
Job->>ES: 이메일 전송 시도 (each recipient)
ES-->>Job: 성공/실패
alt 실패
Job->>Alert: sendError(exception, requestUrl, clientIp)
Alert->>Impl: channel = ERRORS
Impl->>FC: sendMessage(id=errors.id, token=errors.token, errorPayload)
FC->>Discord: POST /api/webhooks/{id}/{token}
end
Note over Job,Discord: 작업 완료 후 요약 알림
Job->>Job: 집계(성공/실패, took, 메트릭)
Job->>Alert: sendNotify(title, description, content)
Alert->>Impl: channel = NOTIFY
Impl->>FC: sendMessage(id=notify.id, token=notify.token, notifyPayload)
FC->>Discord: POST /api/webhooks/{id}/{token}
Discord-->>FC: 응답
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 추가 검토 필요 영역:
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
infrastructure/src/main/kotlin/com/dobby/scheduler/SendMatchingEmailJob.kt (1)
82-97: 알림 본문 날짜 계산 일관성 검토 필요알림 제목에 표시하는 날짜만
LocalDate.now()(시스템 시각)를 사용하고 나머지 시간 계산은TimeProvider.currentDateTime()에 의존하고 있습니다. 운영 환경에서TimeProvider가 테스트/고정 시각을 제공하거나 다른 타임존을 사용하면 날짜가 서로 어긋날 수 있습니다. 동일한 소스(예:start.toLocalDate()또는TimeProvider)를 사용해 날짜와 시간을 산출하도록 정리해 주세요.infrastructure/src/main/kotlin/com/dobby/external/gateway/discord/AlertGatewayImpl.kt (1)
68-83: 함수 가시성을private으로 변경하세요.
createNotifyMessage함수는 내부적으로sendNotify에서만 사용되는 헬퍼 함수입니다. 불필요한 구현 세부사항을 노출하지 않도록private으로 선언하는 것이 좋습니다.다음과 같이 수정하세요:
- fun createNotifyMessage( + private fun createNotifyMessage( title: String, description: String, content: String? = null ): DiscordMessageRequest {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
domain/src/main/kotlin/com/dobby/gateway/AlertChannel.kt(1 hunks)domain/src/main/kotlin/com/dobby/gateway/AlertGateway.kt(1 hunks)infrastructure/src/main/kotlin/com/dobby/config/properties/DiscordProperties.kt(1 hunks)infrastructure/src/main/kotlin/com/dobby/external/feign/discord/DiscordFeignClient.kt(1 hunks)infrastructure/src/main/kotlin/com/dobby/external/gateway/discord/AlertGatewayImpl.kt(4 hunks)infrastructure/src/main/kotlin/com/dobby/persistence/repository/ExperimentPostCustomRepositoryImpl.kt(4 hunks)infrastructure/src/main/kotlin/com/dobby/scheduler/SendMatchingEmailJob.kt(3 hunks)infrastructure/src/main/resources/application.yml(1 hunks)infrastructure/src/main/resources/template-application-local.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-12T04:32:41.302Z
Learnt from: chock-cho
Repo: YAPP-Github/Gradmeet-BE PR: 93
File: src/main/kotlin/com/dobby/backend/infrastructure/scheduler/SendMatchingEmailJob.kt:22-57
Timestamp: 2025-02-12T04:32:41.302Z
Learning: In Quartz Job implementations, use `runBlocking` instead of `CoroutineScope.launch` to ensure all coroutine operations complete before the job ends. This is especially important for tasks that need guaranteed completion, such as email sending jobs.
Applied to files:
infrastructure/src/main/kotlin/com/dobby/scheduler/SendMatchingEmailJob.kt
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Check Code Quality
🔇 Additional comments (10)
infrastructure/src/main/kotlin/com/dobby/external/gateway/discord/AlertGatewayImpl.kt (4)
14-17: LGTM!생성자를 통한 의존성 주입이 적절하게 구현되었습니다.
31-33: LGTM!에러 메시지를 적절하게 채널 기반 전송으로 위임하고 있습니다.
35-41: LGTM!새로운 알림 전송 기능이 적절하게 구현되었습니다. PR 목표인 디스코드 알림 채널로의 Job 정보 전송을 지원합니다.
43-67: LGTM!함수명이 더 명확해졌고, 내장 메시지 포맷팅이 개선되었습니다.
infrastructure/src/main/kotlin/com/dobby/external/feign/discord/DiscordFeignClient.kt (2)
10-13: LGTM!Discord의 베이스 URL을 하드코딩하는 것은 적절합니다. 웹훅별 자격증명(id/token)은 경로 변수로 전달되어 구성 가능하게 유지됩니다.
16-24: LGTM!Discord 웹훅 API 명세에 맞게 경로 기반 엔드포인트로 올바르게 변경되었습니다. 이를 통해 여러 웹훅(errors, notify) 지원이 가능합니다.
infrastructure/src/main/kotlin/com/dobby/persistence/repository/ExperimentPostCustomRepositoryImpl.kt (4)
33-33: LGTM! 필요한 import가 올바르게 추가되었습니다.MDC 기반 메트릭과 시간 포맷터를 위한 import가 적절하게 추가되었습니다.
Also applies to: 38-38
48-50: LGTM! 스레드 안전한 포맷터가 올바르게 정의되었습니다.
DateTimeFormatter는 불변(immutable)이고 스레드 안전하므로 companion object의 상수로 사용하기에 적합합니다.
271-271: LGTM! 로그 메시지가 더 명확해졌습니다."알림 수신에 동의한 participants"라는 설명을 추가하여 로그 메시지의 의미가 더욱 명확해졌습니다.
315-319: MDC 정리는 이미 구현되어 있습니다.검증 결과, SendMatchingEmailJob.kt의 73-76번 줄에서 모든 MDC 키가 명시적으로 정리되고 있습니다:
listOf( "match.windowStart","match.windowEnd","match.todayPosts", "match.consentParticipants","match.matchedRecipients" ).forEach { MDC.remove(it) }ExperimentPostCustomRepositoryImpl.kt에서 설정한 5개의 모든 키가 사용 후 적절히 제거되므로, 메모리 누수나 컨텍스트 누수 문제는 없습니다.
Likely an incorrect or invalid review comment.
| @ConfigurationProperties("discord") | ||
| data class DiscordProperties ( | ||
| val webhooks: Map<String, Webhook> = emptyMap() | ||
| ) { | ||
| data class Webhook( | ||
| val id: String, | ||
| val token: String | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구성 프로퍼티 바인딩 실패 위험
@ConfigurationProperties를 생성자 기반으로 바인딩하려면 스프링이 이 클래스를 전용으로 인스턴스화해야 하는데, 지금처럼 @Component만 붙이면 빈 생성 시점에 기본값(emptyMap())으로 생성되고 이후 바인더가 val webhooks를 덮어쓸 수 없습니다. 결과적으로 discord.webhooks가 항상 비어 있어 런타임에 채널별 웹훅을 찾지 못하는 장애가 발생합니다. 기존 패턴처럼 @ConstructorBinding(Boot 2.x) 또는 @ConfigurationPropertiesScan/@EnableConfigurationProperties에만 등록해 스프링이 생성자 바인딩을 수행하도록 수정해 주세요. 예시는 다음과 같습니다.
-@ConfigurationProperties("discord")
-@Component
-data class DiscordProperties (
+@ConfigurationProperties("discord")
+@ConstructorBinding
+data class DiscordProperties(스캔/등록 방법은 프로젝트에서 사용 중인 방식에 맞춰 정리 부탁드립니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
in
infrastructure/src/main/kotlin/com/dobby/config/properties/DiscordProperties.kt
around lines 6 to 13, the constructor-based @ConfigurationProperties class is at
risk of being instantiated with default values (emptyMap) if picked up only as a
component; change registration so Spring performs constructor binding: remove
@Component if present and either annotate the class with @ConstructorBinding (or
the Boot-appropriate equivalent) or register the type with
@EnableConfigurationProperties(DiscordProperties::class) or ensure it's picked
up by @ConfigurationPropertiesScan in your project; keep the data class
constructor immutable so the binder can populate webhooks at creation time.
| fun send(channel: AlertChannel, body: DiscordMessageRequest) { | ||
| val key = when (channel) { | ||
| AlertChannel.ERRORS -> "errors" | ||
| AlertChannel.NOTIFY -> "notify" | ||
| } | ||
| val hook = discordProperties.webhooks[key] ?: return | ||
| runCatching { | ||
| discordFeignClient.sendMessage(hook.id, hook.token, body) | ||
| }.onFailure { | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
웹훅 전송 실패 시 로깅을 추가하세요.
onFailure 블록이 비어있어 Discord 메시지 전송 실패가 조용히 무시됩니다. 디버깅과 모니터링을 위해 실패 시 로그를 남겨야 합니다.
다음과 같이 로깅을 추가하는 것을 권장합니다:
runCatching {
discordFeignClient.sendMessage(hook.id, hook.token, body)
}.onFailure {
+ logger.error("Failed to send Discord message to channel: $channel", it)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
in
infrastructure/src/main/kotlin/com/dobby/external/gateway/discord/AlertGatewayImpl.kt
around lines 19-29, the onFailure block swallowing exceptions causes webhook
send failures to be silent; update the onFailure to log the failure with context
(e.g., channel name and hook id but not tokens) and the exception stacktrace
(e.g., logger.error("Failed to send Discord webhook for channel=$channel
hookId=${hook.id}", it)), so failures are visible for debugging/monitoring while
preserving current control flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
infrastructure/src/main/kotlin/com/dobby/external/gateway/discord/AlertGatewayImpl.kt (1)
25-28: 웹훅 실패 시 로그가 남지 않습니다.
runCatching의onFailure가 비어 있어 실패가 조용히 묻혀버립니다. 이전 리뷰에서도 동일 지적이 있었으니, 채널·hook 식별 정보와 함께 에러 로그를 남겨 원인 파악이 가능하도록 해주세요. (이 변경에는private val logger = LoggerFactory.getLogger(AlertGatewayImpl::class.java)와LoggerFactoryimport 추가가 필요합니다.)- }.onFailure { - } + }.onFailure { + logger.error("Failed to send Discord webhook for channel=$channel hookId=${hook.id}", it) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/src/main/kotlin/com/dobby/config/properties/DiscordProperties.kt(1 hunks)infrastructure/src/main/kotlin/com/dobby/external/gateway/discord/AlertGatewayImpl.kt(4 hunks)infrastructure/src/main/kotlin/com/dobby/scheduler/SendMatchingEmailJob.kt(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/src/main/kotlin/com/dobby/config/properties/DiscordProperties.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-12T04:32:41.302Z
Learnt from: chock-cho
Repo: YAPP-Github/Gradmeet-BE PR: 93
File: src/main/kotlin/com/dobby/backend/infrastructure/scheduler/SendMatchingEmailJob.kt:22-57
Timestamp: 2025-02-12T04:32:41.302Z
Learning: In Quartz Job implementations, use `runBlocking` instead of `CoroutineScope.launch` to ensure all coroutine operations complete before the job ends. This is especially important for tasks that need guaranteed completion, such as email sending jobs.
Applied to files:
infrastructure/src/main/kotlin/com/dobby/scheduler/SendMatchingEmailJob.kt
⏰ 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
infrastructure/src/main/kotlin/com/dobby/scheduler/SendMatchingEmailJob.kt
Show resolved
Hide resolved
* feature: add NOTIFY enums to seperate DiscordFeign channel * feature: apply MDC ThreadLocal pattern to capture matching summary logs to send discord message * style: apply ktlintFormat * fix: fix count units * style: format time info
* feature: add NOTIFY enums to seperate DiscordFeign channel * feature: apply MDC ThreadLocal pattern to capture matching summary logs to send discord message * style: apply ktlintFormat * fix: fix count units * style: format time info
💡 작업 내용
✅ 셀프 체크리스트
🙋🏻 확인해주세요
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-571
Summary by CodeRabbit
새로운 기능
개선 사항
설정
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-571