[REFACTOR] 인프라 구조를 Docker로 전환 #237
Conversation
Walkthrough모니터링 의존성을 Prometheus에서 StatsD로 교체하고, 다단계 Docker 빌드 및 Datadog/Traefik 기반의 컨테이너 오케스트레이션 구성과 초기화 스크립트(스왑, Docker 설치, LetsEncrypt)를 추가했습니다. JAR 태스크 비활성화와 StatsD 설정 파일도 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer / CI
participant Builder as Docker Builder (jarmode=layertools)
participant Image as Final Image (with dd-java-agent)
participant Compose as Docker Compose (traefik, datadog, app)
participant Traefik as Traefik
participant App as Spring App (JarLauncher)
participant Datadog as Datadog Agent (StatsD UDP)
Dev->>Builder: 제공된 JAR_FILE 복사 및 레이어 추출
Builder->>Image: 복사(dependencies, spring-boot-loader, application)
Image->>Image: dd-java-agent 다운로드 및 이미지 구성
Dev->>Compose: docker-compose up (traefik, datadog, app-blue/green)
Compose->>Datadog: datadog-agent 컨테이너 실행 및 포트(8125,8126) 노출
Compose->>Traefik: traefik 컨테이너 실행 (ACME/라우팅)
Compose->>App: app 컨테이너 시작 (JarLauncher, javaagent 활성화)
App->>Datadog: StatsD/UDP로 메트릭 전송
Traefik->>App: 클라이언트 요청 라우팅
Datadog->>Dev: APM/메트릭/로그 집계
(컴포넌트 박스 색상은 기본 다이어그램 스타일을 사용합니다.) 예상 검토 난이도🎯 3 (Moderate) | ⏱️ ~25 minutes 제안하는 레이블
제안하는 검토자
개요프로젝트에 모니터링 및 인프라 자동화를 위한 여러 구성 요소가 추가되었습니다. Prometheus 모니터링 의존성을 StatsD로 변경하고, Docker 다단계 빌드, Datadog 통합, Traefik 라우팅, 시스템 초기화 스크립트가 도입되었습니다. 변경 사항
제안하는 검토자
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Summary of ChangesHello @leegwichan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 서버 인프라의 초기 설정 및 모니터링 시스템 통합을 목표로 합니다. Docker와 Docker Compose를 활용하여 애플리케이션(WAS), 리버스 프록시(Traefik), 모니터링 에이전트(Datadog Agent)를 컨테이너화하고 오케스트레이션합니다. 또한, 서버 초기 환경 설정을 자동화하는 스크립트와 Datadog 기반의 메트릭 및 추적 시스템을 구축하여 애플리케이션의 가시성을 확보하고 안정적인 운영 환경을 마련하는 데 중점을 둡니다. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 애플리케이션의 Docker 환경을 설정하는 중요한 리팩토링을 수행합니다. Traefik을 이용한 리버스 프록시, Datadog 연동을 통한 모니터링, 그리고 Spring 애플리케이션을 위한 블루-그린 배포 구조를 도입하는 등, 전반적으로 잘 구성되었습니다. 멀티 스테이지 Docker 빌드를 사용하고 docker-compose 파일을 모듈화한 점이 좋습니다.
하지만 몇 가지 개선이 필요한 부분이 있습니다. 특히, healthcheck 설정에 오류가 있어 컨테이너가 정상적으로 실행되지 않을 수 있는 치명적인 문제가 있습니다. 또한 Traefik 설정에 민감 정보가 하드코딩된 보안 이슈와, docker-compose.spring.yaml 파일 내에 중복 코드가 많아 유지보수성을 저해할 수 있는 부분이 발견되었습니다.
제 리뷰는 이러한 문제들을 해결하고 설정의 안정성과 품질을 높이기 위한 구체적인 제안들을 담고 있습니다.
| - "--entrypoints.websecure.address=:443" | ||
|
|
||
| - "--certificatesresolvers.myresolver.acme.tlschallenge=true" | ||
| - "--certificatesresolvers.myresolver.acme.email=debatetimekeeping@gmail.com" |
There was a problem hiding this comment.
Let's Encrypt 인증서 발급에 사용되는 이메일 주소가 코드에 하드코딩되어 있습니다. 이메일 주소는 민감 정보로 취급될 수 있으므로, 환경 변수로 분리하여 관리하는 것이 안전합니다.
- "--certificatesresolvers.myresolver.acme.email=${ACME_EMAIL}"There was a problem hiding this comment.
내 이메일 주소 ㅠㅠ
Certbot 없이 자동으로 발급받고 관리해주네... 미쳤다.. 확실히 편한듯
There was a problem hiding this comment.
이거 노출되고 크게 문제 없는 정보니까, 이렇게 그대로 해도 되겠죠?
-> 그대로 가도 괜찮을듯 ㅇㅇ
Test Results135 files 135 suites 14s ⏱️ Results for commit 8de6dbf. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@docker/docker-compose.spring.yaml`:
- Line 27: Both app-blue and app-green are routed to the same
Host(`api.test.debate-timer.com`) via traefik (see
traefik.http.routers.app-blue.rule and traefik.http.routers.app-green.rule),
which creates a routing conflict; fix it by giving the routers deterministic
precedence or ensuring only one is active: add distinct priority labels
(traefik.http.routers.app-blue.priority and
traefik.http.routers.app-green.priority) with higher value for the intended
active router, or implement an activation script to enable/disable the unused
service during deploy, or switch to a weighted load strategy via Traefik
service/weight labels so Traefik can deterministically choose between app-blue
and app-green.
- Around line 77-84: The Traefik router for app-green is missing an explicit
service assignment causing inconsistency with app-blue; add the label
"traefik.http.routers.app-green.service=app-green" to the app-green service
labels (matching the service name used elsewhere) so the router explicitly
targets the app-green service, keep other labels (rule, port, entrypoints, tls,
certresolver) unchanged.
In `@docker/docker-compose.traefik.yaml`:
- Around line 13-14: Remove the duplicated Traefik flag "--api.insecure=true"
and ensure the dashboard is not exposed insecurely in production by either
removing the flag entirely or replacing it with a secure alternative (e.g.,
enable dashboard with proper authentication/entrypoints) so that
"--api.insecure=true" is not present twice; also change the "--log.level=DEBUG"
flag to a less verbose production-appropriate level such as "--log.level=INFO"
or make the log level configurable via environment/compose variable to avoid
excessive logging in production.
In `@docker/Dockerfile`:
- Line 10: The Dockerfile currently uses ADD with a shortened URL to fetch
dd-java-agent.jar, which lacks version pinning and checksum verification; update
the Dockerfile to set the Dockerfile syntax to 1.6 or newer, introduce an ARG
like DD_AGENT_VERSION and use the full GitHub release URL for
dd-java-agent-${DD_AGENT_VERSION}.jar, and add the ADD
--checksum=sha256:<expected-hash> ... ./dd-java-agent.jar flag to verify
integrity (replace <expected-hash> with the SHA256 from the GitHub release) so
builds are reproducible and secure.
In `@scripts/init/init-docker.sh`:
- Around line 33-39: The pipeline exit status check can miss docker login
failures because echo | sg docker -c "docker login..." leaves the pipe outside
sg; fix by either enabling pipefail before the pipeline (add set -o pipefail so
the pipeline returns a non-zero if any component fails) or by moving the token
pipe inside the sg invocation so docker login runs with the stdin inside the
same shell (e.g. run sg docker -c "printf '%s' \"$DOCKER_TOKEN\" | docker login
-u $DOCKER_USER --password-stdin") and then check the command directly (if ...;
then ...; else ...; fi) instead of inspecting $?. Ensure you update the block
that uses DOCKER_TOKEN, sg docker -c and docker login to use one of these
approaches so login failures are detected reliably.
In `@scripts/init/init-letsencrypt.sh`:
- Line 3: The initial directory change using cd "$(dirname "$0")" must handle
failure: if the cd command fails, log an error and exit the script instead of
proceeding; update the script around the cd invocation so that the shell checks
the cd exit status (for the cd "$(dirname "$0")" command) and calls a meaningful
error message and exit (non-zero) when it fails to prevent creating the
letsencrypt folder in the wrong location.
In `@src/main/resources/application-monitor.yml`:
- Around line 2-9: The environment variable name used for statsd host is
incorrect for Spring Boot 3.4; update the placeholder for the YAML key
statsd.metrics.export.host so it uses MANAGEMENT_STATSD_METRICS_EXPORT_HOST (not
MANAGEMENT_METRICS_EXPORT_STATSD_HOST) to match Spring Boot 3.4 naming
conventions and ensure the host value is picked up correctly.
🧹 Nitpick comments (5)
scripts/init/auto-swap.sh (1)
24-34: 변수 사용 시 따옴표 권장변수를 사용할 때 따옴표로 감싸는 것이 쉘 스크립트 모범 사례입니다. 경로에 공백이 포함될 가능성은 낮지만, 안전한 코딩 습관을 위해 권장됩니다.
♻️ 수정 제안
-fallocate -l $SWAP_SIZE $SWAP_FILE +fallocate -l "$SWAP_SIZE" "$SWAP_FILE" echo " -> 파일 생성 완료" # 4. 권한 설정 (600) -chmod 600 $SWAP_FILE +chmod 600 "$SWAP_FILE" echo " -> 권한 설정 완료" # 5. Swap 활성화 -mkswap $SWAP_FILE > /dev/null -swapon $SWAP_FILE +mkswap "$SWAP_FILE" > /dev/null +swapon "$SWAP_FILE" echo " -> Swap 활성화 완료"scripts/init/init-docker.sh (1)
28-28: 변수 따옴표 처리 권장
$USER변수를 따옴표로 감싸는 것이 좋습니다.♻️ 수정 제안
-sudo usermod -aG docker $USER +sudo usermod -aG docker "$USER"docker/docker-compose.datadog.yaml (1)
16-18: 포트 노출 범위 검토 권장StatsD(8125)와 APM(8126) 포트가 호스트에 직접 노출되어 있습니다. 내부 네트워크(
debate-timer-net)를 통해서만 통신이 필요하다면, 포트 매핑을 제거하고 컨테이너 간 통신만 사용하는 것이 더 안전합니다.🔒 선택적 수정: 포트 노출 제거
- ports: - - "8125:8125/udp" # Metrics (StatsD) - - "8126:8126/tcp" # APM (Trace) + # ports are accessible via debate-timer-net network + expose: + - "8125/udp" + - "8126/tcp"docker/docker-compose.spring.yaml (2)
13-13: DD_VERSION 하드코딩 문제
DD_VERSION=1.0.0이 하드코딩되어 있습니다. 이미지 태그(${TAG})를 사용하거나 별도의 버전 변수를 사용하는 것이 버전 추적에 더 정확합니다.♻️ 제안된 수정
- - DD_VERSION=1.0.0 + - DD_VERSION=${TAG}Also applies to: 65-65
2-52: app-blue와 app-green 간 코드 중복두 서비스의 설정이 거의 동일합니다. YAML 앵커(&)와 별칭(*)을 사용하거나
x-extension fields를 활용하여 중복을 줄일 수 있습니다.♻️ YAML 앵커를 사용한 리팩토링 예시
x-app-common: &app-common image: debatetimer/debate_timer:${TAG} environment: &app-env - SERVER_FORWARD_HEADERS_STRATEGY=framework - SPRING_PROFILES_ACTIVE=${PROFILE:-dev,monitor} - TZ=Asia/Seoul - DD_AGENT_HOST=datadog-agent - DD_SERVICE=debate-timer - DD_ENV=${TAG} - DD_VERSION=${TAG} - DD_LOGS_INJECTION=true - DD_PROFILING_ENABLED=true - DD_PROFILING_ALLOCATION_ENABLED=true - DD_PROFILING_HEAP_ENABLED=true - MANAGEMENT_METRICS_EXPORT_STATSD_ENABLED=true - MANAGEMENT_METRICS_EXPORT_STATSD_FLAVOR=datadog - MANAGEMENT_METRICS_EXPORT_STATSD_HOST=datadog-agent - MANAGEMENT_METRICS_EXPORT_STATSD_PORT=8125 - MANAGEMENT_METRICS_EXPORT_STATSD_PROTOCOL=UDP networks: - debate-timer-net depends_on: traefik: condition: service_healthy datadog-agent: condition: service_healthy healthcheck: &app-healthcheck test: [ "CMD", "wget", "--no-verbose", "--tries=1", "--spider", "http://localhost:8083/monitoring/health" ] interval: 10s retries: 3 start_period: 300s services: app-blue: <<: *app-common container_name: app-blue labels: # app-blue specific labels... app-green: <<: *app-common container_name: app-green labels: # app-green specific labels...Also applies to: 54-103
docker/docker-compose.traefik.yaml
Outdated
| - "--api.insecure=true" | ||
| - "--log.level=DEBUG" |
There was a problem hiding this comment.
--api.insecure=true 중복 및 보안 문제
- Line 13과 Line 29에
--api.insecure=true가 중복 선언되어 있습니다. api.insecure=true는 Traefik 대시보드를 인증 없이 포트 8080에 노출합니다. 프로덕션 환경에서는 보안 위험이 될 수 있습니다.log.level=DEBUG는 프로덕션에서 과도한 로그를 생성하여 성능에 영향을 줄 수 있습니다.
🔒 제안된 수정 사항
command:
- - "--api.insecure=true"
- - "--log.level=DEBUG"
+ - "--api.insecure=false"
+ - "--log.level=INFO"
- "--providers.docker=true"
- "--providers.docker.exposedbydefault=false"
...
- "--entrypoints.monitoring.address=:8083"
- - "--api.insecure=true"
- "--ping=true"프로덕션 배포 전에 API 보안 설정과 로그 레벨을 검토하시기 바랍니다.
Also applies to: 29-29
🤖 Prompt for AI Agents
In `@docker/docker-compose.traefik.yaml` around lines 13 - 14, Remove the
duplicated Traefik flag "--api.insecure=true" and ensure the dashboard is not
exposed insecurely in production by either removing the flag entirely or
replacing it with a secure alternative (e.g., enable dashboard with proper
authentication/entrypoints) so that "--api.insecure=true" is not present twice;
also change the "--log.level=DEBUG" flag to a less verbose
production-appropriate level such as "--log.level=INFO" or make the log level
configurable via environment/compose variable to avoid excessive logging in
production.
There was a problem hiding this comment.
중복 문제 반영 완료
8080 포트 문제는 논의 필요
예시 : 현재 Traefik 대시보드는 8080 포트로 열려 있음 - http://api.test.debate-timer.com:8080/dashboard/#/
There was a problem hiding this comment.
@leegwichan, 중복 문제 수정해 주셔서 감사합니다! 👍
8080 포트 대시보드 노출 관련해서는 다음과 같이 권장드립니다:
테스트 환경 (현재)
- 테스트 환경에서는
--api.insecure=true로 대시보드를 사용하는 것이 개발/디버깅 목적으로 허용될 수 있습니다. - 다만, 다음 중 하나를 고려하시는 것이 좋습니다:
- 방화벽/보안 그룹 규칙으로 8080 포트 접근을 특정 IP로 제한
- VPN이나 사설 네트워크 내에서만 접근 가능하도록 설정
프로덕션 환경 (향후 배포 시)
--api.insecure=true제거- BasicAuth나 다른 인증 메커니즘 적용
- 또는 대시보드를 완전히 비활성화
현재 테스트 환경이므로 네트워크 레벨에서 접근이 제한되어 있다면 현재 설정으로도 괜찮습니다. 다만, PROD 환경 구축 시에는 보안 설정을 강화해 주시기 바랍니다.
/>/)_/)
( •.• )
/>📋 Test 환경 확인 완료!
✏️ Learnings added
Learnt from: leegwichan
Repo: debate-timer/debate-timer-be PR: 237
File: docker/docker-compose.traefik.yaml:13-15
Timestamp: 2026-02-05T12:48:49.989Z
Learning: In the debate-timer project, docker/docker-compose.traefik.yaml configures Traefik for a TEST environment (api.test.debate-timer.com). The `--api.insecure=true` flag exposing the dashboard on port 8080 is acceptable for the test environment, but should be secured with authentication or disabled when deploying to production.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: leegwichan
Repo: debate-timer/debate-timer-be PR: 237
File: docker/docker-compose.spring.yaml:27-27
Timestamp: 2026-02-05T11:51:25.354Z
Learning: In the debate-timer project's docker/docker-compose.spring.yaml, both app-blue and app-green services are intentionally configured to route to the same Traefik host (api.test.debate-timer.com) for blue-green deployment. When both services are running simultaneously, Traefik splits traffic 50/50 between them without conflicts. This is the intended deployment strategy.
| FROM eclipse-temurin:17-jre-alpine | ||
| WORKDIR /workspace | ||
|
|
||
| ADD https://dtdg.co/latest-java-tracer ./dd-java-agent.jar |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Datadog dd-java-agent latest releases 2025 GitHub
💡 Result:
Latest Datadog dd-java-agent releases in 2025 (as reflected by the DataDog/dd-trace-java GitHub release tags and Maven Central publish dates):
- v1.56.1 — Nov 25, 2025 (latest in 2025) [1] [2]
- v1.55.0 — Nov 04, 2025 [1] [3]
- v1.53.0 — Sep 04, 2025 [1] [4]
- v1.52.1 — Aug 14, 2025 [1] [5]
- v1.52.0 — Aug 04, 2025 [1] [6]
- v1.51.1 — Jul 15, 2025 [1] [7]
- v1.50.0 — Jun 20, 2025 [1] [8]
- v1.48.2 — Apr 23, 2025 [1] [9]
- v1.48.0 — Apr 09, 2025 [1] [10]
Sources: GitHub releases list (DataDog/dd-trace-java) [1] plus per-version Maven Central publish dates via MvnRepository [2]-[10].
🌐 Web query:
Docker ADD checksum verification best practices
💡 Result:
Best practices for checksum verification with ADD
-
Prefer
COPYfor local files; useADDmainly for remote/Git sources. Docker recommendsCOPYwhen bringing files from the build context, and usingADDwhen you intentionally need remote downloads (and want better caching + built-in checksum validation). [2] -
Use
ADD --checksumfor remote artifacts (HTTP) instead ofcurl/wget | sha256suminRUN.- For HTTP(S) sources,
--checksumverifies a SHA-256 digest in the formsha256:<hash>(SHA-256 is the only supported algorithm). [1]
Example:
# syntax=docker/dockerfile:1.6 ADD --checksum=sha256:<expected_sha256> https://example.com/tool.tgz /tmp/tool.tgz
This makes integrity checking part of the Dockerfile instruction and improves cache correctness versus ad-hoc
RUNscripts. [2] - For HTTP(S) sources,
-
For Git sources, pin by commit and verify with
--checksum(commit SHA).
With BuildKit’s Git URL support,ADD --checksum=<commit_sha>lets you verify the fetched repo state (checksum is the commit SHA; prefix matching is allowed). [1]
Example:# syntax=docker/dockerfile:1.6 ADD --checksum=<commit_sha> https://github.com/org/repo.git#<ref> /src/repo
-
Pin the Dockerfile syntax version to ensure
--checksumis available and stable.
ADD --checksumrequires Dockerfile syntax 1.6+. Pinning avoids surprises from syntax feature changes. [1] -
Combine with multi-stage builds to avoid leaving downloaded installers/tarballs in the final image.
Docker’s best-practices show downloading withADD --checksumin a source stage, then extracting/copying only what you need into the final stage. [2]
Notes / gotchas
ADD --checksumis specifically designed for remote HTTP and Git resources in the modern Dockerfile reference. [1]ADDhas other remote-related behavior (auto-unpacking local tar, Git URL fragments, etc.); use it intentionally to avoid unexpected results. [1]
Sources: Dockerfile ADD --checksum reference and examples [1], Docker Dockerfile best practices guidance and sample pattern [2], Dockerfile release notes (feature graduation around Dockerfile 1.6 / BuildKit) [3].
🏁 Script executed:
#!/bin/bash
# Check if there's any reference to Datadog agent versions in the codebase
cd /tmp && git clone --depth 1 https://github.com/DataDog/dd-trace-java.git 2>/dev/null || true
ls -la dd-trace-java/releases 2>/dev/null | head -20 || echo "Check via web for releases"Repository: debate-timer/debate-timer-be
Length of output: 54
원격 URL에서 에이전트 다운로드 시 보안 및 재현성 개선 필요
ADD로 단축 URL(dtdg.co)에서 직접 다운로드하면 다음 문제가 발생합니다:
- 체크섬 검증 없음 - 무결성 보장 불가
- 버전 고정 없음 - 빌드 재현성 없음
- 단축 URL 사용 - 리다이렉트 대상 변경 시 예측 불가능
♻️ 개선 제안
특정 버전을 명시하고 체크섬 검증을 추가하세요:
# syntax=docker/dockerfile:1.6
ARG DD_AGENT_VERSION=1.56.1
ADD --checksum=sha256:<hash> https://github.com/DataDog/dd-trace-java/releases/download/v${DD_AGENT_VERSION}/dd-java-agent-${DD_AGENT_VERSION}.jar ./dd-java-agent.jarGitHub 릴리스에서 각 JAR의 SHA256 해시를 확인하여 <hash>를 치환하세요. --checksum을 사용하려면 Dockerfile 문법 버전을 1.6 이상으로 설정해야 합니다.
🤖 Prompt for AI Agents
In `@docker/Dockerfile` at line 10, The Dockerfile currently uses ADD with a
shortened URL to fetch dd-java-agent.jar, which lacks version pinning and
checksum verification; update the Dockerfile to set the Dockerfile syntax to 1.6
or newer, introduce an ARG like DD_AGENT_VERSION and use the full GitHub release
URL for dd-java-agent-${DD_AGENT_VERSION}.jar, and add the ADD
--checksum=sha256:<expected-hash> ... ./dd-java-agent.jar flag to verify
integrity (replace <expected-hash> with the SHA256 from the GitHub release) so
builds are reproducible and secure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docker/docker-compose.spring.yaml`:
- Around line 32-36: The healthcheck uses wget but the base image
(eclipse-temurin:17-rre-alpine) does not include it, causing `wget: not found`;
fix by either installing the tool or switching to an available one: update the
Dockerfile to install wget (e.g., add an apk add --no-cache wget) if you want to
keep the current healthcheck, or change the compose healthcheck test to use curl
(and add apk add --no-cache curl in the Dockerfile) or replace the healthcheck
command with a Java-based HTTP probe; target the healthcheck block in
docker-compose.spring.yaml and the project's Dockerfile to apply the matching
installation or command change so the test ("CMD", "wget", ...) will succeed.
🧹 Nitpick comments (2)
docker/docker-compose.spring.yaml (2)
11-11:DD_VERSION하드코딩 문제버전이
1.0.0으로 하드코딩되어 있습니다. 애플리케이션 버전이 변경될 때마다 이 값을 수동으로 업데이트해야 하며, Datadog에서 버전별 추적이 부정확해질 수 있습니다.♻️ 환경 변수로 분리 제안
- - DD_VERSION=1.0.0 + - DD_VERSION=${APP_VERSION:-1.0.0}
51-54: 모니터링 라우터의 PathPrefix가 너무 광범위함
PathPrefix(\/`)`는 모든 경로를 매칭합니다. 모니터링 엔드포인트에서 의도치 않은 경로가 노출될 수 있습니다.♻️ 더 구체적인 경로 제한 제안
- - "traefik.http.routers.app-blue-monitor.rule=PathPrefix(`/`)" + - "traefik.http.routers.app-blue-monitor.rule=PathPrefix(`/monitoring`) || PathPrefix(`/actuator`)"
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
/noti
@leegwichan 간단한 리뷰 2개 남기고 approve합니다
There was a problem hiding this comment.
단순 질문입니다.
- 이렇게 각 컴포넌트별로 yaml을 따로 만든 이유가 있나요?
2.확장자를 yaml로 한 이유가 있나요?
There was a problem hiding this comment.
- 개인적으로는 각 컴포넌트 성격이 다르기 떄문에 분리되어 있어 읽기는 좋았습니다. 수정할 포인트도 더 잘 확인할 수 있을 것 같다는 생각도 들었구요.
- 확장자는 저도 궁금하네요
There was a problem hiding this comment.
yml 하고 yaml 하고 차이 없어서 아무거나 사용했습니다.
Spring에서 appliction.yml 파일 이용하고 있으니까, yml로 통일할께요~
There was a problem hiding this comment.
이렇게 각 컴포넌트별로 yaml을 따로 만든 이유가 있나요?
- 각 컴포넌트 별로 파일을 분리해서, 나중에 작업할 때 용이하게 하기 위해서 작업함
- 일종의 facade 느낌으로 각 컴포넌트를 분리함
비토 의견
- 코드 결합도 측면에서 너무 과한 분리라고 느껴진다. (스프링 yml에서 depends on으로 다른 컴포넌트도 있는데 한 파일에서 보는 게 더 편하지않나?)
- 과연 어떤 컴포넌트가 수정됐는지 쉽게 파악하는게 중요한가? 그만큼 빈번하게 바뀔 요소인가?
- 컴포넌트의 수정사항은 커밋 메시지나 git의 diff만으로 충분히 파악 가능하지 않나?
- 결국 단일 파일로 실행할 가능성이 얼마나 될까? 하나의 파일로 하면 알아서 이미 띄어져있는 것은 스킵하고 띄울 거고 왠만해선 전체로 돌릴텐데(spring단일로 실행했다가 나머지 의존성 없으면 또 나머지 돌리는 것보단 첨부터 전체가 편할테니)
- -> 네트워크 설정 때문에 단일로 실행이 안됨
비토/커찬 논의 결론
- 둘 다 효용성에 있어 크게 차이가 없다고 판단함
- 비토 : facade 형식이라면, 각각의 컴포넌트가 docker-compose.yml 파일보다는 하위 폴더에 있었으면 좋겠다. (필수는 아님)
- 커찬 : 저기에 Dockerfile도 같이 있어서 하위 폴더가 들어가면 조금 애매해질 것 같다.
There was a problem hiding this comment.
개인의견)
이건 진짜 개인 취향 문제라 생각하긴 하는디... 나는 개인적으로 각 docker compose의 목적이 다르기 때문에 모듈화해두는게 좋다고 생각하기는 하는 것 같아. 단일 파일로 실행할 가능성은 거의 없지만 커밋 내역이나 diff에서도 어떤 파일이 수정되었는지를 기반으로 chnage 추적하기 더 쉬울 것 같기도 함(대충 이 compose 수정되었으면 ~~ 컴포넌트에 수정 있었겠구나를 들어가보지 않고 일단 인지하게 되니까)
또 application-local, application-dev 처럼 하나의 app 컴포넌트 내에서 프로파일이나 성격별로 분리하는게 아니라 각자 별도의 역할을 가진 컴포넌트라는 점에서 분리는 지향되어야 한다고 생각함.
- traefix : 웹 서버
- app : 우리가 띄우는 스프링 서버
- datadog-agent : 모니터링 툴
물론 프로젝트 진행 속도나 인프라 변경이 잦지 않다는 특성을 고려하면 한파일 내에서 모두 모아두어도 추후 유지보수에는 큰 문제는 없을 것 같긴 해서 이건 강하게 주장 못하겠음..
하위폴더:
이건 나도 비토 의견처럼 계층화해두는게 더 좋다고 생각은 하는데 커찬이이야기하는 Dockerfile이 있어서 생기는 하위폴더의 애매모호함이 정확이 어떤 의미인지 캐치를 못해서 한번더 물어보고 싶기도 함..
There was a problem hiding this comment.
만약에 docker-compose랑 그 하위 파일들까지 전부 나눠서 관리 할 것이라면, Dockerfile 도 다른 폴더로 나눠서 관리하는게 좀 더 통일될 것 같다는 생각이 듦
한 폴더 내에서 관리할 거라면 왠만하면 다 같이 관리하고 폴더를 나눌꺼면 확실하게 나눠 나가는 것을 선호하는 편이라서, 서브 파일들은 하위 폴더로 내리고 또 다른 성격의 파일은 위에서 관리하는게 어색하다고 생각했음
| # --- 설정 변수 --- | ||
| SWAP_FILE="/swapfile" | ||
| SWAP_SIZE="2G" | ||
| FSTAB_FILE="/etc/fstab" |
There was a problem hiding this comment.
도커 설치하는 스크립트처럼 최소한 SWAP_SIZE 만이라도 입력으로 받는 건 어떤가요?
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
@leegwichan 고생 많았습니다 커찬 ㅠㅠ 그리고 리뷰가 늦어서 미안해유
traefik 미쳤네요. https 자동관리부터 코드로 스크립트 관리하니까 확실히 기본 세팅 속도가 차원이 다를듯...
몇가지 궁금한점 남겼는데요. approve 하면 제가 확인이 늦어질까봐 일단 RC남겨둡니당.
| - DD_PROCESS_AGENT_ENABLED=true | ||
| - DD_TAGS=env:${ENV:-dev},project:debate-timer | ||
| ports: | ||
| - "8125:8125/udp" # Metrics (StatsD) |
There was a problem hiding this comment.
일단 데이터독 docker compose 이미지로 만드니까 확실히 문서화 효과가 있어서 좋은 것 같아유.
그런데 데이터 독 배경지식이 없어서 그런데 메트릭은 udp, apm은 tcp로 소통하는 이유가 있나요? 아니면 데이터 독 자체 스펙인가?
There was a problem hiding this comment.
추후 문서화 예정이라 대략적인 제미나이의 답변 공유드립니다.
1. 포트 8125 (UDP) - DogStatsD (Metrics)
- 역할: 메트릭(Metrics) 데이터를 수집하는 입구입니다.
- 프로토콜: UDP (User Datagram Protocol)
- 데이터 종류:
- CPU 사용량, 메모리, 힙 메모리 사용량 (Micrometer가 보내는 데이터)
- "사용자 로그인 횟수", "게시글 작성 수" 같은 커스텀 카운터/게이지
- 특징 (Fire and Forget):
- 애플리케이션은 데이터를 던지기만 하고, Agent가 잘 받았는지 확인하지 않습니다.
- 이유: 메트릭 수집 때문에 애플리케이션의 성능이 느려지거나 멈추는 것을 방지하기 위함입니다. Agent가 죽어있어도 앱은 에러를 내지 않고 그냥 패킷을 유실시킵니다.
2. 포트 8126 (TCP) - Trace Agent (APM)
- 역할: 트레이스(Trace/APM) 데이터를 수집하는 입구입니다.
- 프로토콜: TCP (HTTP)
- 데이터 종류:
- API 요청의 흐름 (Controller -> Service -> DB)
- SQL 쿼리 내용, 에러 스택 트레이스
- 각 구간별 소요 시간 (Span)
- 특징 (Reliability):
- 트레이스 데이터는 JSON 형태의 큰 데이터이며 구조가 중요하기 때문에, 데이터의 신뢰성이 보장되는 TCP 통신을 사용합니다.
- Java Agent(
dd-java-agent.jar)가 내부적으로 이 포트로 HTTP 요청을 보냅니다.
docker/docker-compose.traefik.yaml
Outdated
| command: | ||
| - "--api.insecure=true" | ||
| - "--ping=true" | ||
| - "--log.level=DEBUG" |
There was a problem hiding this comment.
[궁금증]
로깅 레벨이 DEBUG로 설정되어 있는데 운영환경에서는 INFO레벨로 조정할 생각이죠?
There was a problem hiding this comment.
어짜피 traefik의 로그라서 문제 없을까 생각했는데, 그냥 INFO로 바꾸는 게 좋을 것 같네요.
INFO로 바꾸겠습니다.
| - "--entrypoints.websecure.address=:443" | ||
|
|
||
| - "--certificatesresolvers.myresolver.acme.tlschallenge=true" | ||
| - "--certificatesresolvers.myresolver.acme.email=debatetimekeeping@gmail.com" |
There was a problem hiding this comment.
내 이메일 주소 ㅠㅠ
Certbot 없이 자동으로 발급받고 관리해주네... 미쳤다.. 확실히 편한듯
There was a problem hiding this comment.
- 개인적으로는 각 컴포넌트 성격이 다르기 떄문에 분리되어 있어 읽기는 좋았습니다. 수정할 포인트도 더 잘 확인할 수 있을 것 같다는 생각도 들었구요.
- 확장자는 저도 궁금하네요
| WORKDIR /workspace | ||
|
|
||
| ADD https://dtdg.co/latest-java-tracer ./dd-java-agent.jar | ||
|
|
There was a problem hiding this comment.
문서화할 때 지금 전부다 jar로 마는 거랑 얼마나 효율화되었는지 수치로 나타내봐도 좋을듯!
| // Monitoring | ||
| implementation 'org.springframework.boot:spring-boot-starter-actuator' | ||
| implementation 'io.micrometer:micrometer-registry-prometheus' | ||
| implementation 'io.micrometer:micrometer-registry-statsd' |
There was a problem hiding this comment.
음..의존성 변경의 의미가 수집하여 반환하는 형식이 변경되었다~ 정도로 이해하고 있는데 맞을까요?
There was a problem hiding this comment.
맞아요~ 기존에는 Datadog Agent가 WAS에 요청을 보내서 가져가는 형식이었음
이번에는 WAS에서 Datadog Agent로 보내는 형식으로 변경함 (블루-그린 적용해도 로그 수집이 원활이 되도록 함)
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti @debate-timer/backend
님들, 리뷰에 대한 반영 & 코멘트 남겼습니다. 확인 부탁드립니다.
| - "--entrypoints.websecure.address=:443" | ||
|
|
||
| - "--certificatesresolvers.myresolver.acme.tlschallenge=true" | ||
| - "--certificatesresolvers.myresolver.acme.email=debatetimekeeping@gmail.com" |
There was a problem hiding this comment.
이거 노출되고 크게 문제 없는 정보니까, 이렇게 그대로 해도 되겠죠?
-> 그대로 가도 괜찮을듯 ㅇㅇ
docker/docker-compose.traefik.yaml
Outdated
| command: | ||
| - "--api.insecure=true" | ||
| - "--ping=true" | ||
| - "--log.level=DEBUG" |
There was a problem hiding this comment.
어짜피 traefik의 로그라서 문제 없을까 생각했는데, 그냥 INFO로 바꾸는 게 좋을 것 같네요.
INFO로 바꾸겠습니다.
| - DD_PROCESS_AGENT_ENABLED=true | ||
| - DD_TAGS=env:${ENV:-dev},project:debate-timer | ||
| ports: | ||
| - "8125:8125/udp" # Metrics (StatsD) |
There was a problem hiding this comment.
추후 문서화 예정이라 대략적인 제미나이의 답변 공유드립니다.
1. 포트 8125 (UDP) - DogStatsD (Metrics)
- 역할: 메트릭(Metrics) 데이터를 수집하는 입구입니다.
- 프로토콜: UDP (User Datagram Protocol)
- 데이터 종류:
- CPU 사용량, 메모리, 힙 메모리 사용량 (Micrometer가 보내는 데이터)
- "사용자 로그인 횟수", "게시글 작성 수" 같은 커스텀 카운터/게이지
- 특징 (Fire and Forget):
- 애플리케이션은 데이터를 던지기만 하고, Agent가 잘 받았는지 확인하지 않습니다.
- 이유: 메트릭 수집 때문에 애플리케이션의 성능이 느려지거나 멈추는 것을 방지하기 위함입니다. Agent가 죽어있어도 앱은 에러를 내지 않고 그냥 패킷을 유실시킵니다.
2. 포트 8126 (TCP) - Trace Agent (APM)
- 역할: 트레이스(Trace/APM) 데이터를 수집하는 입구입니다.
- 프로토콜: TCP (HTTP)
- 데이터 종류:
- API 요청의 흐름 (Controller -> Service -> DB)
- SQL 쿼리 내용, 에러 스택 트레이스
- 각 구간별 소요 시간 (Span)
- 특징 (Reliability):
- 트레이스 데이터는 JSON 형태의 큰 데이터이며 구조가 중요하기 때문에, 데이터의 신뢰성이 보장되는 TCP 통신을 사용합니다.
- Java Agent(
dd-java-agent.jar)가 내부적으로 이 포트로 HTTP 요청을 보냅니다.
There was a problem hiding this comment.
yml 하고 yaml 하고 차이 없어서 아무거나 사용했습니다.
Spring에서 appliction.yml 파일 이용하고 있으니까, yml로 통일할께요~
| # --- 설정 변수 --- | ||
| SWAP_FILE="/swapfile" | ||
| SWAP_SIZE="2G" | ||
| FSTAB_FILE="/etc/fstab" |
| // Monitoring | ||
| implementation 'org.springframework.boot:spring-boot-starter-actuator' | ||
| implementation 'io.micrometer:micrometer-registry-prometheus' | ||
| implementation 'io.micrometer:micrometer-registry-statsd' |
There was a problem hiding this comment.
맞아요~ 기존에는 Datadog Agent가 WAS에 요청을 보내서 가져가는 형식이었음
이번에는 WAS에서 Datadog Agent로 보내는 형식으로 변경함 (블루-그린 적용해도 로그 수집이 원활이 되도록 함)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/init/auto-swap.sh`:
- Around line 23-34: Add robust error handling and quoting: enable immediate
exit on failure by adding set -e at the script top (so any failing command
aborts), and ensure all variable uses (SWAP_FILE, SWAP_SIZE) are quoted;
additionally, after running fallocate check its failure and fall back to a
portable dd-based allocation before proceeding to chmod, mkswap and swapon, and
treat failures of mkswap/swapon as fatal (let set -e or explicit checks stop the
script).
🧹 Nitpick comments (5)
scripts/init/auto-swap.sh (1)
1-6:SWAP_SIZE입력값 검증 부재첫 번째 인자로 임의의 문자열이 전달될 수 있습니다.
fallocate가 잘못된 크기 값으로 실패할 수 있으므로, 간단한 형식 검증을 추가하면 좋겠습니다.💡 입력값 검증 예시
SWAP_SIZE=${1:-"2G"} FSTAB_FILE="/etc/fstab" + +# 입력값 검증 +if ! [[ "$SWAP_SIZE" =~ ^[0-9]+(G|M|K)$ ]]; then + echo "오류: 유효하지 않은 스왑 크기입니다: $SWAP_SIZE (예: 2G, 512M)" + exit 1 +fidocker/docker-compose.datadog.yml (1)
1-30: 전반적으로 잘 구성된 Datadog agent 설정입니다.몇 가지 개선 사항을 제안드립니다:
restart정책 미설정: 서버 재시작이나 에이전트 크래시 시 컨테이너가 자동 복구되지 않습니다.restart: unless-stopped추가를 권장합니다.- 포트 바인딩 범위:
8125와8126이 모든 인터페이스(0.0.0.0)에 바인딩됩니다. 외부에서 접근할 필요가 없다면127.0.0.1:8125:8125/udp형태로 제한하는 것이 안전합니다.🔧 제안된 수정
services: datadog-agent: image: gcr.io/datadoghq/agent:7 container_name: datadog-agent + restart: unless-stopped environment: - DD_API_KEY=${DD_API_KEY} ... ports: - - "8125:8125/udp" # Metrics (StatsD) - - "8126:8126/tcp" # APM (Trace) + - "127.0.0.1:8125:8125/udp" # Metrics (StatsD) + - "127.0.0.1:8126:8126/tcp" # APM (Trace)docker/docker-compose.traefik.yml (2)
5-9: 포트8080외부 노출 주의
--api.insecure=true로 인해 Traefik 대시보드가 인증 없이 포트 8080에 노출됩니다. Based on learnings, 테스트 환경에서는 허용되지만 프로덕션 배포 시에는 반드시 인증을 추가하거나 비활성화해야 합니다.포트 바인딩도
127.0.0.1:8080:8080으로 제한하면 외부 직접 접근을 차단할 수 있습니다.
1-39:restart정책 추가 권장Traefik은 리버스 프록시로서 핵심 인프라이므로, 장애 시 자동 복구가 중요합니다.
restart: unless-stopped추가를 권장합니다.🔧 제안된 수정
services: traefik: image: traefik:v2.11 container_name: traefik + restart: unless-stopped ports:docker/docker-compose.spring.yml (1)
1-37: 잘 구성된 앱 템플릿입니다.YAML 앵커를 활용한 DRY 패턴과
depends_on헬스체크 조건이 잘 설정되어 있습니다. 몇 가지 제안사항:
DD_VERSION=1.0.0하드코딩: 배포 시 버전 추적을 위해 환경 변수로 외부화하는 것을 고려해 주세요 (예:DD_VERSION=${APP_VERSION:-1.0.0}).restart정책 미설정: 앱 컨테이너도restart: unless-stopped추가를 권장합니다.
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
궁금했던 부분들에 대해서 잘 대답해줘서 감사합니다 커찬 ㅎㅎ
인생 쉽지 않네요 ㅋㅋㅋ 비토와의 논의사안에 대해선 제 의견 남겻어요. 다만 애매모호함이란 말이 애매모호해서 한번더 물어보고 싶네요
|
/gemini review |
There was a problem hiding this comment.
Code Review
안녕하세요. Docker를 도입하여 인프라 구조를 개선하는 PR을 리뷰했습니다. 전반적으로 코드 품질이 우수하며, 특히 초기 서버 설정 스크립트와 Blue/Green 배포를 고려한 Docker Compose 구성이 인상적입니다. 인프라를 코드로 관리하려는 방향성이 돋보입니다.
PR 설명에서 질문 주신 "서버 초기 실행 스크립트를 Git으로 관리해야 할까?"에 대해서는, 현재와 같이 Git으로 관리하는 것이 매우 좋은 방법이라고 생각합니다. 이는 인프라 구성의 변경 이력을 추적하고, 누구나 동일한 환경을 쉽게 구성할 수 있도록 돕는 IaC(Infrastructure as Code)의 핵심적인 장점입니다.
몇 가지 개선점을 제안드렸습니다.
- 보안 강화: Traefik 대시보드 접근 제어는 시급히 필요해 보입니다.
- 안정성 및 재현성 향상: Docker 이미지 및 에이전트의 버전을 명시하여 예기치 않은 문제를 방지하는 것이 좋습니다.
- 유연성 확보: 하드코딩된 설정값들을 환경 변수로 관리하여 유연성을 높이는 것을 제안합니다.
자세한 내용은 각 파일에 남긴 코멘트를 참고해주세요. 훌륭한 리팩터링 작업에 감사드립니다!
| environment: | ||
| - DOCKER_API_VERSION=1.44 | ||
| command: | ||
| - "--api.insecure=true" |
There was a problem hiding this comment.
Traefik API 및 대시보드를 인증 없이(--api.insecure=true) 활성화하고 포트 8080을 외부에 노출하고 있습니다. 이는 서버의 네트워크 구성, 라우팅 규칙 등 민감한 정보가 외부에 그대로 노출되는 심각한 보안 취약점입니다. 운영 환경에서는 Basic Auth와 같은 인증을 설정하거나, 포트 포워딩(8080:8080)을 제거하여 외부 접근을 차단하는 것을 강력히 권장합니다.
| FROM eclipse-temurin:17-jre-alpine | ||
| WORKDIR /workspace | ||
|
|
||
| ADD https://dtdg.co/latest-java-tracer ./dd-java-agent.jar |
There was a problem hiding this comment.
ADD 명령어로 latest 버전의 Datadog 에이전트를 직접 다운로드하고 있습니다. latest 태그는 언제든지 변경될 수 있어 빌드의 재현성을 해칠 수 있습니다. 특정 버전을 명시하여 안정성을 높이는 것을 권장합니다. 또한, URL에서 직접 파일을 ADD하는 것보다 RUN과 wget 또는 curl을 사용하면 캐시 동작을 더 명확하게 제어할 수 있습니다.
RUN wget -O ./dd-java-agent.jar https://dtdg.co/latest-java-tracer
| @@ -0,0 +1,30 @@ | |||
| services: | |||
| datadog-agent: | |||
| image: gcr.io/datadoghq/agent:7 | |||
There was a problem hiding this comment.
agent:7 이미지 태그는 7.x.x 버전 중 가장 최신 버전을 가리키는 부동 태그(floating tag)입니다. 이로 인해 의도치 않은 에이전트 버전 업데이트가 발생하여 예기치 않은 문제가 생길 수 있습니다. 7.54.0과 같이 특정 버전을 명시하여 빌드의 재현성과 안정성을 확보하는 것을 권장합니다.
image: gcr.io/datadoghq/agent:7.54.0| - DD_AGENT_HOST=datadog-agent | ||
| - DD_SERVICE=debate-timer | ||
| - DD_ENV=${ENV:-dev} | ||
| - DD_VERSION=1.0.0 |
There was a problem hiding this comment.
Datadog 서비스 버전(DD_VERSION)이 1.0.0으로 하드코딩되어 있습니다. 이 경우 배포 버전이 변경되어도 Datadog 대시보드에서는 항상 동일한 버전으로 표시됩니다. CI/CD 파이프라인에서 Git 태그나 커밋 해시 등을 동적으로 주입하여, 배포된 애플리케이션의 버전을 정확하게 추적할 수 있도록 개선하는 것을 권장합니다.
- DD_VERSION=${APP_VERSION:-1.0.0}| - "--entrypoints.websecure.address=:443" | ||
|
|
||
| - "--certificatesresolvers.myresolver.acme.tlschallenge=true" | ||
| - "--certificatesresolvers.myresolver.acme.email=debatetimekeeping@gmail.com" |
There was a problem hiding this comment.
Let's Encrypt 인증서 발급에 사용되는 이메일 주소가 하드코딩되어 있습니다. 이메일 주소는 환경에 따라 달라지거나 변경될 수 있으므로, .env 파일과 환경 변수를 사용하여 관리하는 것이 더 유연하고 안전합니다.
- "--certificatesresolvers.myresolver.acme.email=${ACME_EMAIL}"| start_period: 120s | ||
| volumes: | ||
| - /var/run/docker.sock:/var/run/docker.sock:ro | ||
| - ../letsencrypt:/letsencrypt |
There was a problem hiding this comment.
../letsencrypt와 같이 상대 경로를 사용하여 볼륨을 마운트하고 있습니다. docker-compose 명령어를 실행하는 위치에 따라 경로가 달라질 수 있어 오류를 유발할 수 있습니다. Docker의 네임드 볼륨(named volume)을 사용하면 이 문제를 해결하고 더 안정적으로 데이터를 관리할 수 있습니다.
docker-compose.traefik.yml에서 상대 경로 대신 네임드 볼륨을 지정하고, 최상위 docker-compose.yml 파일에 볼륨을 정의하는 것을 권장합니다.
- letsencrypt:/letsencrypt3 participants
🚩 연관 이슈
#235 #236
작업 내용
추후 작업 내용
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
인프라 및 배포
스크립트