Skip to content

feat: Sentry Performance 전체 활성화 (APM + Prisma + Profiling)#348

Open
happycastle114 wants to merge 3 commits intomainfrom
feat/sentry-full-performance
Open

feat: Sentry Performance 전체 활성화 (APM + Prisma + Profiling)#348
happycastle114 wants to merge 3 commits intomainfrom
feat/sentry-full-performance

Conversation

@happycastle114
Copy link
Copy Markdown

개요

Sentry의 Performance Monitoring 기능을 모든 앱에 걸쳐 전면 활성화합니다.

변경 사항

🏗 아키텍처 개선

  • 각 앱에 instrument.ts 파일을 생성하여 Sentry v9 권장 패턴을 따릅니다
  • Sentry.init()이 모든 모듈보다 먼저 로드되도록 보장합니다
  • server-consumer의 초기화 순서 버그를 수정합니다 (기존: NestFactory.create 이후 → 수정: 이전)

📊 활성화된 기능

기능 설명
HTTP Tracing 모든 API 요청의 응답 시간, 상태 코드 추적
Prisma Integration 모든 DB 쿼리를 Span으로 추적 (N+1 탐지, 느린 쿼리 식별)
CPU Profiling 샘플링된 트랜잭션의 CPU 프로파일 수집 (핫 코드 경로 식별)
Environment/Release 태깅 환경별(local/dev/prod) 필터링 및 릴리즈 추적

🎯 샘플링 전략

  • Production: traces 20%, profiles 10% (비용 최적화)
  • Dev/Local: traces 100%, profiles 100% (디버깅 용이)

📦 영향 범위

  • server — 기존 설정 개선 (instrument.ts 분리 + integrations 추가)
  • server-consumer — 초기화 순서 수정 + instrument.ts 분리
  • notification-consumer신규 Sentry 연동
  • scholar-sync신규 Sentry 연동

⚠️ 배포 시 필요한 작업

  • notification-consumer: SENTRY_DSN_NOTIFICATION_CONSUMER 환경변수 추가
  • scholar-sync: SENTRY_DSN_SCHOLAR_SYNC 환경변수 추가
  • Self-hosted Sentry에 해당 프로젝트 생성 필요
  • @sentry/profiling-node 패키지 설치 (yarn install)

- Create dedicated instrument.ts files for each app (server, server-consumer,
  notification-consumer, scholar-sync) following Sentry v9 best practices
- Move Sentry.init() to instrument.ts imported before all other modules
- Add Prisma integration (prismaIntegration) for DB query tracing
- Add CPU profiling (nodeProfilingIntegration) for bottleneck detection
- Set environment-aware sample rates (prod: 20% traces, 10% profiles)
- Add environment and release tags to all Sentry events
- Fix server-consumer init order (was after NestFactory.create)
- Add Sentry to notification-consumer and scholar-sync (previously missing)
- Add SentryModule.forRoot() to notification-consumer and scholar-sync modules
- Add @sentry/profiling-node dependency
- Add --release flag to all sentry:sourcemaps:* scripts for code mapping
- Add sourcemap scripts for notification-consumer (dev/prod)
- Add sourcemap scripts for scholar-sync (dev/prod)
- Add sourcemap script for server-consumer prod
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review by ChatGPT

Comment thread package.json
"sentry:sourcemaps:consumer-prod": "sentry-cli sourcemaps inject --org sentry --project otl-server-consumer-prod ./dist/apps/server-consumer && sentry-cli --url https://sentry.sparcs.org/ sourcemaps upload --org sentry --project otl-server-consumer-prod --release=$(node -p \"require('./package.json').version\") ./dist/apps/server-consumer"
},
"devDependencies": {
"@eslint/eslintrc": "^3.3.1",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 개선 제안

  1. 중복 제거: Sentry sourcemaps 관련 명령어들이 여러 번 반복되고 있습니다. 각 환경별로 유사한 형식을 사용하고 있으므로, 이를 함수로 추출해 중복을 줄일 수 있습니다.

  2. 에러 관리: 각 명령어가 성공적으로 실행되지 않을 경우에 대한 처리가 없습니다. && 연산자를 사용하는 대신, 각 단계에서 에러가 발생했을 경우 로그를 남기거나 대체 작업을 수행할 수 있는 방법을 구현하는 것이 좋습니다.

  3. 유지보수성 향상: $NODE_ENV를 사용하여 환경에 맞는 구성을 관리하는 방식이 좋지만, 에러 발생 시 구체적인 로그를 남길 필요가 있습니다. 예를 들어, sentry-cli에 의해 반환된 에러 메시지를 로깅하는 코드를 추가할 수 있습니다.

  4. 패키지 버전 조건: $(node -p "require('./package.json').version")을 사용할 경우, 현재 패키지 JSON 파일을 읽어야 하고 이에 대한 종속성이 필요합니다. 이러한 사항을 명확히 문서화하고, 필요 시 버전 파일의 확인 절차를 추가하는 것이 좋습니다.

  5. 코드 가독성: 명령어들이 길어지면서 가독성이 떨어지고 있습니다. 각 명령어를 설명하는 주석을 추가하여 나중에 이해하기 쉽게 만드는 방법을 고려해 보세요.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review by ChatGPT

import { SentryModule } from '@sentry/nestjs/setup'
import { ClsModule } from 'nestjs-cls'

import { PrismaModule, PrismaService } from '@otl/prisma-client'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SentryModule을 추가하면서 초기 설정값이 필요할 수 있습니다. SentryModule.forRoot() 메서드에 적절한 설정을 전달하는 것이 중요합니다. 그렇지 않으면 Sentry가 올바르게 초기화되지 않을 수 있습니다. 따라서 Sentry의 초기화 설정이 없거나 비어 있다면, 애플리케이션의 에러 로깅 및 모니터링 기능이 제대로 동작하지 않을 위험이 있습니다.


import { NestFactory } from '@nestjs/core'
import settings from '@otl/notification-consumer/settings'
import * as admin from 'firebase-admin'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 변경 사항:

  • import './instrument'를 추가했습니다. 이 변경 사항은 Sentry 계측 데이터를 조기에 가져오기 위해 필요한 것으로 보입니다.

문제점:

  • 상대 경로로 Sentry를 가져오는 것이므로, 이 파일이 다른 경로에서 호출될 경우 경로 문제로 오류가 발생할 수 있습니다. 절대 경로 사용 또는 tsconfig.json 설정을 통해 컴파일러의 경로 별칭을 활용하는 것이 좋습니다.

개선 제안:

  • instrument 파일의 존재 여부를 확인하고, 필요한 경우 이 파일이 있는 위치에 대한 검증 또는 확인을 추가하세요. 이를 통해 런타임 오류를 예방할 수 있습니다.

// await appContext.startAllMicroservices()
console.log('start')
const serviceAccount: ServiceAccount = settings().getFirebaseConfig()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 변경 사항:

  • await appContext.startAllMicroservices() 줄이 주석 처리되었습니다. 이 변경은 마이크로서비스 시작 방식을 변경할 수 있습니다.

문제점:

  • 주석 처리된 코드는 마이크로서비스가 시작되지 않게 되어 의도치 않은 오류를 발생시킬 수 있습니다. 마이크로서비스가 필요하다면 이 줄의 주석을 해제해야 합니다. 또한 이 코드에 대한 의존성이 있을 가능성이 있습니다.

개선 제안:

  • 마이크로서비스가 정말 필요 없는 경우, 이 변경이 의도된 것인지 확인할 필요가 있습니다. 필요하다면, 관련 문서나 주석을 추가하여 이 주석 처리의 이유를 명확히 해야 합니다.

],

sendDefaultPii: true,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드는 Sentry 초기화를 위한 것입니다. 그러나 몇 가지 잠재적인 문제가 있습니다:

  1. 환경 변수 확인: process.env.NODE_ENV가 'prod'로 설정되어 있지 않으면 다른 설정을 사용합니다. 하지만 이를 명확하게 다루는 추가적인 검증이 필요할 수 있습니다. 잘못된 환경에서 코드를 실행하게 될 경우 예기치 않은 결과가 발생할 수 있습니다.

  2. 성능 모니터링 비율: tracesSampleRateprofilesSampleRate의 값을 하드코딩하는 대신 환경 변수에서 직접 설정할 수 있도록 해주는 것이 좋습니다. 이는 더 큰 유연성을 제공합니다.

  3. 예외 처리: Sentry 초기화 중 오류가 발생할 경우 적절한 예외 처리가 없습니다. 문제 발생 시 시스템이 어떻게 반응할지 정의할 필요가 있습니다.

  4. PII 전송: sendDefaultPii를 true로 설정한 것은 개인 식별 정보를 전송하게 합니다. 이는 개인정보 보호 관련 법률을 위반할 수 있으므로 사용을 신중히 고려해야 합니다.

이러한 문제를 해결하면 코드의 안정성과 가독성이 크게 향상될 것입니다.


const getVersion = () => String(process.env.npm_package_version)

const getRedisConfig = (): RedisModuleOptions => ({
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sentry DSN 값을 process.env에서 가져오고 있지만, 이 값이 정의되어 있는지 확인하는 방법이 없습니다. 만약 정의되어 있지 않다면, 애플리케이션에서 실행 시 오류가 발생할 수 있습니다. 이를 방지하기 위해 기본 값을 설정하거나, 환경 변수가 제공되지 않을 경우 오류를 발생시키도록 개선할 수 있습니다.

],

sendDefaultPii: true,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 새로 추가된 코드 조각은 전반적으로 Sentry 초기화와 관련된 설정을 포함하고 있습니다. 그러나 몇 가지 주의해야 할 점이 있습니다:

  1. 민감한 정보 노출: sendDefaultPii를 true로 설정하면 개인 식별 정보(PII)가 Sentry로 전송됩니다. 이는 개인 정보 보호 규정에 위배될 수 있으므로, 꼭 필요한 경우에만 true로 설정하는 것이 좋습니다.

  2. 환경 변수 사용: 환경 변수를 사용할 때는 해당 변수가 정의되어 있는지 확인하는 로직을 추가하여, 정의되지 않은 경우 기본값을 설정하거나 오류를 발생시키는 것이 좋습니다. 예를 들어, process.env.NODE_ENV가 정의되어 있지 않을 경우에 대한 예외 처리가 필요합니다.

  3. 윗줄 주석 추가: 각 설정 항목에 대해 주석을 추가하여 어떤 목적으로 사용되는지, 특히 tracesSampleRateprofilesSampleRate의 설정 값이 어떤 의미를 가지는지 명확히 할 필요가 있습니다.

  4. 에러 핸들링: Sentry 초기화가 실패할 경우에 대한 에러 핸들링이 전혀 없습니다. 초기화 실패 시를 대비하여 적절한 예외 처리를 추가해야 합니다.

import * as Sentry from '@sentry/nestjs'
import cookieParser from 'cookie-parser'
import csrf from 'csurf'
import { json } from 'express'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Sentry instrumentation import is necessary for error tracking and should be done at the beginning of the file, which you addressed correctly. However, make sure that the Sentry import statement does not get removed later.\n- Ensure that there are no circular dependencies introduced by importing 'instrument' at this point. Circular dependencies can lead to unexpected behavior.\n- Consider adding error handling mechanisms when initializing Sentry to prevent the application from crashing if Sentry import fails.


const app = await NestFactory.create(AppModule)

app.enableVersioning({
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The initialization of Sentry has been commented out. If Sentry is necessary for error tracking in production, it should be properly initialized and not commented out. \n- If you plan to re-enable Sentry initialization, ensure the configuration (DSN, tracesSampleRate, etc.) is correctly managed and set up according to the environment (development, staging, production). \n- It is also advisable to review and implement logging or error handling for the Sentry initialization to catch potential issues.


// Attach request headers/cookies/body to error events for debugging
sendDefaultPii: true,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Sentry의 초기화를 위한 파일이므로, 다른 모듈보다 먼저 가져와야 하는 것은 명확한 지침이지만, 호출 순서가 잘못될 경우 전체 애플리케이션의 에러 보고 기능이 작동하지 않을 수 있습니다. 이 부분을 문서에 명시하여 개발자들이 주의할 수 있도록 하는 것이 좋습니다.

  • 환경변수 NODE_ENV에 대해 'prod'와 'local'의 형태로 분기하는 것은 일반적이지만, 운영 환경에 대한 체크가 완벽하지 않을 수 있습니다. 추가적인 환경(예: 'staging')을 고려해야 할 수 있습니다.

  • dsnrelease에 대한 설정 값이 누락 또는 잘못된 경우 초기화 실패할 수 있습니다. 이러한 경우에는 적절한 에러 처리를 추가하여 원인을 쉽게 추적할 수 있도록 하는 것이 좋습니다.

  • tracesSampleRateprofilesSampleRate의 비율이 운영 환경에 따라 다르게 설정되었는데, 이러한 비율의 결정 이유를 코드 주석으로 명확히 해두는 것이 좋습니다. 각 비율의 선택이 어떤 기준에 기반하여 이루어졌는지를 설명하는 것이 바람직합니다.

  • sendDefaultPiitrue로 설정된 경우, 개인 식별 정보(PII)가 Sentry로 전송되므로 해당 설정은 보안 정책에 부합하는지 확인해야 합니다. 이와 관련된 문서나 정책을 포함시키는 것이 필요합니다.

Comment thread package.json
"@sentry/profiling-node": "^9.17.0",
"@types/cookie": "^1.0.0",
"@types/express": "^5.0.0",
"@types/inquirer": "^9.0.7",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

새롭게 추가된 의존성 @sentry/profiling-node의 버전이 ^9.17.0인데, 이 버전이 기존 프로젝트의 다른 의존성과 호환되는지 확인할 필요가 있습니다. 만약 호환되지 않는다면, 런타임 오류가 발생할 수 있습니다. 또한, 이 의존성이 프로젝트에 필요한 것인지 여부를 명확히 하고 문서화 하는 것이 좋습니다.

…x lint

- Remove @sentry/profiling-node (causes @sentry/core type duplication with @sentry/nestjs)
- Remove nodeProfilingIntegration() from all instrument.ts files
- Add eslint-disable-next-line for import order (instrument must load first)
- Keep prismaIntegration, environment, release, and sample rate configs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant