[Observability] 비동기 Pipeline 상태 전이 메트릭과 실패율 경보 추가

[Gimini-3]

문제와 결과

인덱싱·RAG·Sync Outbox의 실패와 복구는 로그와 누적 DB 상태로만 확인할 수 있어 최근 장애 증가를 Prometheus가 판단할 수 없었습니다. 누적 FAILED 행은 자동으로 줄지 않기 때문에 현재 개수에 경보를 걸면 장애가 끝나도 해제되지 않습니다.

이 변경은 실제 DB 상태 전이가 커밋된 뒤에만 Micrometer Counter를 증가시킵니다. retryable 인프라 실패와 사용자 문서 오류, RAG provider fallback과 timeout 회수, Outbox 재시도와 최종 실패를 제한된 label로 구분합니다.

변경 내용

• docgrid_embedding_job_attempts_total 추가
  • success, retry_scheduled, terminal_failure
  • 고정 failure_type, retryable label
• docgrid_rag_job_completions_total 추가
  • success, no_context, provider_fallback, timeout_swept, unexpected_failure
• docgrid_sync_event_attempts_total 추가
  • processed, retry_scheduled, terminal_failure, lease_recovered
• Transaction event와 AFTER_COMMIT listener로 rollback·경합·멱등 재생의 중복 집계 방지
• 최소 표본과 지속 시간을 포함한 Pipeline 경보 4개와 promtool 시나리오 추가
• Metric 계약, cardinality 제한, 검증 결과 문서화

검증

• Backend 전체 테스트: 1164개 통과, 실패 0, errors 0, skipped 0
• Embedding 성공·재시도·최종 실패와 RAG 예상 밖 실패의 metric event 발행 분기 검증
• Sync Outbox 실패·Lease 회수의 재예약과 최종 실패 metric event 분기 검증
• commit 후 Counter 증가·rollback 시 미증가 통합 테스트 통과
• RAG 조건부 UPDATE 경합 패배와 Sync Handler 실패 시 event 미발행 테스트 통과
• promtool check config: 성공, rule file 3개·총 14개 규칙
• promtool check rules: 성공
• promtool test rules: Backend와 Pipeline suite 모두 성공
• 기본·monitoring profile docker compose config: 성공
• git diff --check: 성공

Closes #332

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 26128af1-8e70-46dc-9f62-db44d1fe5d86

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟡 Changes recommended

A critical test matcher defect and unresolved metric-coverage issues remain, along with tagging and documentation nits.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds commit-aware Micrometer metrics and Prometheus alerts for asynchronous Embedding, RAG, and Sync Outbox state transitions.

Changes:

• Added bounded-label metric events with AFTER_COMMIT recording.
• Added four pipeline alerts, scenarios, and monitoring documentation.
• Added service, integration, and observability tests with validation documentation.

File summaries

File	Summary / review note
monitoring/prometheus/tests/docgrid-pipeline-alerts.test.yml	Pipeline alert scenarios
monitoring/prometheus/rules/docgrid-pipeline-alerts.yml	Four pipeline alert rules
monitoring/prometheus/README.md	Monitoring documentation
docs/test-results/gimin-#332-async-pipeline-transition-metrics.md	Verification results
docs/design/gimin-#332-async-pipeline-transition-metrics.md	Metric design; nit, 1 vote: add closes #332
backend/src/test/java/com/opensource/docgrid/global/observability/AsyncPipelineMetricsTest.java	Metric unit tests
backend/src/test/java/com/opensource/docgrid/global/observability/AsyncPipelineMetricsAfterCommitIntegrationTest.java	Commit/rollback tests; nit, 2 votes: add the integration tag
backend/src/test/java/com/opensource/docgrid/domain/sync/service/command/SyncEventDispatchServiceTest.java	Sync transition tests
backend/src/test/java/com/opensource/docgrid/domain/rag/service/RagFacadeTest.java	RAG behavior tests
backend/src/test/java/com/opensource/docgrid/domain/embedding/service/command/EmbeddingJobLeaseRecoveryServiceTest.java	Lease recovery tests; critical, 1 vote: use anyBoolean() instead of any(Boolean.class)
backend/src/main/java/com/opensource/docgrid/global/observability/SyncEventAttemptMetricEvent.java	Sync metric event contract
backend/src/main/java/com/opensource/docgrid/global/observability/RagJobCompletionMetricEvent.java	RAG metric event contract
backend/src/main/java/com/opensource/docgrid/global/observability/EmbeddingJobAttemptMetricEvent.java	Embedding metric event contract
backend/src/main/java/com/opensource/docgrid/global/observability/AsyncPipelineMetrics.java	After-commit metric recording
backend/src/main/java/com/opensource/docgrid/domain/sync/service/command/SyncEventLeaseRecoveryService.java	Sync lease recovery metrics
backend/src/main/java/com/opensource/docgrid/domain/sync/service/command/SyncEventFailureService.java	Sync failure metrics
backend/src/main/java/com/opensource/docgrid/domain/sync/service/command/SyncEventDispatchService.java	Sync dispatch metrics
backend/src/main/java/com/opensource/docgrid/domain/rag/service/RagFacade.java	RAG completion metrics; moderate, 1 vote: cover unexpected-failure success and update-loss paths
backend/src/main/java/com/opensource/docgrid/domain/embedding/service/command/EmbeddingJobLeaseRecoveryService.java	Embedding lease recovery metrics
backend/src/main/java/com/opensource/docgrid/domain/embedding/service/command/DocumentIndexingFailureService.java	Moderate, 1 vote: assert metric events and failure_type/retryable labels
backend/src/main/java/com/opensource/docgrid/domain/embedding/service/command/DocumentIndexingCompletionService.java	Moderate, 1 vote: assert successful metric-event publication

Review details

Suppressed comments (4)

backend/src/main/java/com/opensource/docgrid/domain/embedding/service/command/DocumentIndexingCompletionService.java:144

• The existing completion-service tests exercise this successful transition but never assert the newly added EmbeddingJobAttemptMetricEvent.success() publication. Add an assertion so a regression cannot leave the embedding success counter missing while the database transition still passes.

        applicationEventPublisher.publishEvent(new EmbeddingJobStatusChangedEvent(embeddingJob.getId()));
        applicationEventPublisher.publishEvent(EmbeddingJobAttemptMetricEvent.success());

backend/src/main/java/com/opensource/docgrid/domain/embedding/service/command/DocumentIndexingFailureService.java:179

• The failure-service tests cover both retry scheduling and terminal failure state changes, but do not assert the new metric event or its failure_type/retryable labels. Add assertions for both outcomes; otherwise the alerting counter can regress without failing the existing state-transition tests.

        applicationEventPublisher.publishEvent(new EmbeddingJobStatusChangedEvent(embeddingJob.getId()));
        applicationEventPublisher.publishEvent(EmbeddingJobAttemptMetricEvent.failure(
            embeddingJob.getStatus(),
            request.failureType()
        ));

backend/src/main/java/com/opensource/docgrid/domain/rag/service/RagFacade.java:255

• The new UNEXPECTED_FAILURE metric path is not covered by RagFacadeTest: no test invokes markUnexpectedFailure and verifies that the event is emitted only when completeFailed actually updates the row. A regression here would silently omit unexpected worker failures from the RAG completion counter; add success and conditional-update-loss cases.

        if (completed) {
            applicationEventPublisher.publishEvent(
                new RagJobCompletionMetricEvent(Outcome.UNEXPECTED_FAILURE)
            );
        }

docs/design/gimin-#332-async-pipeline-transition-metrics.md:1

• The repository documentation rule requires every design document to include a closes #<issue> line. This document names #332 in the heading but omits that closing reference, so add closes #332 as the other design documents do.

# 비동기 Pipeline 상태 전이 메트릭 설계 (#332)

• Files reviewed: 21/21 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Test coverage is missing for Sync retry/terminal-failure and lease-recovery metric branches.

Review details

Suppressed comments (2)

backend/src/main/java/com/opensource/docgrid/domain/sync/service/command/SyncEventFailureService.java:63

• The new retry/terminal metric branches are not asserted by the test suite: there is no SyncEventFailureServiceTest, and the existing integration cases only verify persisted status/attempt rows. A regression that drops or swaps these two outcome labels could therefore pass; add publisher assertions for both branches (and the rollback/commit boundary as appropriate).

            applicationEventPublisher.publishEvent(
                new SyncEventAttemptMetricEvent(Outcome.TERMINAL_FAILURE)
            );

backend/src/main/java/com/opensource/docgrid/domain/sync/service/command/SyncEventLeaseRecoveryService.java:76

• The new LEASE_RECOVERED/TERMINAL_FAILURE selection is not covered by a service test: no SyncEventLeaseRecoveryServiceTest exists, while the integration test only checks that recovery leaves the event PENDING and records the lease error. Please add assertions for both post-recovery outcomes so the metric label cannot regress independently of the database transition.

        // 5. 최종 실패는 즉시 조치 경보 대상이며, 재예약된 Lease 만료는 회수 활동으로 구분한다.
        Outcome outcome = event.getStatus() == SyncEventStatus.FAILED
            ? Outcome.TERMINAL_FAILURE
            : Outcome.LEASE_RECOVERED;
        applicationEventPublisher.publishEvent(new SyncEventAttemptMetricEvent(outcome));

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Replace the failure-type valueOf mapping with an explicit or shared mapping before approval.

Review details

Suppressed comments (2)

backend/src/main/java/com/opensource/docgrid/domain/embedding/service/command/EmbeddingJobLeaseRecoveryService.java:94

• The terminal lease-recovery branch is not covered by the new tests: EmbeddingJobLeaseRecoveryServiceTest.setUp() forces every mocked transition to PENDING, so a regression in the FAILED -> terminal_failure mapping could pass the suite. Add an exhausted-retry case that leaves the job FAILED and asserts leaseExpired(FAILED).

        // 5. 실제 회수 전이가 커밋된 뒤에만 재시도 또는 최종 실패 Counter가 증가하게 한다.
        applicationEventPublisher.publishEvent(
            EmbeddingJobAttemptMetricEvent.leaseExpired(embeddingJob.getStatus())
        );

backend/src/main/java/com/opensource/docgrid/global/observability/EmbeddingJobAttemptMetricEvent.java:116

• FailureType duplicates the IndexingFailureType names and retryability flags, then relies on valueOf. If a new domain failure is added or its retryability changes without a synchronized edit here, this throws after the database transition has been mutated, rolling back the request and losing the metric. Use a compile-time explicit mapping or a single source of truth so adding a failure type cannot break the failure path.

        private static FailureType from(IndexingFailureType failureType) {
            return valueOf(Objects.requireNonNull(failureType, "failureType은 필수입니다.").name());

• Files reviewed: 25/25 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Embedding metric events allow contradictory retryability labels, which can undercount retryable-failure alerts.

Review details

Suppressed comments (1)

backend/src/main/java/com/opensource/docgrid/global/observability/EmbeddingJobAttemptMetricEvent.java:62

• The public record constructor still allows contradictory labels such as RETRY_SCHEDULED + EMBEDDING_PROVIDER_TIMEOUT + retryable=false; the validation here only constrains the SUCCESS/NONE combination. Because AsyncPipelineMetrics exports this boolean directly, a future call site can silently make the retryable-failure alert undercount. Derive or validate retryable from FailureType (including WORKER_LEASE_EXPIRED) instead of accepting an independent policy value.

    public EmbeddingJobAttemptMetricEvent {
        Objects.requireNonNull(outcome, "outcome은 필수입니다.");
        Objects.requireNonNull(failureType, "failureType은 필수입니다.");
        if ((outcome == Outcome.SUCCESS) != (failureType == FailureType.NONE)) {
            throw new IllegalArgumentException("성공 결과만 NONE 실패 유형을 사용할 수 있습니다.");

• Files reviewed: 25/25 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate test-coverage findings and one documentation nit remain unresolved.

Review details

Suppressed comments (3)

backend/src/test/java/com/opensource/docgrid/global/observability/AsyncPipelineMetricsAfterCommitIntegrationTest.java:52

• This test only inspects the Counter after executeWithoutResult returns, so it would also pass if the listener were accidentally changed to BEFORE_COMMIT; it does not verify the required post-commit boundary. Assert that the meter is still absent inside the transaction immediately after publishing the event, then retain the existing post-commit assertion.

        transactionTemplate.executeWithoutResult(status ->
            applicationEventPublisher.publishEvent(EmbeddingJobAttemptMetricEvent.success())
        );

docs/test-results/gimin-#332-async-pipeline-transition-metrics.md:10

• When this block is run from the repository root (as the other verification docs do), ./gradlew does not exist; the wrapper is under backend/. Use the repository-root invocation so the documented test command is reproducible.

./gradlew test

monitoring/prometheus/tests/docgrid-pipeline-alerts.test.yml:56

• This positive-case block only checks the alerts at 4m, after the for: 1m window has elapsed; it never asserts that the alert stays inactive while the threshold is first met. Since the PR relies on this duration to suppress transient spikes, add an earlier eval_time with exp_alerts: [] for each alert before retaining the firing assertion.

    alert_rule_test:
      - eval_time: 4m
        alertname: DocGridRagProviderFallbackSpike
        exp_alerts:
          - exp_labels:

• Files reviewed: 25/25 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.