feat: 행동 로그를 analytics_events 테이블에 적재 + 유입 전환율 추적#235
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesAnalytics 이벤트 DB 적재 전환
Sequence DiagramsequenceDiagram
participant Client
participant TrackingController
participant AnalyticsEventLogger
participant AnalyticsEventRepository
participant analytics_events_DB
Client->>TrackingController: GET /api/track/visit?utm_source=...
TrackingController->>TrackingController: UtmAttribution.of() 생성
TrackingController->>TrackingController: utmCookie.writeFirstTouch()
TrackingController->>AnalyticsEventLogger: log(landing_visited 이벤트 + UTM 속성)
AnalyticsEventLogger->>AnalyticsEventLogger: HTTP 컨텍스트에서 userId/platform 추출
AnalyticsEventLogger->>AnalyticsEventLogger: properties JSON 직렬화
AnalyticsEventLogger->>AnalyticsEventRepository: save(AnalyticsEventRecord)
AnalyticsEventRepository->>analytics_events_DB: INSERT 1건
TrackingController->>Client: 응답
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/ject/vs/analytics/TrackingController.java (1)
31-46:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUTM 입력 길이 상한 검증이 없어 DB/로그 적재 경로가 과도하게 커질 수 있습니다.
/api/track/visit는 외부 입력(utm_*)을 길이 제한 없이 받아 쿠키/analytics 저장 경로로 전달합니다. 비정상적으로 긴 쿼리스트링이 반복 유입되면properties(TEXT) 적재량이 급증해 저장소/처리량에 영향을 줄 수 있습니다. 컨트롤러에서 필드별 최대 길이(예: 100~255) 제한 또는 절단을 적용해 주세요.🤖 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 `@src/main/java/com/ject/vs/analytics/TrackingController.java` around lines 31 - 46, The endpoint is accepting UTM parameters (source, medium, campaign, content) without any length validation, which can lead to excessively large data being stored in cookies and analytics logs. Add validation logic before the UtmAttribution.of() call to enforce maximum length limits on each utm_* parameter (e.g., 100-255 characters). Either reject requests that exceed these limits or truncate the values to the maximum allowed length before passing them to utmCookie.writeFirstTouch() and analytics.log().
🧹 Nitpick comments (1)
src/main/resources/db/migration/V15__add_analytics_events.sql (1)
12-12: ⚡ Quick win
TIMESTAMP를TIMESTAMP WITH TIME ZONE으로 변경 검토 필요.
Instant는 UTC 기반이지만 PostgreSQL의TIMESTAMP(without time zone)는 타임존 정보 없이 저장됩니다. JDBC 드라이버/Hibernate 설정에 따라 저장·조회 시 암묵적 변환이 발생해 혼란을 초래할 수 있습니다.
TIMESTAMPTZ를 사용하면 명시적으로 UTC로 저장되어 직접 DB 쿼리 시에도 일관성이 유지됩니다.🔧 권장 수정
- occurred_at TIMESTAMP NOT NULL, + occurred_at TIMESTAMPTZ NOT NULL,🤖 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 `@src/main/resources/db/migration/V15__add_analytics_events.sql` at line 12, The occurred_at column is defined as TIMESTAMP without timezone information, but the Java code uses Instant which is UTC-based. This mismatch can cause implicit conversion issues depending on JDBC driver and Hibernate settings. Change the occurred_at column definition from TIMESTAMP to TIMESTAMP WITH TIME ZONE (or TIMESTAMPTZ) to explicitly store UTC values and ensure consistency across all database interactions and queries.
🤖 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
`@src/integrationTest/java/com/ject/vs/analytics/AnalyticsEventLoggerIntegrationTest.java`:
- Line 50: The code directly accesses findAll().get(0) without first validating
that results were returned, which reduces test reliability by failing to
distinguish between data load failures and empty results. At both line 50 and
line 60 in the AnalyticsEventLoggerIntegrationTest class, add an assertion to
verify the list has the expected size using hasSize(1) or singleElement() before
accessing the element at index 0, ensuring clear test failure messages when data
is not properly loaded or unexpectedly present.
---
Outside diff comments:
In `@src/main/java/com/ject/vs/analytics/TrackingController.java`:
- Around line 31-46: The endpoint is accepting UTM parameters (source, medium,
campaign, content) without any length validation, which can lead to excessively
large data being stored in cookies and analytics logs. Add validation logic
before the UtmAttribution.of() call to enforce maximum length limits on each
utm_* parameter (e.g., 100-255 characters). Either reject requests that exceed
these limits or truncate the values to the maximum allowed length before passing
them to utmCookie.writeFirstTouch() and analytics.log().
---
Nitpick comments:
In `@src/main/resources/db/migration/V15__add_analytics_events.sql`:
- Line 12: The occurred_at column is defined as TIMESTAMP without timezone
information, but the Java code uses Instant which is UTC-based. This mismatch
can cause implicit conversion issues depending on JDBC driver and Hibernate
settings. Change the occurred_at column definition from TIMESTAMP to TIMESTAMP
WITH TIME ZONE (or TIMESTAMPTZ) to explicitly store UTC values and ensure
consistency across all database interactions and queries.
🪄 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: fcffd372-d3a6-44a8-898d-ee576e6e5ea4
📒 Files selected for processing (6)
src/integrationTest/java/com/ject/vs/analytics/AnalyticsEventLoggerIntegrationTest.javasrc/main/java/com/ject/vs/analytics/AnalyticsEventLogger.javasrc/main/java/com/ject/vs/analytics/AnalyticsEventRecord.javasrc/main/java/com/ject/vs/analytics/AnalyticsEventRepository.javasrc/main/java/com/ject/vs/analytics/TrackingController.javasrc/main/resources/db/migration/V15__add_analytics_events.sql
| .userId(1024L) | ||
| .put("method", "kakao")); | ||
|
|
||
| AnalyticsEventRecord row = analyticsEventRepository.findAll().get(0); |
There was a problem hiding this comment.
행 수 검증 없이 findAll().get(0)에 직접 접근하면 테스트 신뢰성이 떨어집니다.
Line 50, Line 60에서 먼저 hasSize(1)(또는 singleElement())를 검증한 뒤 행을 꺼내야, 적재 실패와 데이터 오염을 명확히 구분할 수 있습니다.
제안 수정안
- AnalyticsEventRecord row = analyticsEventRepository.findAll().get(0);
+ List<AnalyticsEventRecord> rows = analyticsEventRepository.findAll();
+ assertThat(rows).hasSize(1);
+ AnalyticsEventRecord row = rows.get(0);
assertThat(row.getUserId()).isEqualTo(1024L);
assertThat(row.isMember()).isTrue();
assertThat(row.getProperties()).contains("\"method\":\"kakao\"");
@@
- AnalyticsEventRecord row = analyticsEventRepository.findAll().get(0);
+ List<AnalyticsEventRecord> rows = analyticsEventRepository.findAll();
+ assertThat(rows).hasSize(1);
+ AnalyticsEventRecord row = rows.get(0);
assertThat(row.getEvent()).isEqualTo("simple_event");
assertThat(row.getProperties()).isNull();Also applies to: 60-60
🤖 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
`@src/integrationTest/java/com/ject/vs/analytics/AnalyticsEventLoggerIntegrationTest.java`
at line 50, The code directly accesses findAll().get(0) without first validating
that results were returned, which reduces test reliability by failing to
distinguish between data load failures and empty results. At both line 50 and
line 60 in the AnalyticsEventLoggerIntegrationTest class, add an assertion to
verify the list has the expected size using hasSize(1) or singleElement() before
accessing the element at index 0, ensuring clear test failure messages when data
is not properly loaded or unexpectedly present.
|
빌드 실패 |
|
빌드 실패 |
|
빌드 성공 |
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.
📌 관련 이슈
🔍 작업 내용
행동 로그(analytics event)를 stdout 로그가 아니라 RDB(
analytics_events) 테이블에 직접 INSERT하도록 바꿨습니다. 이제 별도 로그 수집기 없이 SQL로 바로 집계/조회할 수 있습니다. 또한 UTM 유입 클릭(landing_visited)을 적재해 가입 전환율까지 계산할 수 있게 했습니다.📝 변경 사항
V15__add_analytics_events.sql—analytics_events테이블(append-only, FK 없음,(event, occurred_at)·(occurred_at)인덱스)AnalyticsEventRecord— 매핑 엔티티. 공통 필드는 컬럼, 이벤트별 가변 속성은properties(JSON 문자열) 컬럼AnalyticsEventRepository—JpaRepositoryAnalyticsEventLogger가 JSON 로그 출력 대신analytics_events에 INSERT. 적재 실패는 그대로 삼켜 비즈니스 로직에 영향 없음TrackingController가/api/track/visit호출 시landing_visited이벤트 1건 적재 (전환율 분모)AnalyticsEventLoggerIntegrationTest— 실제 PostgreSQL(Testcontainers)로 적재/properties직렬화/회원여부 검증 (3건)💬 리뷰어에게
properties컬럼 타입: 로컬 H2(PostgreSQL 모드)와 운영 PostgreSQL 양쪽 호환을 위해jsonb가 아닌 **TEXT(JSON 문자열)**로 저장합니다. PostgreSQL에선(properties::jsonb)->>'utm_campaign'으로 캐스팅해 쿼리 가능합니다.@Async분리가 후속 과제입니다.ddl-auto: validate) + 적재 테스트 3건 통과 확인했습니다.landing_visited는 프론트가 랜딩 시/api/track/visit를credentials:'include'로 호출해줘야 쌓입니다(기존 UTM 추적과 동일 전제).Summary by CodeRabbit
릴리스 노트
New Features
/api/track/visit호출 시 UTM 정보를 바탕으로landing_visited분석 이벤트가 기록됩니다.properties가 비워집니다.Tests