-
Notifications
You must be signed in to change notification settings - Fork 0
[25.07.12 / TASK-92] Refactor - API에 Sentry 추가 #40
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
""" WalkthroughSentry 오류 및 성능 모니터링 도구가 프로젝트에 통합되었습니다. 환경 변수, 패키지 의존성, Sentry 초기화 설정, 그리고 오류 처리 미들웨어가 Sentry로 오류를 전송하도록 수정되었습니다. 새로운 설정 파일이 추가되고, 기존 애플리케이션 초기화 흐름이 업데이트되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Sentry
User->>App: 요청
App->>App: 환경변수 로드
App->>App: initSentry() 호출
App->>Sentry: Sentry 초기화
User->>App: 오류 발생 요청
App->>App: errorHandlingMiddleware 실행
App->>Sentry: Sentry.captureException(err) (비정상 오류 시)
App->>User: 500 Internal Server Error 응답
Suggested reviewers
Poem
✨ 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. 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 (
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.env.sample
(1 hunks)package.json
(1 hunks)src/app.ts
(2 hunks)src/configs/sentry.config.ts
(1 hunks)src/middlewares/errorHandling.middleware.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/app.ts (6)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/types/dto/user-with-token.dto.ts:3-26
Timestamp: 2024-12-04T13:28:34.692Z
Learning: In `src/types/dto/user-with-token.dto.ts`, since the project is using Express instead of NestJS, `nestjs/swagger` cannot be used. Documentation will be implemented later using an Express-specific Swagger library.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
src/middlewares/errorHandling.middleware.ts (2)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
🧬 Code Graph Analysis (1)
src/app.ts (1)
src/configs/sentry.config.ts (1)
initSentry
(6-24)
⏰ 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). (4)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (21)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (22)
🔇 Additional comments (6)
package.json (1)
27-27
: Sentry 의존성 추가 승인공식 Sentry Node.js SDK의 최신 버전을 추가한 것이 적절합니다.
src/configs/sentry.config.ts (1)
12-22
: Sentry 설정이 적절히 구성됨트레이스 샘플링 비율(10%), 디버그 모드 비활성화, HTTP/Express 통합 설정이 프로덕션 환경에 적합하게 구성되었습니다.
src/app.ts (2)
12-12
: Sentry 초기화 import 승인Sentry 설정 모듈을 적절히 임포트했습니다.
16-17
: Sentry 초기화 시점이 적절함환경 변수 로드 후 즉시 Sentry를 초기화하는 것이 모범 사례입니다. 애플리케이션 전체에서 에러 추적이 가능하도록 올바르게 설정되었습니다.
src/middlewares/errorHandling.middleware.ts (2)
4-4
: Sentry import 승인에러 핸들링 미들웨어에 Sentry를 적절히 임포트했습니다.
19-22
: Sentry 에러 캡처 로직이 올바르게 구현됨커스텀 에러가 아닌 경우에만 Sentry로 에러를 전송하는 것이 적절합니다. 예상치 못한 시스템 에러만 Sentry로 추적하고, 비즈니스 로직 에러는 기존 방식으로 처리하는 것이 모범 사례입니다.
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.
총평 이전에 hotfix 가 필수인 부분들이 있어서,, 해당 부분 바꿔주세요 ㅠㅠ
그리고 batch (CI/CD) flow 에 영향을 주지 않도록, default 값에 대한 명시가 필요할 것 같아요.
더욱이 Notion 에 업데이트 되었나요?
.env.sample
Outdated
@@ -24,3 +24,4 @@ POSTGRES_PORT=5432 | |||
# ETC | |||
SLACK_WEBHOOK_URL=https://hooks.slack.com/services | |||
SLACK_CLIENT_SECRET=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 | |||
SLACK_DSN=https://ingest.us.sentry.io |
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.
환경변수... 주의 부탁 드립니다.. 네이밍... 이번에도..ㅠㅠ
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.
이 부분을 제가 계속 놓치고 있었던 것 같네요;;
슬랙 쪽에서 발급받는 부분은 SLACK, 센트리 쪽에서 발급받는 부분은 SENTRY로 통일하는게 좋을 것 같습니다!
물론 두 값 사이에 줄바꿈은 넣어야 할 것 같네요
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.
@six-standard 그 도메인을 나누는 데에는 전혀 이견 없습니다.
근데 제가 말드리고 있는 포인트는 지금 PR에서 기준님은 전혀 지금 SLACK 이랑 관련된 값을 건들이지 않았어요. 지금은 철저하게 SENTRY 관련된 값입니다.
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.
이해했습니다.
말씀해주신대로 수정해서 반영했습니다!
|
||
export const initSentry = () => { | ||
Sentry.init({ | ||
dsn: process.env.SENTRY_DSN, |
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.
SENTRY_DSN
vs SLACK_DSN
...
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.
이 부분을 제가 계속 놓치고 있었던 것 같네요;;
슬랙 쪽에서 발급받는 부분은 SLACK, 센트리 쪽에서 발급받는 부분은 SENTRY로 통일하는게 좋을 것 같습니다!
물론 두 값 사이에 줄바꿈은 넣어야 할 것 같네요
@Nuung 이전에도 제가 놓친 부분이 또 있었던 것으로 아는데, 이런 식으로 맞추는건 어떠신가요?
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.
아뇨 이게 slack 이면 절대 안될 것 같아요.
슬랙 쪽에서 발급받는 부분은 SLACK 인데 지금 PR 은 slack 쪽에서 발급 받는게 하나도 없어요. dsn 값은 무조건 SENTRY 가 맞습니다. @six-standard
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.
이 부분도 방금 말씀해주신대로 맞췄습니다.
src/configs/sentry.config.ts
Outdated
export const initSentry = () => { | ||
Sentry.init({ | ||
dsn: process.env.SENTRY_DSN, | ||
release: 'production', |
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.
코드레빗의 코멘트가 매우 중요한데요..! 그냥 production 이면 저희가 local 이랑 구분을 못해요
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.
테스트 목적으로 수정하다가 놓친 듯 합니다;;
수정해두었습니다!
@@ -15,7 +16,10 @@ export const errorHandlingMiddleware: ErrorRequestHandler = ( | |||
.json({ success: false, message: err.message, error: { code: err.code, statusCode: err.statusCode } }); | |||
return; | |||
} | |||
|
|||
Sentry.captureException(err); |
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.
만약 여기서 에러가 발생한다면?
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.
Sentry에서 전역 오류로 감지해서 다시 전송하는 것으로 알고 있습니다!
제가 포인트를 못 잡고 있었네요;; |
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.sample
(1 hunks).github/workflows/test-ci.yaml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/test-ci.yaml
🧰 Additional context used
🧠 Learnings (1)
.env.sample (3)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: .env.sample:7-16
Timestamp: 2024-11-29T14:01:55.277Z
Learning: The AES keys provided in the `.env.sample` file are sample values and not actual encryption keys.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: .env.sample:2-6
Timestamp: 2024-11-29T14:01:26.914Z
Learning: `velog-dashboard-v2-api` 프로젝트의 `.env.sample` 파일에서는 데이터베이스 접속 정보를 포함한 환경 변수에 예시 값을 사용하며, 이는 실제 자격 증명이 아니므로 그대로 두어도 됩니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
🪛 Gitleaks (8.26.0)
.env.sample
28-28: Found a Sentry.io Access Token (old format), risking unauthorized access to error tracking services and sensitive application data.
(sentry-access-token)
⏰ 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). (4)
- GitHub Check: build-and-test (22)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (21)
- GitHub Check: build-and-test (20)
🔇 Additional comments (1)
.env.sample (1)
24-29
: 환경 변수 참조 일치 확인 완료레포 전체에서
SENTRY_DSN
/SENTRY_CLIENT_SECRET
만 사용 중이며,SLACK_DSN
과 같은 오타·잔존 레퍼런스는 존재하지 않습니다. 추가 확인이나 수정은 필요 없어 보입니다.
SLACK_CLIENT_SECRET=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 | ||
|
||
# Sentry Hook | ||
SENTRY_CLIENT_SECRET=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 |
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.
이거 제가 배포 핫픽스 하면서 SENTRY_CLIENT_SECRET
로 바궛다고 생각했는데 아니엇나!! 이 부분을 말하신거군요!
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.
저도 이 부분이 이상하다 싶어서 이번에 다 수정해두었는데, 지난번에 놓치신 부분이었나 봅니다.
사실 이런 식으로 분리하는게 필요하다 생각은 했었는데, 막상 실행을 못 했었네요;;
@@ -59,6 +59,7 @@ jobs: | |||
echo "POSTGRES_PASSWORD=${{ secrets.POSTGRES_PASSWORD }}" >> .env | |||
echo "POSTGRES_PORT=${{ secrets.POSTGRES_PORT }}" >> .env | |||
echo "SENTRY_CLIENT_SECRET=${{ secrets.SENTRY_CLIENT_SECRET }}" >> .env |
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.
이거봐! 내가 분명 바꿧는데! ㅋㅋㅋㅋ과거 코드도 ㅋㅋㅋ 실제 환경은 다 SENTRY_CLIENT_SECRET 쓰긴할거에유
🔥 변경 사항
API와 센트리를 연결하였습니다.
사실 연결 자체는 목요일쯤 끝냈는데, 이전에 현우님이 말씀해주신 '복잡한 방식'을 계속 시도하다 보니 밀렸네요..
결론적으로, 복잡한 방식은 제외하고 단순한 방식만 반영해서 올렸습니다.
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
📌 체크리스트
Summary by CodeRabbit
신규 기능
환경설정
의존성 추가