Conversation
WalkthroughAmplitude 기반 메트릭 수집 기능(Phase 1–2)을 추가합니다. KST 날짜 유틸리티와 config 검증(TZ=Asia/Seoul), MetricSourceAdapter 타입·토큰, Amplitude HTTP 클라이언트(요청/타임아웃/체크섬/민감값 필터), Amplitude 어댑터(주간만 지원), 스냅샷 저장소·서비스(재귀 민감키 검증, upsert) 및 관련 테스트·문서·모듈 연결이 포함됩니다. ChangesAmplitude Adapter (Phase 1-2)
sequenceDiagram
participant Caller
participant Adapter as AmplitudeMetricSourceAdapter
participant Client as AmplitudeClient
participant Amplitude as Amplitude API
participant Service as SnapshotsService
participant Repository as SnapshotsRepository
participant Database as Prisma/DB
Caller->>Adapter: collect(definition, period)
alt period type not WEEKLY
Adapter-->>Caller: MetricCollectionFailure(UNSUPPORTED_PERIOD_TYPE)
else
Adapter->>Adapter: parseAmplitudeEventCountQuerySpec()
alt parse fails
Adapter-->>Caller: MetricCollectionFailure(UNSUPPORTED_QUERY_SPEC)
else
Adapter->>Client: fetchEventSegmentation(startDate, endDate)
alt API error
Client-->>Adapter: AmplitudeApiError (sanitized)
Adapter-->>Caller: MetricCollectionFailure(AMPLITUDE_API_ERROR)
else success
Client->>Client: compute SHA-256 checksum
Client-->>Adapter: {body, rawRef}
Adapter->>Adapter: extract value from seriesCollapsed
alt invalid response
Adapter-->>Caller: MetricCollectionFailure(AMPLITUDE_RESPONSE_INVALID)
else valid
Adapter-->>Caller: MetricCollectionSuccess
end
end
end
end
Caller->>Service: saveCollectionResult(result)
alt result.status === 'failed'
Service-->>Caller: {status: 'skipped', reason: 'COLLECTION_FAILED'}
else status === 'success'
Service->>Repository: upsertCollectionSuccess(result)
Repository->>Repository: validateRawRef (scan for sensitive keys)
alt sensitive key found
Repository-->>Service: Error
Service-->>Caller: Exception
else no sensitive data
Repository->>Database: metricSnapshot.upsert(...)
Database-->>Repository: MetricSnapshot
Repository-->>Service: MetricSnapshot
Service-->>Caller: {status: 'saved', snapshot}
end
end
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Poem
개요이 변경사항은 Amplitude 외부 API에서 메트릭을 수집하고, 수집 결과를 데이터베이스에 저장하는 메트릭 소스 어댑터 아키텍처를 구현합니다. KST 기반 날짜 처리, 구성 스키마 확장, 핵심 타입 정의, Amplitude 클라이언트 및 어댑터, 그리고 스냅샷 저장소/서비스를 포함합니다. 변경사항
시퀀스 다이어그램sequenceDiagram
participant Caller
participant Adapter as AmplitudeMetricSourceAdapter
participant Client as AmplitudeClient
participant Amplitude as Amplitude API
participant Service as SnapshotsService
participant Repository as SnapshotsRepository
participant Database as Prisma/DB
Caller->>Adapter: collect(definition, period)
alt period type not WEEKLY
Adapter-->>Caller: MetricCollectionFailure(UNSUPPORTED_PERIOD_TYPE)
else
Adapter->>Adapter: parseAmplitudeEventCountQuerySpec()
alt parse fails
Adapter-->>Caller: MetricCollectionFailure(UNSUPPORTED_QUERY_SPEC)
else
Adapter->>Client: fetchEventSegmentation(startDate, endDate)
alt API error
Client-->>Adapter: AmplitudeApiError (sanitized)
Adapter-->>Caller: MetricCollectionFailure(AMPLITUDE_API_ERROR)
else success
Client->>Client: compute SHA-256 checksum
Client-->>Adapter: {body, rawRef}
Adapter->>Adapter: extract value from seriesCollapsed
alt invalid response
Adapter-->>Caller: MetricCollectionFailure(AMPLITUDE_RESPONSE_INVALID)
else valid
Adapter-->>Caller: MetricCollectionSuccess
end
end
end
end
Caller->>Service: saveCollectionResult(result)
alt result.status === 'failed'
Service-->>Caller: {status: 'skipped', reason: 'COLLECTION_FAILED'}
else status === 'success'
Service->>Repository: upsertCollectionSuccess(result)
Repository->>Repository: validateRawRef (scan for sensitive keys)
alt sensitive key found
Repository-->>Service: Error
Service-->>Caller: Exception
else no sensitive data
Repository->>Database: metricSnapshot.upsert(...)
Database-->>Repository: MetricSnapshot
Repository-->>Service: MetricSnapshot
Service-->>Caller: {status: 'saved', snapshot}
end
end
예상 코드 리뷰 노력🎯 4 (복잡함) | ⏱️ ~45분 시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/modules/metric-sources/core/metric-source-adapter.ts (1)
1-13: 코어 계약에서 Prisma 타입 의존을 분리하는 것을 권장합니다.
core레이어 인터페이스가generated/prisma타입을 직접 참조하면, 소스 어댑터 계약이 영속성 구현 세부사항에 결합됩니다. 코어 전용 입력 타입(예: 최소 필드 DTO)을 두고 매핑을 호출부에서 처리하면 향후 소스/저장소 교체가 더 쉬워집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/metric-sources/core/metric-source-adapter.ts` around lines 1 - 13, The MetricSourceAdapter interface currently depends on the Prisma-generated MetricDefinition type, coupling core contracts to persistence details; create a core DTO (e.g., MetricDefinitionCore or MetricDefinitionDTO) that contains only the minimal fields needed by adapters, change the collect signature on MetricSourceAdapter.collect to accept that DTO instead of MetricDefinition, and update adapter implementations and callers to map between generated/prisma MetricDefinition and the new core DTO at the application boundary (e.g., in repository/adapter factory code) so the core layer no longer imports generated/prisma types.src/modules/metric-sources/amplitude/amplitude-query-spec.spec.ts (1)
14-59:eventType비허용값 거부 케이스도 테스트에 고정해두는 것을 권장합니다.현재 케이스들은 충분히 좋지만, Phase 범위(
timer_started만 허용)를 테스트로 명시하면 파서 완화 회귀를 더 확실히 막을 수 있습니다.테스트 보강 예시
describe('Amplitude querySpec 검증', () => { @@ test('filters 또는 groupBy가 비어 있지 않으면 거부한다', () => { @@ }); + + test('timer_started가 아닌 eventType은 거부한다', () => { + expect(() => + parseAmplitudeEventCountQuerySpec({ + ...validQuerySpec, + eventType: 'session_start', + }), + ).toThrow('Unsupported Amplitude querySpec'); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/metric-sources/amplitude/amplitude-query-spec.spec.ts` around lines 14 - 59, Add a negative test that explicitly rejects disallowed eventType values to prevent parser relaxation: in the amplitude-query-spec.spec.ts tests for parseAmplitudeEventCountQuerySpec (using validQuerySpec), add an assertion that passing an eventType other than 'timer_started' (e.g., 'timer_ended' or '') throws 'Unsupported Amplitude querySpec'. This uses the existing test pattern (expect(() => parseAmplitudeEventCountQuerySpec({...validQuerySpec, eventType: '...'})).toThrow(...)) so the parser remains strict about only allowing the 'timer_started' eventType.src/modules/metric-sources/amplitude/amplitude-metric-source.adapter.spec.ts (1)
141-154: “형식 오류” 테스트에 구조 손상 케이스도 추가하는 것을 권장합니다.현재 Line 141-154는
value타입 오류만 다룹니다.seriesCollapsed가 배열이 아닌 경우까지 포함하면, 응답 스키마 변경 시 0으로 오인 처리되는 회귀를 더 잘 막을 수 있습니다.🧪 제안 테스트 케이스
+ test('seriesCollapsed 구조가 비정상이면 실패 처리한다', async () => { + const client = createClientMock({ + body: { data: { seriesCollapsed: 'not-an-array' } }, + rawRef, + } as unknown as AmplitudeSegmentationResult); + const adapter = new AmplitudeMetricSourceAdapter(client); + + const result = await adapter.collect(definition, period); + + expect(result).toMatchObject({ + status: 'failed', + reasonCode: 'AMPLITUDE_RESPONSE_INVALID', + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/metric-sources/amplitude/amplitude-metric-source.adapter.spec.ts` around lines 141 - 154, Add a new unit test to the existing suite that asserts AmplitudeMetricSourceAdapter.collect returns a failed result with reasonCode 'AMPLITUDE_RESPONSE_INVALID' when the Amplitude response's seriesCollapsed is not an array (e.g., seriesCollapsed: null or a plain object); use the same test setup utilities (createClientMock, AmplitudeSegmentationResult, rawRef, definition, period) and the AmplitudeMetricSourceAdapter.collect call to produce the invalid response scenario and assert the expected failure, mirroring the existing malformed-value test to cover the structural corruption case.src/modules/metric-sources/core/metric-collection.types.ts (1)
14-24:core계약이 이미 Amplitude 전용으로 굳어져 있습니다.
MetricSnapshotRawRef,MetricCollectionSuccess,MetricCollectionFailure가'AMPLITUDE'와 단일 endpoint literal을 직접 들고 있어서 다음 source를 추가할 때 adapter만 붙이는 대신 core 타입/저장 계층까지 같이 수정하게 됩니다.MetricSource별칭과 source별rawRefunion(or generic)으로 한 단계 분리해 두면, PR 목표였던 “새 원천 추가 시 기존 코드 변경 최소화”에 더 잘 맞습니다.Also applies to: 35-38, 74-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/metric-sources/core/metric-collection.types.ts` around lines 14 - 24, The core types are hard-coded to Amplitude (MetricSnapshotRawRef, MetricCollectionSuccess, MetricCollectionFailure) which forces core changes for new sources; introduce a MetricSource alias/union (e.g., 'AMPLITUDE' | 'OTHER') and refactor MetricSnapshotRawRef into a generic or discriminated union keyed by MetricSource so source and endpoint literals move into per-source adapters (e.g., MetricSnapshotRawRef<T extends MetricSource> or a RawRefMap union), then update MetricCollectionSuccess and MetricCollectionFailure to reference the generic/discriminated raw-ref instead of fixed literals so adding a new source only requires adding a new per-source raw-ref type and adapter without changing core storage types; apply the same pattern to the other analogous types that currently embed AMPLITUDE literals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 19: 현재 .gitignore에 있는 "specs/" 전체 제외는 범위가 과도하므로 "specs/" 항목을 제거하고 대신 로컬
전용 또는 임시 파일만 좁게 무시하도록 패턴화하세요; 예를 들어 "specs/" 전체 대신 특정 패턴 (예: specs/*.local,
specs/*.tmp, specs/*.bak, specs/*.swp 등)만 추가하여 문서(명세) 파일이 버전 관리에서 누락되지 않도록 하세요 —
즉 .gitignore에서 "specs/" 항목을 삭제하고 필요한 로컬/임시 파일 패턴으로 대체하십시오.
In `@src/common/time/kst-date.ts`:
- Around line 10-27: Add input validation to guard against invalid Date objects:
in formatKstDate(date: Date) check if date.getTime() is NaN and throw a
TypeError (with a clear message referencing formatKstDate), and similarly
validate periodEnd in formatAmplitudeEndDateFromExclusiveEnd(periodEnd: Date)
(or call a small shared validateDate helper used by both). This prevents
producing bogus strings when date inputs are invalid; keep existing logic using
KST_OFFSET_MILLISECONDS and ONE_MILLISECOND unchanged.
In `@src/modules/metric-sources/amplitude/amplitude-metric-source.adapter.ts`:
- Around line 135-143: The current guards incorrectly treat malformed responses
as valid zero values; update the checks around seriesCollapsed and firstSeries
so that when !isUnknownArray(seriesCollapsed) or seriesCollapsed.length === 0,
and when !isUnknownArray(firstSeries) or firstSeries.length === 0, the function
returns { isValid: false } (and omit or set value to undefined) instead of {
isValid: true, value: 0 }; adjust the code paths using this function to skip
snapshot/aggregation on isValid === false and optionally add a
processLogger.warn (or similar) noting the broken Amplitude response for easier
debugging.
In `@src/modules/metric-sources/amplitude/amplitude-query-spec.ts`:
- Around line 34-47: The current isSupported check only validates required field
values but allows extra keys on querySpec; update the validation in
amplitude-query-spec.ts to enforce an exact shape by verifying
Object.keys(querySpec) (or a helper like validateExactKeys) matches the allowed
set for AMPLITUDE EVENT_COUNT queries (e.g.,
['version','source','kind','eventType','aggregation','filters','groupBy']), and
reject if any unexpected keys exist before using isSupported; alternatively use
a small JSON-schema/strict-parse helper to ensure no extra properties are
present when evaluating querySpec.
In `@src/modules/metric-sources/amplitude/amplitude.client.ts`:
- Around line 123-135: The checksum is computed from JSON.stringify(body) after
calling response.json(), which loses original byte fidelity and skips JSON parse
errors; change the logic in the function that builds the
AmplitudeSegmentationResponse/rawRef so you first call response.text() to
capture the rawResponseText, compute the sha256 over that raw text using
createHash, then attempt JSON.parse(rawResponseText) inside a try/catch and on
parse failure throw or rethrow an AmplitudeApiError with context (include
response.headers.get('x-amplitude-request-id') if present); ensure the returned
body is the parsed AmplitudeSegmentationResponse and rawRef.responseChecksum
uses the hash of rawResponseText rather than JSON.stringify(body).
- Around line 94-102: The fetch in amplitude.client.ts (inside the block using
this.amplitudeFetch) lacks a timeout and can hang; wrap the call with an
AbortController, pass controller.signal into the this.amplitudeFetch options,
start a setTimeout that calls controller.abort() after a chosen timeout (e.g.,
configurable DEFAULT_AMPLITUDE_TIMEOUT), and clear the timer when the request
completes; in the catch path detect abort errors (error.name === 'AbortError' or
similar) and rethrow an AmplitudeApiError so aborts are converted to
AmplitudeApiError; ensure the timer is always cleared (finally) to avoid leaks
and make the timeout value configurable via the class or method options.
In `@src/modules/metric-sources/metric-sources.module.ts`:
- Around line 9-11: The provider registering AMPLITUDE_FETCH currently uses
useValue: fetch which will crash on Node <18; update the provider so it
guarantees a runtime-safe fetch instead of assuming global fetch: either add an
engines entry ("node": ">=18") to package.json, add and initialize a polyfill
(e.g., install and import node-fetch or cross-fetch and use that), or change the
provider implementation to use a runtime fallback (e.g., use globalThis.fetch if
defined otherwise require/import a node-fetch fallback) so AMPLITUDE_FETCH
always receives a valid function.
In `@src/modules/snapshots/snapshots.repository.ts`:
- Around line 49-81: The current sensitive key set in
assertRawRefDoesNotContainSensitiveKeys/containsSensitiveKey is too narrow (only
'apikey','secretkey','authorization') and misses variants like api_key,
accessToken, refresh-token, password, etc.; update the detection to normalize
each object key (lowercase and strip non-alphanumeric characters or convert
camelCase to delimiter form) and then match against a broader list and/or
patterns (e.g., tokens: 'token', 'access', 'refresh'; keys:
'api','key','secret'; auth/password variants:
'auth','authorization','password','pass','cred') or use substring/regex matching
rather than exact equality so keys like api_key, apiKey, refresh-token,
accessToken are caught; apply this logic inside containsSensitiveKey when
iterating Object.entries and keep throwing the same Error in
assertRawRefDoesNotContainSensitiveKeys when a match is found.
---
Nitpick comments:
In
`@src/modules/metric-sources/amplitude/amplitude-metric-source.adapter.spec.ts`:
- Around line 141-154: Add a new unit test to the existing suite that asserts
AmplitudeMetricSourceAdapter.collect returns a failed result with reasonCode
'AMPLITUDE_RESPONSE_INVALID' when the Amplitude response's seriesCollapsed is
not an array (e.g., seriesCollapsed: null or a plain object); use the same test
setup utilities (createClientMock, AmplitudeSegmentationResult, rawRef,
definition, period) and the AmplitudeMetricSourceAdapter.collect call to produce
the invalid response scenario and assert the expected failure, mirroring the
existing malformed-value test to cover the structural corruption case.
In `@src/modules/metric-sources/amplitude/amplitude-query-spec.spec.ts`:
- Around line 14-59: Add a negative test that explicitly rejects disallowed
eventType values to prevent parser relaxation: in the
amplitude-query-spec.spec.ts tests for parseAmplitudeEventCountQuerySpec (using
validQuerySpec), add an assertion that passing an eventType other than
'timer_started' (e.g., 'timer_ended' or '') throws 'Unsupported Amplitude
querySpec'. This uses the existing test pattern (expect(() =>
parseAmplitudeEventCountQuerySpec({...validQuerySpec, eventType:
'...'})).toThrow(...)) so the parser remains strict about only allowing the
'timer_started' eventType.
In `@src/modules/metric-sources/core/metric-collection.types.ts`:
- Around line 14-24: The core types are hard-coded to Amplitude
(MetricSnapshotRawRef, MetricCollectionSuccess, MetricCollectionFailure) which
forces core changes for new sources; introduce a MetricSource alias/union (e.g.,
'AMPLITUDE' | 'OTHER') and refactor MetricSnapshotRawRef into a generic or
discriminated union keyed by MetricSource so source and endpoint literals move
into per-source adapters (e.g., MetricSnapshotRawRef<T extends MetricSource> or
a RawRefMap union), then update MetricCollectionSuccess and
MetricCollectionFailure to reference the generic/discriminated raw-ref instead
of fixed literals so adding a new source only requires adding a new per-source
raw-ref type and adapter without changing core storage types; apply the same
pattern to the other analogous types that currently embed AMPLITUDE literals.
In `@src/modules/metric-sources/core/metric-source-adapter.ts`:
- Around line 1-13: The MetricSourceAdapter interface currently depends on the
Prisma-generated MetricDefinition type, coupling core contracts to persistence
details; create a core DTO (e.g., MetricDefinitionCore or MetricDefinitionDTO)
that contains only the minimal fields needed by adapters, change the collect
signature on MetricSourceAdapter.collect to accept that DTO instead of
MetricDefinition, and update adapter implementations and callers to map between
generated/prisma MetricDefinition and the new core DTO at the application
boundary (e.g., in repository/adapter factory code) so the core layer no longer
imports generated/prisma types.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f58a197-b6a3-41cd-86ed-020b40e70675
📒 Files selected for processing (26)
.gitignoredocs/README.mddocs/learning-and-portfolio.mddocs/learning/README.mddocs/learning/architecture/2026-04-27-layered-boundaries.mddocs/learning/testing/2026-04-28-tdd-external-api-isolation.mdsrc/app.module.tssrc/common/time/kst-date.spec.tssrc/common/time/kst-date.tssrc/modules/config/config.schema.spec.tssrc/modules/config/config.schema.tssrc/modules/metric-sources/amplitude/amplitude-metric-source.adapter.spec.tssrc/modules/metric-sources/amplitude/amplitude-metric-source.adapter.tssrc/modules/metric-sources/amplitude/amplitude-query-spec.spec.tssrc/modules/metric-sources/amplitude/amplitude-query-spec.tssrc/modules/metric-sources/amplitude/amplitude.client.spec.tssrc/modules/metric-sources/amplitude/amplitude.client.tssrc/modules/metric-sources/core/metric-collection.types.tssrc/modules/metric-sources/core/metric-source-adapter.tssrc/modules/metric-sources/core/metric-source.tokens.tssrc/modules/metric-sources/metric-sources.module.tssrc/modules/snapshots/snapshots.module.tssrc/modules/snapshots/snapshots.repository.spec.tssrc/modules/snapshots/snapshots.repository.tssrc/modules/snapshots/snapshots.service.spec.tssrc/modules/snapshots/snapshots.service.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specs/feat/007-amplitude-adapter/spec.md (1)
64-65: ⚡ Quick winKST 경계 처리에 대한 “고정 예시”를 요구사항/성공기준에 1개 추가해 주세요.
현재 문구만으로도 의도는 맞지만, UTC↔KST 경계(주간 시작/종료, exclusive end)를 숫자 예시 1건으로 고정하면 테스트 해석 차이를 줄일 수 있습니다. 예:
periodEnd=2026-04-19T15:00:00.000Z일 때 Amplitude end date가20260419로 계산되는지 등.Also applies to: 112-113
🤖 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 `@specs/feat/007-amplitude-adapter/spec.md` around lines 64 - 65, 요구사항의 KST 경계 해석을 명확히 하기 위해 한 건의 고정 예시를 추가하세요: 예를 들어 periodEnd=2026-04-19T15:00:00.000Z(UTC)일 때 시스템은 KST 기준 종결일을 사용해 Amplitude end date를 20260419로 계산한다는 문구를 성공기준에 삽입하고(즉, UTC로 저장되는 시작·종료 시각은 그대로 유지하되 해석은 KST이며 end는 exclusive 규칙을 적용), 동일한 예시를 명세의 대응 섹션(현재 112-113에 해당하는 위치)에도 복제하여 테스트 해석 차이를 없애세요.
🤖 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.
Nitpick comments:
In `@specs/feat/007-amplitude-adapter/spec.md`:
- Around line 64-65: 요구사항의 KST 경계 해석을 명확히 하기 위해 한 건의 고정 예시를 추가하세요: 예를 들어
periodEnd=2026-04-19T15:00:00.000Z(UTC)일 때 시스템은 KST 기준 종결일을 사용해 Amplitude end
date를 20260419로 계산한다는 문구를 성공기준에 삽입하고(즉, UTC로 저장되는 시작·종료 시각은 그대로 유지하되 해석은 KST이며
end는 exclusive 규칙을 적용), 동일한 예시를 명세의 대응 섹션(현재 112-113에 해당하는 위치)에도 복제하여 테스트 해석 차이를
없애세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0287d4cb-dea6-41a0-853e-55bcf1de4bba
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.gitignorepackage.jsonspecs/feat/007-amplitude-adapter/checklists/requirements.mdspecs/feat/007-amplitude-adapter/contracts/metric-source-adapter.mdspecs/feat/007-amplitude-adapter/contracts/snapshot-persistence.mdspecs/feat/007-amplitude-adapter/data-model.mdspecs/feat/007-amplitude-adapter/plan.mdspecs/feat/007-amplitude-adapter/research.mdspecs/feat/007-amplitude-adapter/spec.mdspecs/feat/007-amplitude-adapter/tasks.mdspecs/feat/007-amplitude-adapter/test-contracts/amplitude-adapter.mdspecs/feat/007-amplitude-adapter/test-contracts/amplitude-client.mdspecs/feat/007-amplitude-adapter/test-contracts/kst-date.mdspecs/feat/007-amplitude-adapter/test-contracts/snapshots.mdsrc/common/time/kst-date.spec.tssrc/common/time/kst-date.tssrc/modules/metric-sources/amplitude/amplitude-metric-source.adapter.spec.tssrc/modules/metric-sources/amplitude/amplitude-metric-source.adapter.tssrc/modules/metric-sources/amplitude/amplitude-query-spec.spec.tssrc/modules/metric-sources/amplitude/amplitude-query-spec.tssrc/modules/metric-sources/amplitude/amplitude.client.spec.tssrc/modules/metric-sources/amplitude/amplitude.client.tssrc/modules/snapshots/snapshots.repository.spec.tssrc/modules/snapshots/snapshots.repository.ts
✅ Files skipped from review due to trivial changes (15)
- package.json
- specs/feat/007-amplitude-adapter/research.md
- src/common/time/kst-date.ts
- .gitignore
- specs/feat/007-amplitude-adapter/test-contracts/kst-date.md
- specs/feat/007-amplitude-adapter/data-model.md
- specs/feat/007-amplitude-adapter/plan.md
- specs/feat/007-amplitude-adapter/test-contracts/amplitude-adapter.md
- src/common/time/kst-date.spec.ts
- specs/feat/007-amplitude-adapter/tasks.md
- specs/feat/007-amplitude-adapter/checklists/requirements.md
- specs/feat/007-amplitude-adapter/test-contracts/snapshots.md
- specs/feat/007-amplitude-adapter/contracts/snapshot-persistence.md
- specs/feat/007-amplitude-adapter/contracts/metric-source-adapter.md
- src/modules/metric-sources/amplitude/amplitude-metric-source.adapter.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/modules/snapshots/snapshots.repository.spec.ts
- src/modules/metric-sources/amplitude/amplitude-query-spec.spec.ts
- src/modules/metric-sources/amplitude/amplitude-query-spec.ts
- src/modules/snapshots/snapshots.repository.ts
- src/modules/metric-sources/amplitude/amplitude.client.spec.ts
- src/modules/metric-sources/amplitude/amplitude.client.ts
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.
관련 이슈
closes #7
개요
Phase 1-1에서 잡아둔 골격 위에, 이번 단계에서는 외부 지표를 처음으로 우리 DB에 끌어다 놓는 파이프라인을 만들었습니다. 구체적으로는 Amplitude에서
timer_started이벤트 발생 횟수를 주간 단위로 가져와metric_snapshots테이블에 저장하는 흐름까지입니다.이번 단계의 핵심은 "Amplitude를 빠르게 붙이는 것" 자체가 아니라, 이후에 Sentry 같은 다른 원천이 추가돼도 기존 코드를 건드리지 않을 수 있는 경계를 먼저 만드는 것이었습니다. Phase 9에서
SentryErrorSourceAdapter가 들어올 때 이 경계가 그대로 재사용되도록 설계 우선순위를 잡았습니다.전체 흐름은 이렇게 동작합니다:
이 구조에서 한 클래스는 한 가지 일만 합니다. Amplitude 응답 형식이 바뀌어도 저장 코드는 흔들리지 않고, 중복 저장 정책을 바꿔도 Amplitude 코드를 건드릴 필요가 없습니다.
설계 판단
1.
MetricSourceAdapter인터페이스를 먼저 정의했다가장 빨리 만드는 길은
AmplitudeService하나에 호출, 변환, 저장을 모두 우겨넣는 것입니다. 그런데 이 프로젝트는 Phase 9에 Sentry 에러 지표를, 그 뒤로도 새 원천을 받아들일 가능성이 큽니다. 그때마다 기존 클래스를 뜯어 고치는 건 위험합니다.그래서 외부 원천에서 값을 가져오는 행위를 다음과 같은 계약으로 추상화했습니다:
AmplitudeMetricSourceAdapter는 이 계약의 첫 구현체일 뿐입니다. 새 원천이 들어올 때는 이 인터페이스를 한 번 더 구현하면 됩니다. 호출하는 쪽(이후 Phase 1-3의 RunsService)은 인터페이스만 알면 되기 때문에, 구현이 무엇인지에 대한 의존이 생기지 않습니다.DI 등록은
Symbol토큰으로 했습니다:문자열 토큰을 쓰지 않은 이유는 두 가지입니다. 첫째, 문자열은 오타가 나도 컴파일이 통과합니다. 둘째, 다른 모듈에서 같은 문자열을 우연히 쓰면 충돌이 납니다.
Symbol은 import해서 쓰는 유일한 참조라 이 두 문제가 사라집니다.2. 외부 호출과 도메인 변환을 분리했다 (Client ↔ Adapter)
Amplitude 통신을 두 클래스로 쪼갰습니다.
AmplitudeClientAmplitudeMetricSourceAdapterMetricDefinition→ API 입력 변환, 결과 → 우리 도메인 결과로 변환이렇게 나눈 이유는, HTTP 세부사항이 바뀌는 빈도와 도메인 규칙이 바뀌는 빈도가 다르기 때문입니다. Amplitude가 API path를 v3로 바꾸면 Client만 고치면 되고, 우리가 "주간 시작 요일"을 바꾸면 Adapter만 고치면 됩니다.
테스트 관점에서도 효과가 큽니다. fetch 함수 자체를 DI 토큰으로 주입받게 만들었기 때문에:
테스트에서는 가짜 fetch를 넣어 네트워크 없이도 Client 동작을 검증할 수 있습니다. 실수로 진짜 Amplitude로 요청이 나가는 사고를 구조적으로 막아주는 장치입니다.
3. 결과 타입을 Discriminated Union으로 만들었다
처음에는 실패 시
throw를 던지는 방식도 고려했습니다. 하지만 Phase 1-2의 정책은 명확합니다 — 한 지표가 실패해도 전체 실행은 멈추면 안 됩니다. Phase 1-3 이후 여러 지표를 동시에 돌릴 때, 한 곳의 예외가 다른 지표 수집까지 죽이면 곤란합니다.그래서
collect()는 예외를 던지지 않고 항상 결과 객체를 반환합니다:status필드 하나로 분기되도록 만들어서 호출자는 이렇게 쓰면 됩니다:실패 원인은 자유 문자열이 아니라 정해진 코드(
UNSUPPORTED_PERIOD_TYPE,UNSUPPORTED_QUERY_SPEC,AMPLITUDE_API_ERROR,AMPLITUDE_RESPONSE_INVALID)로 분류했습니다. 이렇게 하면 나중에 "API 오류는 재시도, querySpec 오류는 알림"처럼 정책을 바꿀 때 코드 매칭으로 분기할 수 있습니다.4.
querySpec을 DB의 JSON 필드로 두고, 런타임에 좁게 검증한다MetricDefinition.querySpec은 Prisma JSON 필드입니다. DB는 어떤 모양이든 받아주지만, Phase 1-2에서 우리가 실제로 처리할 수 있는 모양은 매우 좁습니다:이 모양이 아니면
parseAmplitudeEventCountQuerySpec이 던지고, Adapter는UNSUPPORTED_QUERY_SPEC으로 실패를 반환합니다. "좁게 시작하고, 명세에 합의된 만큼만 확장한다" 원칙을 코드로 박아둔 셈입니다.이 검증 함수는 unknown 입력에서 시작합니다. DB JSON은 TypeScript가 모양을 보장해주지 않기 때문에, "object인지 → 각 필드가 정확히 그 값인지" 순으로 단계적으로 좁혀나가서 마지막에야 정의된 타입으로 좁힙니다.
5. credential이 새지 않도록 3중 방어선
Amplitude는 Basic Auth(API Key + Secret Key)로 호출합니다. 이 값들이 로그, error message, DB의
rawRef어디에도 남아서는 안 됩니다. 그래서 보호 장치를 여러 층으로 깔았습니다:AmplitudeClient의sanitizeMessageAmplitudeApiError.toJSON()JSON.stringify(error)시name,message,status만 노출SnapshotsRepository의assertRawRefDoesNotContainSensitiveKeysapiKey/secretKey/authorization키가 들어있으면 저장 자체를 막음3차 방어선이 좀 과해 보일 수 있는데, 외부 API 계층의 실수가 DB 영구 저장으로 이어지는 사고를 막기 위한 마지막 안전장치입니다. 한번 DB에 들어간 credential은 회수하기가 정말 어렵습니다.
rawRef에는 응답 본문 대신sha256:...checksum과 Amplitude의x-amplitude-request-id만 저장합니다. 추후 디버깅 시 "이 스냅샷이 어떤 응답에서 왔는지"는 추적할 수 있되, 응답 본문 자체는 남기지 않는 절충점입니다.6. KST 변환 로직을
common/time에 모았다이 프로젝트는 모든 시간 계산이
Asia/Seoul기준입니다. 그런데 Date 객체 자체는 UTC 기반이라 변환이 자주 필요합니다. 변환 코드를 여기저기 흩어놓으면 한 군데서 잘못 만지는 순간 데이터가 하루씩 밀립니다.그래서
formatKstDate,formatAmplitudeEndDateFromExclusiveEnd두 함수만common/time/kst-date.ts에 두고, 다른 모듈은 무조건 이 함수들을 통해서만 변환하도록 했습니다.formatAmplitudeEndDateFromExclusiveEnd가 살짝 흥미로운 부분입니다. 우리 DB는periodEnd를 exclusive(이 시각은 포함 안 함)로 저장하는데, Amplitude의end파라미터는 inclusive입니다. 그래서 1ms를 빼서 "마지막 포함 날짜"를 만들어 넘깁니다. 이걸 빼먹으면 주간 경계에서 하루가 어긋납니다.7.
metric_snapshots는upsert로 저장한다스케줄러가 같은 주에 두 번 돌거나, 수동으로 재실행할 때 같은 스냅샷이 중복으로 들어가면 안 됩니다. 그래서 저장은 항상 compound unique key 기준의
upsert입니다:update: {}로 둔 건 의도적입니다. 이미 저장된 스냅샷의 값은 절대 덮어쓰지 않습니다. Amplitude 응답이 시간이 지나면서 미세하게 흔들릴 수 있는데, 한 번 기록한 값은 그 시점의 fact로 유지하는 게 비교·분석의 일관성을 위해 더 안전하다고 판단했습니다. 만약 정말로 다시 받아야 한다면 명시적인 재수집 기능을 따로 만들 예정입니다.8.
TZ=Asia/Seoul환경변수를 필수로 검증한다코드 레벨에서 KST를 잘 다루더라도, Node 프로세스 자체의
TZ가 다르면Date출력이나pino로그 타임스탬프가 어긋납니다. 이건 잡기 정말 어려운 종류의 버그라서, ConfigSchema에 다음 한 줄을 추가했습니다:다른 값이면 앱이 부팅조차 안 됩니다. "운영자가 컨테이너에 TZ를 잊어먹어서 KST가 아닌 채로 한 주가 흘러갔다"는 사고를 부팅 시점에 막는 장치입니다.
추가로 정비한 것
코드 외에 학습 아카이브 운영 방식도 이번 PR에 함께 넣었습니다 (
docs/learning/). 학습 노트를 모든 대화마다 남기는 게 아니라, 면접/포트폴리오/후속 구현에서 다시 설명할 가치가 있을 때만 정리하는 흐름으로 정했습니다. 이번 단계에서 뽑은 두 개의 노트:architecture/2026-04-27-layered-boundaries.md— Adapter ↔ Repository 경계 분리의 이유testing/2026-04-28-tdd-external-api-isolation.md— 외부 API를 DI fetch로 격리해 테스트하는 방법specs/는 로컬 작업 산출물이므로.gitignore에 추가했습니다.브랜치 타입
feat/– 신규 기능fix/– 버그 수정hotfix/– 프로덕션 긴급 수정refactor/– 기능 변경 없는 코드 개선docs/– 문서 작성·수정test/– 테스트 추가·수정chore/– 의존성·설정·도구 관련ci/– GitHub Actions·CI/CD 설정체크리스트
[TYPE] 작업 제목형식인가?{type}/#{issue_number}형식인가?develop)Summary by CodeRabbit
Documentation
New Features
Tests
Chores