Open Source APM/Observability 운영 경로 설계 및 구현 - #149
Merged
Merged
Conversation
Contributor
Contributor
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Observability Infrastructure & Configuration docker-compose.observability.yml, observability/prometheus.yml, observability/tempo.yaml, observability/grafana/provisioning/datasources/datasources.yaml |
Adds Docker Compose orchestration for Prometheus, Grafana, Loki, Tempo with mounted configs; Prometheus scrapes backend:8000; Tempo configured for OTLP ingestion, compaction, and local storage; Grafana datasources provisioned (Prometheus default). |
Backend Dependencies & Configuration backend/requirements.txt, backend/pytest.ini |
Adds OpenTelemetry and Prometheus instrumentation packages to requirements and adds pytest warnings filters for pkg_resources deprecation messages. |
FastAPI App Instrumentation backend/main.py |
Reinitializes FastAPI with title/version/lifespan; conditionally configures OpenTelemetry TracerProvider and OTLP exporter when OTEL_EXPORTER_OTLP_ENDPOINT is set; attaches Prometheus Instrumentator; instruments app with OpenTelemetry; moves and broadens CORS middleware. |
Observability Verification & Documentation backend/tests/test_apm_observability.py, docs/plans/2026-05-11-apm-observability-implementation.md |
Adds test suite verifying observability compose and config files exist and that /metrics returns HTTP 200 with Prometheus-style metrics; adds implementation plan document. |
Sequence Diagram(s)
sequenceDiagram
participant Client as User/TestClient
participant FastAPI as FastAPI App
participant Prom as Prometheus
participant OTLP as OTLP Exporter
participant Tempo as Tempo
participant Grafana as Grafana
Client->>FastAPI: GET /metrics
FastAPI->>Prom: expose metrics endpoint (/metrics)
Prom->>Grafana: Grafana queries Prometheus datasource
FastAPI->>OTLP: send spans to OTLP exporter (if OTEL_EXPORTER_OTLP_ENDPOINT)
OTLP->>Tempo: OTLP receiver ingests traces
Grafana->>Tempo: Grafana queries Tempo datasource for traces
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
🐰 A rabbit's observability tune
Traces hop from span to span,
Metrics bloom beneath each plan,
Dashboards watch the traffic flow,
Small hops logged in evening glow.
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Linked Issues check | ❓ Inconclusive | PR partially addresses issue #135 by implementing observability infrastructure (docker-compose, prometheus/grafana/tempo/loki, opentelemetry instrumentation) but does not fully satisfy all completion conditions like PII/secret redaction policies, SLO documentation, and AKS Dev verification. |
Complete remaining objectives: document PII/secret redaction criteria, define SLO candidates (latency/error rate), verify smoke tests in AKS Dev environment, and connect results to #118 release evidence. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR title is in Korean and describes APM/observability design and implementation, which aligns with the main objectives and changes introducing observability infrastructure. |
| Out of Scope Changes check | ✅ Passed | All changes align with issue #135 objectives: observability infrastructure setup (prometheus/grafana/tempo/loki), backend FastAPI instrumentation with opentelemetry and prometheus metrics, grafana provisioning, and supporting tests and documentation. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/apm-observability-20260511
Warning
Review ran into problems
🔥 Problems
Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.
Comment @coderabbitai help to get the list of available commands and usage tips.
Contributor
1 similar comment
Contributor
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
Author
|
@coderabbitai approve |
Greptile Summary
Confidence Score: 3/5이전 리뷰에서 지적된 CORSMiddleware 중복 등록과 Prometheus 네트워크 분리 문제가 아직 해결되지 않아 바로 병합하기 어렵습니다. 이전 스레드에서 P1 수준의 CORSMiddleware 중복 등록(CORS 헤더 이중 삽입)과 Prometheus 네트워크 분리 문제(스크랩 무음 실패)가 지적되었으나 코드에 그대로 남아 있습니다. 신규 P2 이슈(Grafana 익명 Admin, setuptools 핀)도 추가됩니다. P1이 복수로 잔존하므로 4/5 상한에서 추가 하향합니다.
|
| Filename | Overview |
|---|---|
| backend/main.py | Prometheus + OpenTelemetry 계측 추가. CORSMiddleware 중복 등록(lines 59-65, 75-81)이 여전히 남아 있어 브라우저 CORS 오류를 유발하며, 미사용 settings import도 잔존. |
| backend/requirements.txt | OpenTelemetry·Prometheus 패키지 추가. setuptools<71.0.0 핀이 런타임 의존성 파일에 직접 포함되어 잠재적 충돌 가능성 있음. |
| backend/tests/test_apm_observability.py | APM 관련 통합 테스트 추가. 상대 경로 기반 파일 존재 확인으로 CWD 의존성 있고, 미사용 subprocess import 잔존. |
| docker-compose.observability.yml | Prometheus/Grafana/Loki/Tempo 로컬 스택 정의. Grafana 익명 Admin 권한 설정, 포트 3000 충돌(프론트엔드), Prometheus의 별도 네트워크 문제 있음. |
| observability/prometheus.yml | Prometheus 스크랩 설정. backend:8000 타겟이 별도 Docker 네트워크로 인해 도달 불가능한 상태. |
| observability/tempo.yaml | Tempo 트레이스 설정. block_retention: 1h가 매우 짧아 디버깅에 부적합. |
| observability/grafana/provisioning/datasources/datasources.yaml | Prometheus·Tempo·Loki 데이터소스 자동 프로비저닝. 구성 자체는 문제 없음. |
Sequence Diagram
sequenceDiagram
participant FE as Frontend (Next.js :3000)
participant BE as Backend (FastAPI :8000)
participant PROM as Prometheus (:9090)
participant TEMPO as Tempo (:3200 / :4317)
participant LOKI as Loki (:3100)
participant GRAF as Grafana (:3000)
FE->>BE: HTTP Request
BE-->>BE: OTel Middleware (span 생성)
BE-->>TEMPO: OTLP gRPC (if OTEL_EXPORTER_OTLP_ENDPOINT 설정)
PROM->>BE: GET /metrics (scrape, 15s 간격)
Note over PROM,BE: ⚠️ 다른 Docker 네트워크 → backend:8000 접근 불가
GRAF->>PROM: Query (PromQL)
GRAF->>TEMPO: Query (TraceQL)
GRAF->>LOKI: Query (LogQL)
Reviews (4): Last reviewed commit: "fix(security): resolve CORS overly permi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
backend/main.py (1)
36-51: 💤 Low valueConsider adding graceful shutdown for the trace provider.
The
TracerProviderandBatchSpanProcessorare initialized but never shut down. TheBatchSpanProcessorbuffers spans and flushes them periodically—without a shutdown call, spans buffered at process exit may be lost.Proposed enhancement in lifespan
`@asynccontextmanager` async def lifespan(app: FastAPI): if not DISABLE_WORKERS: await imap_worker.start() yield if not DISABLE_WORKERS: await imap_worker.stop() + # Flush remaining spans on shutdown + if os.environ.get("OTEL_EXPORTER_OTLP_ENDPOINT"): + from opentelemetry import trace + provider = trace.get_tracer_provider() + if hasattr(provider, 'shutdown'): + provider.shutdown()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 36 - 51, The TracerProvider and BatchSpanProcessor (variables trace_provider and processor) are never shut down, risking loss of buffered spans; add a graceful shutdown that calls processor.shutdown() and trace_provider.shutdown() (or trace.get_tracer_provider().shutdown()) on application exit—e.g., register an atexit handler or hook into the app's shutdown lifecycle/signal handlers to invoke these shutdown calls so the OTLPSpanExporter flushes buffered spans before process exit.docker-compose.observability.yml (1)
8-9: ⚡ Quick winPersist observability state with named volumes.
Current mounts provide config only; metrics/traces/dashboards are ephemeral on container recreation. For 운영 검증/증적 보존, add named data volumes for at least Prometheus and Grafana.
Example volume persistence (minimum set)
services: prometheus: @@ volumes: - ./observability/prometheus.yml:/etc/prometheus/prometheus.yml:ro + - prometheus_data:/prometheus @@ grafana: @@ volumes: - ./observability/grafana/provisioning:/etc/grafana/provisioning + - grafana_data:/var/lib/grafana @@ +volumes: + prometheus_data: + grafana_data:Also applies to: 18-19, 30-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose.observability.yml` around lines 8 - 9, Update docker-compose.observability.yml to persist observability state by replacing ephemeral bind mounts with named volumes for Prometheus and Grafana: for the prometheus service (currently mounting ./observability/prometheus.yml:/etc/prometheus/prometheus.yml:ro) keep the config bind but add a named volume (e.g., prometheus_data:/prometheus) to persist TSDB data; for the grafana service add a named volume (e.g., grafana_storage:/var/lib/grafana) instead of ephemeral mounts so dashboards and plugins survive restarts; finally declare the named volumes under the top-level volumes: section (prometheus_data: and grafana_storage:) so Docker manages them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Around line 59-65: Remove the duplicate permissive CORS middleware block that
calls app.add_middleware with CORSMiddleware and
allow_origins=["*"]/allow_credentials=True; keep the existing CORS configuration
that restricts origins (the later app.add_middleware(CORSMiddleware, ...) block)
so credentialed requests use the specific allow_origins (e.g.,
"http://localhost:3000"); ensure only one app.add_middleware(CORSMiddleware,
...) call remains and that it does not combine allow_origins=["*"] with
allow_credentials=True.
In `@backend/tests/test_apm_observability.py`:
- Around line 5-11: The tests test_observability_compose_file_exists and
test_observability_provisioning_exists currently use relative paths that break
if pytest is run from a different CWD; change them to compute absolute paths
from the repository/test file location using __file__ (e.g. base =
Path(__file__).resolve().parent) and then assert existence with
base.joinpath("../docker-compose.observability.yml").resolve() and similarly for
"../observability/grafana/provisioning/datasources/datasources.yaml",
"../observability/prometheus.yml", and "../observability/tempo.yaml" so the
assertions no longer depend on the current working directory.
- Line 2: The file contains an unused top-level import "subprocess" which should
be removed; delete the line "import subprocess" from
backend/tests/test_apm_observability.py (or wherever the import appears) so
there are no unused imports, then run the test/lint suite (pytest/flake8) to
confirm no import warnings remain.
In `@docker-compose.observability.yml`:
- Around line 16-17: The Docker Compose Grafana config currently grants
anonymous users admin rights via GF_AUTH_ANONYMOUS_ENABLED and
GF_AUTH_ANONYMOUS_ORG_ROLE; change this to remove admin privileges by either
setting GF_AUTH_ANONYMOUS_ENABLED=false to disable anonymous access or, if
anonymous access is desired, set GF_AUTH_ANONYMOUS_ORG_ROLE=Viewer (not Admin)
and ensure admin operations require authenticated credentials. Update the
docker-compose.observability.yml environment entries for
GF_AUTH_ANONYMOUS_ENABLED and GF_AUTH_ANONYMOUS_ORG_ROLE accordingly and verify
admin access is gated behind proper credentials.
- Around line 1-10: Prometheus in the prometheus service cannot resolve
backend:8000 because the observability compose file creates its own network; fix
by adding a shared network or documenting the combined compose run: either (A)
add a named external network (e.g., "project_default") in
docker-compose.observability.yml and attach the prometheus service to that
external network (and ensure the main docker-compose.yml attaches backend
service to the same external network), or (B) merge the prometheus service into
the main docker-compose.yml, or (C) update the README to instruct users to run
both files together with docker compose -f docker-compose.yml -f
docker-compose.observability.yml up so prometheus can resolve backend:8000;
reference the prometheus service and its target backend:8000 when making the
change.
---
Nitpick comments:
In `@backend/main.py`:
- Around line 36-51: The TracerProvider and BatchSpanProcessor (variables
trace_provider and processor) are never shut down, risking loss of buffered
spans; add a graceful shutdown that calls processor.shutdown() and
trace_provider.shutdown() (or trace.get_tracer_provider().shutdown()) on
application exit—e.g., register an atexit handler or hook into the app's
shutdown lifecycle/signal handlers to invoke these shutdown calls so the
OTLPSpanExporter flushes buffered spans before process exit.
In `@docker-compose.observability.yml`:
- Around line 8-9: Update docker-compose.observability.yml to persist
observability state by replacing ephemeral bind mounts with named volumes for
Prometheus and Grafana: for the prometheus service (currently mounting
./observability/prometheus.yml:/etc/prometheus/prometheus.yml:ro) keep the
config bind but add a named volume (e.g., prometheus_data:/prometheus) to
persist TSDB data; for the grafana service add a named volume (e.g.,
grafana_storage:/var/lib/grafana) instead of ephemeral mounts so dashboards and
plugins survive restarts; finally declare the named volumes under the top-level
volumes: section (prometheus_data: and grafana_storage:) so Docker manages them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c793e02-6e04-44c5-a27e-5f6e44d33bb9
📒 Files selected for processing (9)
backend/main.pybackend/pytest.inibackend/requirements.txtbackend/tests/test_apm_observability.pydocker-compose.observability.ymldocs/plans/2026-05-11-apm-observability-implementation.mdobservability/grafana/provisioning/datasources/datasources.yamlobservability/prometheus.ymlobservability/tempo.yaml
Contributor
✅ Actions performedComments resolved and changes approved. |
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for |
1 similar comment
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for
|
… and ignore warnings
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for |
1 similar comment
Contributor
|
PR governance metadata gate is not ready for |
Contributor
Author
|
@coderabbitai approve |
Contributor
✅ Actions performedComments resolved and changes approved. |
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for |
Contributor
|
PR governance metadata gate is not ready for |
Contributor
Author
|
@coderabbitai approve |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/main.py (1)
59-81:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep only one
CORSMiddlewarepolicy.The second CORS registration at Line 75 still overrides/conflicts with the broader origin list added here, so
127.0.0.1:3000and:8000are not reliably allowed in practice. Merge on a single middleware block instead of stacking two policies.Proposed fix
app.add_middleware( CORSMiddleware, allow_origins=["http://localhost:3000", "http://127.0.0.1:3000", "http://localhost:8000", "http://127.0.0.1:8000"], allow_credentials=True, allow_methods=["*"], allow_headers=["*"], ) @@ -app.add_middleware( - CORSMiddleware, - allow_origins=["http://localhost:3000"], - allow_credentials=True, - allow_methods=["*"], - allow_headers=["*"], -)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 59 - 81, There are two app.add_middleware(CORSMiddleware, ...) registrations which conflict; keep a single CORSMiddleware call that contains the full allow_origins list (including "http://localhost:3000", "http://127.0.0.1:3000", "http://localhost:8000", "http://127.0.0.1:8000"), remove the duplicate block that only allows "http://localhost:3000", and ensure the remaining app.add_middleware(CORSMiddleware, ...) uses allow_credentials=True, allow_methods=["*"], and allow_headers=["*"] so CORS is applied once consistently.
🧹 Nitpick comments (1)
backend/main.py (1)
53-57: ⚡ Quick winExclude
/metricsfrom tracing.Once Prometheus starts scraping, Line 57 will emit a span for every scrape. That adds steady Tempo noise and makes real request traces harder to inspect.
Proposed fix
# Instrument Prometheus Metrics Instrumentator().instrument(app).expose(app, include_in_schema=False, should_gzip=True) # Instrument OpenTelemetry -FastAPIInstrumentor.instrument_app(app) +FastAPIInstrumentor.instrument_app(app, excluded_urls="/metrics")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 53 - 57, The FastAPIInstrumentor.instrument_app call currently instruments the /metrics scrape; update it to exclude that path so Prometheus scrapes don't create spans. Replace the current FastAPIInstrumentor.instrument_app(app) invocation with a call that passes an excluded_urls list (e.g., FastAPIInstrumentor.instrument_app(app, excluded_urls=[r"^/metrics$"]) or equivalent regex) so the /metrics endpoint is not traced; keep the Instrumentator() call for Prometheus unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Around line 37-52: The current init block only checks
OTEL_EXPORTER_OTLP_ENDPOINT and will skip tracing when only
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT is set; update the guard to check for either
environment variable (OTEL_EXPORTER_OTLP_ENDPOINT or
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) before importing and configuring
OpenTelemetry so OTLPSpanExporter() is initialized when either endpoint var is
present; leave the rest of the setup (resource, TracerProvider/trace_provider,
BatchSpanProcessor/processor, trace.set_tracer_provider) unchanged.
---
Duplicate comments:
In `@backend/main.py`:
- Around line 59-81: There are two app.add_middleware(CORSMiddleware, ...)
registrations which conflict; keep a single CORSMiddleware call that contains
the full allow_origins list (including "http://localhost:3000",
"http://127.0.0.1:3000", "http://localhost:8000", "http://127.0.0.1:8000"),
remove the duplicate block that only allows "http://localhost:3000", and ensure
the remaining app.add_middleware(CORSMiddleware, ...) uses
allow_credentials=True, allow_methods=["*"], and allow_headers=["*"] so CORS is
applied once consistently.
---
Nitpick comments:
In `@backend/main.py`:
- Around line 53-57: The FastAPIInstrumentor.instrument_app call currently
instruments the /metrics scrape; update it to exclude that path so Prometheus
scrapes don't create spans. Replace the current
FastAPIInstrumentor.instrument_app(app) invocation with a call that passes an
excluded_urls list (e.g., FastAPIInstrumentor.instrument_app(app,
excluded_urls=[r"^/metrics$"]) or equivalent regex) so the /metrics endpoint is
not traced; keep the Instrumentator() call for Prometheus unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a941605-e33e-456e-99be-c2aa5cb889fa
📒 Files selected for processing (1)
backend/main.py
Contributor
✅ Actions performedComments resolved and changes approved. |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
목표
Application Performance Monitoring을 Open Source 기반으로 설계 및 연동합니다.
변경 사항
docker-compose.observability.yml을 통해 Prometheus, Grafana, Loki, Tempo의 로컬 스택을 마련했습니다.prometheus-fastapi-instrumentator와opentelemetry-instrumentation-fastapi를 도입해 FastAPI에서/metrics가 노출되고 OTLP로 트레이스를 쏠 수 있는 뼈대를 구성했습니다.observability/grafana/provisioning/datasources.yaml을 통해 기본 데이터 소스가 연동되도록 구성했습니다.관련 이슈
Resolves: #135
Summary by CodeRabbit
New Features
Tests
Documentation