174 lines
18 KiB
Markdown
174 lines
18 KiB
Markdown
# shl_mcp 소스 분석 보고서
|
||
|
||
- 대상: `C:\axhub\shl_mcp`에 있는 현재 로컬 소스(커밋하지 않은 수정분 포함)
|
||
- 범위: `src/main/java` 전체 49개 파일, 설정(`application*.yml`, `logback-spring.xml`), 배포(`Dockerfile`, Helm, `.gitea`), 일부 테스트
|
||
- 기준일: 2026-09-17
|
||
|
||
---
|
||
|
||
## 0. 총평
|
||
|
||
구조와 설계 수준은 높은 편입니다. 판단 근거를 담은 설계 결정 기록(ADR), 계약 테스트, 외부에 보여줄 에러 정리(SafeError), single-flight 갱신, last-good 스냅샷 유지, 요청 전체 제한시간(deadline), 재시도를 도구 annotation 기준으로 제한한 점이 모두 잘 되어 있어요.
|
||
|
||
다만 **운영에 올리기 전에 반드시 고쳐야 할 항목**이 몇 가지 있습니다. 로그, 비밀값, 배포 설정 불일치, 타임아웃 분류 문제입니다. 또 코드는 Portal 기반 다중 route 방식으로 발전했는데, **문서와 Helm 설정은 아직 "배포 하나에 Tool Service 하나"(ADR-0007) 방식에 머물러 있습니다.** 이 차이를 먼저 정리해야 합니다.
|
||
|
||
| 등급 | 개수 | 요약 |
|
||
|---|---|---|
|
||
| 🔴 치명 (운영 전 필수) | 5 | 운영 로그에 DEBUG 본문(개인정보) 기록, API Key 기본값, 배포 설정 불일치, 타임아웃 오분류, 도구 endpoint를 임의 host로 지정 가능 |
|
||
| 🟠 높음 | 7 | 요청 처리 중 동기 갱신으로 지연 발생, 가상 스레드 pinning, 응답 크기 무제한, 이름 하나 중복 시 route 전체 장애, 도구 서버 에러 내용 유실 등 |
|
||
| 🟡 보통 | 9 | 프로토콜 협상, ping 미지원, 사용하지 않는 이벤트, 상태 키 충돌 가능성 등 |
|
||
| 🟢 추가 개발 | 8 | 메트릭, 트레이싱, 서킷브레이커, 동시성 제한, 감사 로그 등 |
|
||
|
||
---
|
||
|
||
## 1. 🔴 치명: 운영 전 반드시 수정
|
||
|
||
### 1-1. 운영에서도 DEBUG 로그로 요청·응답 본문이 그대로 기록됨 (개인정보 및 성능)
|
||
- `logback-spring.xml`에서 `<root level="DEBUG">`가 **profile 구분 없이** 적용됩니다.
|
||
- 그래서 prod에서도 아래 로그가 모두 출력됩니다.
|
||
- `McpExchangeFilter`: `TEMP_MCP_HTTP_REQUEST_BODY` (Agent가 보낸 요청 본문 전체 = 도구 인자)
|
||
- `McpController`: `TEMP_MCP_HTTP_RESPONSE_BODY` (응답 전체 = 도구 결과)
|
||
- `HttpToolClient`: `TEMP_TOOL_HTTP_REQUEST/RESPONSE_BODY`
|
||
- `ToolRegistryService`: 스냅샷 전체, `PortalToolRegistryClient`: Portal registry 전체
|
||
- Spring, Lettuce, JDK HttpClient 같은 라이브러리의 DEBUG 로그까지 포함
|
||
- 보험 업무 도구의 인자와 결과에는 고객정보가 들어갈 가능성이 높아서 **개인정보 로그 유출 위험**이 있습니다.
|
||
- **성능 문제도 있습니다.** `prettyJson(...)`, `toJson(response)`는 `log.debug(...)`의 *인자*로 넘어가기 때문에, 로그 레벨과 상관없이 **매 요청마다 JSON을 파싱하고 pretty-print**합니다. 요청 본문은 한 요청에 3번까지 파싱됩니다(extractMethod, prettyJson, 컨트롤러).
|
||
- **조치**
|
||
1. root는 `INFO`로 두고, `<springProfile name="local,dev">`에서만 `io.shinhanlife` 패키지를 DEBUG로 켭니다.
|
||
2. `TEMP_` 로그는 `if (log.isDebugEnabled())`로 감싸거나 제거합니다.
|
||
3. 본문 로그가 꼭 필요하면 크기 제한과 필드 마스킹(주민번호, 전화번호 등)을 추가합니다.
|
||
|
||
### 1-2. Tool Server API Key가 기본값 `tool-server-key`로 운영에 나감
|
||
- `application.yml`: `api-key: ${TOOL_SERVER_API_KEY:tool-server-key}`
|
||
- `McpProperties.ToolClient.defaults()`에도 같은 값이 하드코딩되어 있습니다.
|
||
- Helm `deployment.yaml`에는 `TOOL_SERVER_API_KEY`를 넣는 부분(Secret 참조)이 **없습니다.** 그래서 운영 Pod가 이 약한 기본값으로 호출합니다.
|
||
- **조치**
|
||
- non-local profile에서는 기본값을 없애고, 값이 없으면 기동이 실패하게 합니다(`@NotBlank`, 기본값 제거).
|
||
- Helm에서 `valueFrom.secretKeyRef`로 주입합니다.
|
||
|
||
### 1-3. 코드 방향(Portal)과 배포 설정(Helm/ADR-0007)이 서로 다름
|
||
- **코드**: `/mcp/{routeKey}` path로 route를 나누고, Portal registry에서 routeKey별 Tool Service 목록을 받는 **다중 route, Portal 방식**이 주력입니다(`PortalToolRegistryClient`가 가장 큰 클래스이고 최근 수정도 집중되어 있음).
|
||
- **Helm**: `discovery.enabled: true` + bundle 1개, Portal 비활성화, `MCP_PORTAL_*`와 `MCP_REDIS_ENABLED` 주입 없음. 즉 ADR-0007 방식입니다.
|
||
- Redis host는 주입하지만 `mcp.redis.enabled` 기본값이 false여서 Redis가 실제로는 쓰이지 않습니다.
|
||
- **CI**: `.gitea/workflows/deploy.yaml`은 `main`에 push하면 `/home/ubuntu/apps/prd-dap-gateway/deploy.sh`를 실행합니다. Helm/OpenShift 배포와는 **별개의 배포 경로**이고, **테스트 없이** 곧바로 운영에 반영됩니다.
|
||
- `Dockerfile`도 `build -x test`로 테스트를 건너뜁니다.
|
||
- `application.yml`의 `spring.profiles.active: local` 때문에 환경변수가 빠지면 **운영에서 local profile로 뜰 수 있습니다.**
|
||
- `mcp.identity` 기본값에 오타가 있습니다: `ax-hu-mcp` → `ax-hub-mcp`
|
||
- **조치**
|
||
- 목표 아키텍처를 하나로 정하고(Portal 방식이면 ADR을 새로 작성), Helm, ConfigMap, 문서를 맞춥니다.
|
||
- CI에 `./gradlew check`를 필수 단계로 추가하고, 운영 배포는 태그나 수동 승인 뒤에만 실행되게 합니다.
|
||
- `spring.profiles.active` 기본값을 제거합니다.
|
||
|
||
### 1-4. 도구 호출 타임아웃이 "TIMEOUT"이 아니라 "NETWORK"로 분류될 가능성이 높음
|
||
- `HttpToolClient.hasTimeoutCause()`는 원인 체인에서 `SocketTimeoutException`만 찾습니다.
|
||
- 그런데 실제로 사용하는 `JdkClientHttpRequestFactory`(JDK HttpClient)는 타임아웃이 나면 **`java.net.http.HttpTimeoutException` / `HttpConnectTimeoutException`**을 던집니다. 이 둘은 `SocketTimeoutException`의 하위 클래스가 아닙니다.
|
||
- 결과적으로 도구 타임아웃이 `TOOL_UNAVAILABLE`(NETWORK)로 보고되고, 모니터링과 Agent 응답에서 원인을 잘못 판단하게 됩니다.
|
||
- `HttpToolClientTest`에 타임아웃 케이스가 없어서 이 문제가 드러나지 않았습니다.
|
||
- **조치**: `instanceof java.net.http.HttpTimeoutException`도 함께 검사합니다. MockWebServer로 응답을 지연시키는 테스트를 추가합니다.
|
||
|
||
### 1-5. Manifest에 절대 URL을 적으면 어느 host로든 도구를 호출함 (API Key와 사용자 헤더 유출 경로)
|
||
- `ToolBundleDiscovery.resolveManifestEndpoint()`는 manifest의 `endpoint`가 **절대 URL이면 base host 검증 없이 그대로 사용**합니다. 테스트 `acceptsAbsoluteToolEndpointDeclaredByTheManifest`로 확인한 의도된 동작입니다.
|
||
- 반면 `application.yml` 주석에는 *"nothing a Tool Service returns can change where MCP sends the call"*이라고 되어 있어서 **설계 의도와 구현이 반대**입니다.
|
||
- Manifest 한 건만 잘못되거나 변조되어도 `X-Tool-Server-API-Key`, `X-Praf-No`, `X-User-Ip` 같은 헤더와 인자가 임의 host로 전송됩니다.
|
||
- **조치**: 절대 URL은 base와 scheme, host, port가 같을 때만 허용합니다. 다른 host가 필요하면 Portal에 등록한 allowlist에 있을 때만 허용합니다.
|
||
|
||
---
|
||
|
||
## 2. 🟠 높음
|
||
|
||
### 2-1. TTL이 만료되면 사용자 요청을 처리하는 도중에 동기로 갱신함 → 주기적인 지연
|
||
- `ToolRegistryService.listTools()` / `findEnabledTool()` → `refreshIfStale()` → `refresh()`로 이어지면서, **tools/list와 tools/call 요청 처리 중에** Portal 조회와 manifest 조회를 동기로 실행합니다.
|
||
- 추가로 `McpExchangeFilter.validateKnownRoute()`에서도 Portal 갱신을 동기로 실행합니다.
|
||
- 5분(TTL)마다 첫 요청이 connect 1초 + read 3초 × Tool Service 수만큼 느려질 수 있습니다.
|
||
- Tool Service가 죽어 있고 메모리 스냅샷도 없으면 TTL마다 사용자가 타임아웃을 그대로 겪습니다.
|
||
- **조치**: stale-while-revalidate 방식으로 바꿉니다. 만료된 스냅샷은 즉시 반환하고, 갱신은 가상 스레드에서 비동기로 한 번만 실행합니다. 연속 실패 시 backoff도 적용합니다.
|
||
|
||
### 2-2. 가상 스레드 + `synchronized` 안의 네트워크 I/O → carrier 스레드 pinning
|
||
- `spring.threads.virtual.enabled: true`이고 런타임은 Java 21입니다. JDK 24 미만에서는 synchronized 안에서 블로킹하면 가상 스레드가 carrier 스레드에 고정됩니다(pinning).
|
||
- `PortalToolRegistryClient.fetchRoutingManifests()`는 `synchronized (refreshLock)` 안에서 **routing manifest를 HTTP로 순차 조회**합니다. 이 코드는 initialize 요청마다 실행됩니다.
|
||
- 동시 요청이 몰리면 carrier 스레드(CPU 코어 수만큼)가 모두 막혀서 **서버 전체가 멈춘 것처럼 보일 수 있습니다.**
|
||
- **조치**: `ReentrantLock`이나 CompletableFuture 기반 single-flight로 바꿉니다(`ToolRegistryService`에서 이미 쓰는 패턴). 또는 JDK 25 LTS로 올립니다.
|
||
|
||
### 2-3. 도구 응답과 Portal 응답 크기에 제한이 없음
|
||
- manifest는 `maxManifestBytes`로 제한하지만, `HttpToolClient`는 `toEntity(String.class)`로 **응답 전체를 메모리에 올립니다.**
|
||
- Portal registry도 `body(JsonNode.class)`로 제한 없이 읽습니다.
|
||
- 대용량 응답이 오면 OOM이 나거나, LLM 컨텍스트가 넘쳐서 Agent 품질이 떨어집니다.
|
||
- **조치**: `exchange()` + `readNBytes(max+1)`로 도구 응답 최대 크기를 설정합니다(예: 1MB). 넘으면 `isError` 결과를 반환하거나 잘라서 반환합니다.
|
||
|
||
### 2-4. 도구 이름 하나만 중복되거나 개수가 초과돼도 route 전체가 불능이 됨
|
||
- `PortalToolRegistryClient.merge()`에서 서비스 간 이름 중복이나 `maxToolsTotal` 초과가 있으면 route 전체를 `TOOL_REGISTRY_UNAVAILABLE`로 처리합니다.
|
||
- Tool Service 하나의 배포 실수가 같은 route의 **모든 도구를 막습니다.**
|
||
- 메모리 스냅샷이 있으면 그 스냅샷으로 버티지만, 신규 Pod은 readiness를 통과하지 못합니다.
|
||
- **조치**: 중복된 도구만 제외하고 경고 로그와 메트릭을 남깁니다. 실패 격리 단위를 route가 아니라 Tool Service로 낮춥니다.
|
||
|
||
### 2-5. 도구 서버 에러 본문이 모두 버려짐 → LLM이 스스로 수정할 수 없음
|
||
- `HttpToolClient`는 4xx/5xx 응답의 본문을 읽지 않습니다. `ToolsCallHandler.failureResult()`는 고정 문구("Tool execution failed.")만 반환합니다.
|
||
- 인자 검증 실패(`TOOL_ARGUMENT_ERROR`)도 `"'custNo' is required"` 같은 상세 내용이 빠진 채 반환됩니다.
|
||
- MCP 스펙이 입력 검증 에러를 `isError` 결과로 돌려주라고 하는 이유가 **모델이 인자를 고쳐서 다시 호출하게 하려는 것**인데, 지금은 그 효과가 없습니다.
|
||
- **조치**
|
||
- 인자 오류는 필드 단위 상세 내용을 포함합니다(스키마 정보라서 민감하지 않음).
|
||
- 도구 서버 4xx 본문 중 합의한 필드(예: `error.message`)만 제한된 길이로 전달합니다.
|
||
|
||
### 2-6. 요청을 받을 때 Portal 장애가 "등록되지 않은 route" 에러로 보임
|
||
- 기동 시 Portal에 실패하고 Redis fallback도 없으면 `bundlesByRoute`가 비어 있습니다.
|
||
- `refreshSourceRegistry()`의 `finally`가 시도 시각을 기록하기 때문에, 이후 TTL(300초) 동안 모든 요청이 `"route key is not registered"`(INVALID_REQUEST)로 거부됩니다.
|
||
- 실제 원인은 레지스트리 장애인데 클라이언트 오류로 보여서 **장애 원인 파악을 방해합니다.**
|
||
- **조치**: 레지스트리를 아직 불러오지 못한 상태면 `TOOL_REGISTRY_UNAVAILABLE`로 응답합니다. 실패 시에는 짧은 backoff(예: 5~10초)로 다시 시도합니다.
|
||
|
||
### 2-7. `ToolBundleDiscovery`의 상태 키가 `bundle.id`(serviceKey)만 사용함
|
||
- `states` map의 키가 serviceKey뿐입니다. 같은 serviceKey를 **서로 다른 route에서 다른 serviceDomain으로** 쓰면 last-good 스냅샷이 섞입니다.
|
||
- 이 경우 한 route의 장애 fallback이 **다른 도메인의 endpoint를 가진 도구 목록**을 반환할 수 있습니다.
|
||
- **조치**: 키를 `routeKey + serviceKey + serviceDomain`으로 바꿉니다.
|
||
|
||
---
|
||
|
||
## 3. 🟡 보통
|
||
|
||
1. **프로토콜 버전 협상**: `InitializeHandler`는 클라이언트가 요청한 `protocolVersion`을 보지 않고 항상 preferredVersion을 반환합니다. 지금은 지원 버전이 하나라 문제가 없지만, 버전을 추가하는 순간 스펙 위반이 됩니다. 요청 버전을 지원하면 그 버전을 반환하도록 바꿉니다.
|
||
2. **`ping` 메서드 미지원**: MCP의 `ping` 요청에 METHOD_NOT_FOUND를 반환합니다. 빈 결과(`{}`)를 반환하는 handler를 추가합니다(5줄 정도).
|
||
3. **`ToolListChangedEvent`를 받는 곳이 없음**: 이벤트를 발행하지만 listener도 SSE도 없어서 사용되지 않는 코드입니다. capabilities도 `listChanged:false`입니다. 쓸 계획이 없으면 제거합니다.
|
||
4. **`Mcp-Session-Id`**: initialize마다 UUID를 발급하지만 검증하거나 저장하지 않습니다. stateless 방침(ADR-0001)이라면 발급 자체를 생략하거나, 발급하는 이유를 문서에 명시합니다.
|
||
5. **Origin 헤더 검증 없음**: Streamable HTTP 스펙은 Origin 검증을 요구합니다(DNS rebinding 방지). 내부망이고 NetworkPolicy가 있어서 위험은 낮지만 간단한 allowlist를 추가하는 것을 권장합니다.
|
||
6. **도구 이름 규칙**: `[A-Za-z0-9_./-]{1,64}`이 `/`와 `..`을 허용하고, `exactEndpoint=false`일 때는 이름이 endpoint path에 그대로 붙습니다(`ToolRoutingService`). 이름은 manifest가 정하므로 신뢰 경계 안이지만 `..`은 막는 게 좋습니다. 최신 스펙 권장 문자셋은 `[A-Za-z0-9_.-]`, 최대 128자입니다.
|
||
7. **HTTP/2 업그레이드**: JDK HttpClient는 기본값이 HTTP/2라서 `http://` 호출 시 h2c Upgrade 헤더를 보냅니다. 오래된 WAS나 프록시에서 POST 본문 처리 문제가 날 수 있으니 `.version(HttpClient.Version.HTTP_1_1)`로 고정하는 것을 권장합니다.
|
||
8. **요청마다 RestClient 새로 생성**: `HttpToolClient.clientFor()`가 호출마다 factory와 RestClient를 만듭니다. 지금 부하에서는 문제가 없지만, timeout별로 캐시하거나 요청 단위 timeout으로 바꾸면 좋습니다.
|
||
9. **`claimStaleRefreshSlot` 경쟁 조건**: `ToolExecutionService`에서 get과 put이 원자적이지 않습니다. `ToolRegistryService`처럼 `compute`를 쓰면 됩니다. 영향은 작습니다.
|
||
10. **`CachedBodyHttpServletRequest`**: 잘못된 charset이 오면 `Charset.forName`이 예외를 던집니다. 기본값을 UTF-8로 처리합니다.
|
||
11. **`searchTime` meta**: 값에 재시도와 backoff 시간까지 포함되고 이름도 의미와 맞지 않습니다. `durationMillis`와 `attempts`로 나누는 것을 권장합니다.
|
||
12. **Portal 방식의 bundle 상태 조회 불가**: `ToolBundleStatusEndpoint`는 `mcp.bundles`(정적 설정)만 보여줘서 Portal 방식에서는 항상 비어 있습니다. route별 서비스 상태를 조회할 수 있게 확장합니다.
|
||
|
||
---
|
||
|
||
## 4. 🟢 추가 개발 권장
|
||
|
||
| 우선순위 | 항목 | 내용 |
|
||
|---|---|---|
|
||
| 1 | **메트릭** | `micrometer-registry-prometheus`를 추가합니다. 도구별 호출 수, 지연(p95/p99), 에러 유형, 재시도 수, 레지스트리 갱신 성공/실패, 스냅샷 경과 시간을 수집합니다. 지금은 health와 info만 노출됩니다. |
|
||
| 2 | **분산 트레이싱** | Micrometer Tracing + OTel을 붙입니다. `X-Guid`를 trace와 연결해서 Agent Builder → MCP → Tool Service 전 구간을 추적합니다. |
|
||
| 3 | **서킷브레이커 / 동시성 제한** | Tool Service별로 Resilience4j CircuitBreaker와 Bulkhead를 적용합니다. 느린 서비스 하나가 가상 스레드와 커넥션을 모두 차지하지 못하게 합니다. |
|
||
| 4 | **감사 로그** | tools/call마다 누가(agentId, prafNo), 무엇을(toolName, version), 결과와 소요 시간을 본문 없이 구조화 로그로 따로 남깁니다. 금융권 감사 대응용입니다. |
|
||
| 5 | **Rate limit** | route나 agentId 단위로 호출량을 제한합니다(LLM 반복 호출 폭주 방지). |
|
||
| 6 | **structuredContent / outputSchema** | 결과를 텍스트뿐 아니라 `structuredContent`로도 반환하고, manifest의 `outputSchema`를 tools/list에 노출합니다. |
|
||
| 7 | **JSON 로그 인코더** | 지금 key=value 텍스트 형식을 logstash-logback-encoder 같은 JSON 형식으로 바꿔 수집과 검색을 쉽게 합니다. |
|
||
| 8 | **통합 테스트** | Testcontainers(Redis) + MockWebServer(Portal, Tool)로 Portal 장애, Redis fallback, TTL 갱신, 타임아웃 시나리오를 end-to-end로 검증합니다. |
|
||
|
||
---
|
||
|
||
## 5. 권장 진행 순서
|
||
|
||
1. **즉시 (반나절)**: 1-1 로그 레벨과 TEMP 로그, 1-2 API Key, 1-3 profile 기본값과 identity 오타, 1-4 타임아웃 분류 + 테스트
|
||
2. **이번 스프린트**: 1-5 endpoint host 검증, 2-2 pinning, 2-3 응답 크기 제한, 2-5 에러 상세 전달, CI 테스트 게이트
|
||
3. **다음 스프린트**: 2-1 비동기 갱신, 2-4 실패 격리, 2-6, 2-7, 아키텍처(Portal vs ADR-0007) 결정 후 Helm과 문서 정리
|
||
4. **이후**: 메트릭, 트레이싱, 서킷브레이커, 감사 로그
|
||
|
||
---
|
||
|
||
## 부록: 잘 된 점 (유지 권장)
|
||
|
||
- SafeError / PublicErrorMapper로 내부 예외 메시지가 외부로 새지 않게 막은 구조
|
||
- 요청 전체 제한시간(270초, Agent Builder 300초보다 먼저 종료)과 남은 시간 기준 timeout 계산
|
||
- `idempotentHint`/`readOnlyHint` 기반 재시도 허용과 404/410 수신 시 즉시 레지스트리 갱신
|
||
- single-flight 갱신, last-good 스냅샷, Redis를 캐시로만 쓰는 원칙
|
||
- graceful shutdown 40초 < terminationGracePeriod 45초
|
||
- 패키지 경계, 코드 스타일, Helm 배포 계약을 테스트로 강제하는 방식
|