Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughFirebase Analytics 연동을 위한 추상화와 구현체를 추가합니다. AnalyticsService/AnalyticsRepository 프로토콜, Firebase 기반 서비스(actor), 저장소(actor), 의존성 주입용 클라이언트 및 이벤트/파라미터 타입들이 새로 도입됩니다. (50 words 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant Feature as Feature Module
participant Client as AnalyticsClient
participant Repository as DefaultAnalyticsRepository
participant Service as FirebaseAnalyticsService
participant Firebase as Firebase Analytics SDK
Feature->>Client: configure(userID: Int?)
activate Client
Client->>Client: Task.detached(.background)
Client->>Repository: setUserSession(with: Int?)
activate Repository
Repository->>Repository: Int? -> String?
Repository->>Service: setUserID(_ String?)
activate Service
Service->>Firebase: Analytics.setUserID(userID)
Firebase-->>Service: ✓
Service-->>Repository: ✓
deactivate Service
Repository-->>Client: ✓
deactivate Repository
Client-->>Feature: ✓
deactivate Client
Feature->>Client: logEvent(_ event)
activate Client
Client->>Client: Task.detached(.background)
Client->>Repository: logEvent(_ event)
activate Repository
Repository->>Repository: extract name & parameters
Repository->>Service: sendEvent(name: String, parameters: [String: Any]?)
activate Service
Service->>Firebase: Analytics.logEvent(name, parameters)
Firebase-->>Service: ✓
Service-->>Repository: ✓
deactivate Service
Repository-->>Client: ✓
deactivate Repository
Client-->>Feature: ✓
deactivate Client
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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
🧹 Nitpick comments (2)
Neki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Implementations/DefaultAnalyticsRepository.swift (1)
26-37: 파라미터 매핑 로직 간소화 가능현재 for 루프를 사용한 파라미터 변환 로직을
reduce(into:)를 사용하여 더 간결하게 작성할 수 있습니다.♻️ 간소화 제안
public func logEvent(_ event: any AnalyticsEvent) async { let eventName = event.name.value - var parsedParameters: [String: Any]? - - if let parameters = event.parameters { - parsedParameters = [:] - for (key, value) in parameters { - parsedParameters?[key.value] = value - } - } + let parsedParameters = event.parameters?.reduce(into: [String: Any]()) { result, pair in + result[pair.key.value] = pair.value + } await service.sendEvent(name: eventName, parameters: parsedParameters) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Implementations/DefaultAnalyticsRepository.swift` around lines 26 - 37, In logEvent, simplify the manual for-loop that maps event.parameters into parsedParameters by using reduce(into:) to build the [String: Any] dictionary; locate the logic in the logEvent(_ event: any AnalyticsEvent) method where parsedParameters is created from event.parameters and replace the for (key, value) loop with a reduce(into:) that assigns parsedParameters?[key.value] = value, then continue to call service.sendEvent(name: eventName, parameters: parsedParameters) as before.Neki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Clients/AnalyticsClient.swift (1)
22-26: Fire-and-forget 패턴의 이벤트 손실 가능성 고려
Task.detached로 생성된 태스크가 추적되지 않아, 앱이 갑자기 종료되거나 백그라운드로 전환될 때 이벤트가 손실될 수 있습니다. 또한 실패한 이벤트에 대한 에러 핸들링이 없습니다.분석 이벤트의 특성상 일부 손실은 허용 가능할 수 있으나, 중요한 이벤트의 경우 재시도 로직이나 로컬 큐잉을 고려해볼 수 있습니다. 현재 구현이 의도된 것이라면 이 부분은 무시하셔도 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Clients/AnalyticsClient.swift` around lines 22 - 26, The current fire-and-forget usage of Task.detached in the AnalyticsClient initializer (the userID handler calling repository.setUserSession and the logEvent handler calling repository.logEvent) risks losing events and provides no error handling; replace Task.detached with structured Task (so the tasks are attached to the current context) or otherwise enqueue the work to your existing analytics queue mechanism, and wrap the await calls to repository.setUserSession(...) and repository.logEvent(...) with proper error handling (catch errors and log them via your logger and/or implement a simple retry/local-queue fallback for failed events) so failures are recorded and important events can be retried.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Neki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Entities/AnalyticsParamterKey.swift`:
- Line 10: Rename the misspelled public enum AnalyticsParamterKey to
AnalyticsParameterKey and rename the file AnalyticsParamterKey.swift →
AnalyticsParameterKey.swift; update all references (e.g., usages in
AnalyticsEvent.swift) to the new name. To avoid breaking downstream consumers,
add a deprecated typealias AnalyticsParamterKey = AnalyticsParameterKey (marked
`@available`(*, deprecated, message: "...") ) in the old-named file or a
compatibility shim and update any exports/imports accordingly. Ensure tests and
any public API docs are updated to the corrected name.
---
Nitpick comments:
In
`@Neki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Implementations/DefaultAnalyticsRepository.swift`:
- Around line 26-37: In logEvent, simplify the manual for-loop that maps
event.parameters into parsedParameters by using reduce(into:) to build the
[String: Any] dictionary; locate the logic in the logEvent(_ event: any
AnalyticsEvent) method where parsedParameters is created from event.parameters
and replace the for (key, value) loop with a reduce(into:) that assigns
parsedParameters?[key.value] = value, then continue to call
service.sendEvent(name: eventName, parameters: parsedParameters) as before.
In
`@Neki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Clients/AnalyticsClient.swift`:
- Around line 22-26: The current fire-and-forget usage of Task.detached in the
AnalyticsClient initializer (the userID handler calling
repository.setUserSession and the logEvent handler calling repository.logEvent)
risks losing events and provides no error handling; replace Task.detached with
structured Task (so the tasks are attached to the current context) or otherwise
enqueue the work to your existing analytics queue mechanism, and wrap the await
calls to repository.setUserSession(...) and repository.logEvent(...) with proper
error handling (catch errors and log them via your logger and/or implement a
simple retry/local-queue fallback for failed events) so failures are recorded
and important events can be retried.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2124919b-9df2-4a91-b578-eee1e94f664f
📒 Files selected for processing (8)
Neki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Implementations/DefaultAnalyticsRepository.swiftNeki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Implementations/FirebaseAnalyticsService.swiftNeki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Interfaces/AnalyticsRepository.swiftNeki-iOS/Core/Sources/Analytics/Sources/Data/Sources/Interfaces/AnalyticsService.swiftNeki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Clients/AnalyticsClient.swiftNeki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Entities/AnalyticsEvent.swiftNeki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Entities/AnalyticsEventName.swiftNeki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Entities/AnalyticsParamterKey.swift
|
|
||
| import Foundation | ||
|
|
||
| public enum AnalyticsParamterKey: String { |
There was a problem hiding this comment.
오타 수정 필요: "Paramter" → "Parameter"
파일명과 enum 이름에 "Parameter"가 "Paramter"로 잘못 작성되어 있습니다. public API이므로 다른 모듈에서 사용되기 전에 수정하는 것이 좋습니다.
✏️ 수정 제안
파일명: AnalyticsParamterKey.swift → AnalyticsParameterKey.swift
-public enum AnalyticsParamterKey: String {
+public enum AnalyticsParameterKey: String {관련 파일 (AnalyticsEvent.swift)도 함께 수정 필요:
- var parameters: [AnalyticsParamterKey: Any]? { get }
+ var parameters: [AnalyticsParameterKey: Any]? { get }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public enum AnalyticsParamterKey: String { | |
| public enum AnalyticsParameterKey: String { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Core/Sources/Analytics/Sources/Domain/Sources/Entities/AnalyticsParamterKey.swift`
at line 10, Rename the misspelled public enum AnalyticsParamterKey to
AnalyticsParameterKey and rename the file AnalyticsParamterKey.swift →
AnalyticsParameterKey.swift; update all references (e.g., usages in
AnalyticsEvent.swift) to the new name. To avoid breaking downstream consumers,
add a deprecated typealias AnalyticsParamterKey = AnalyticsParameterKey (marked
`@available`(*, deprecated, message: "...") ) in the old-named file or a
compatibility shim and update any exports/imports accordingly. Ensure tests and
any public API docs are updated to the corrected name.
OneTen19
left a comment
OneTen19
left a comment
There was a problem hiding this comment.
GA는 저도 처음 붙여봐서 궁금한 것들이 조금 있네요!!
POP-DIP
너무 좋습니다. 역시 고능하시군여
logEvent의 파라미터로 userID를 매번 넘기지 않고, 최초 초기화 용도의 configure 메서드를 별도로 유지했습니다. 각 하위 기능인 지도, 아카이빙, 포즈추천 등이 유저의 로그인 상태를 억지로 알아야 하는, 강결합을 방지하고자 했습니다.
최초 초기화로 userID를 지니고 캐싱을 해둔다는 뜻인 것 같은데 userID를 어딘가에 저장해두는 부분은 따로 보이지 않네요. 그렇다면 Firebase는 한 번 설정된 식별값을 유지한다고 합니다. 이 내용이 Firebase가 한 번 연결이 되면 알아서 userID를 계속 지니고 그 값을 사용한다는 의미로 해석이 되는데, 이로 인해서 이벤트를 수집할 때 userID가 따로 필요 없나보군요!!
그런데 POP-DIP에서 고려하신 것처럼 추후 다른 수집도구를 붙이게 되고, 해당 도구는 이벤트마다 userID가 요구된다면 지금의 구조에서 조금 달라지게 되겠네요.
이제 각 Reducer에 어떻게 붙이고 이벤트 수집을 해야 좋을지 생각하는 것이 좋을 것 같습니다.
진짜 어떻게 붙이지.
Firebase Analytics 라이브러리의 Analytics라는 타입은 thread-safety한지 공식문서 설명을 못찾았어서(내부 Queue로 직렬화하도록 설계되어 있다는 스택오버플로우 글이 있긴하다만) 혹시 몰라서 actor로 선언했습니다.
굳. 느릴지언정 혹시 모를 문제를 예방한다. actor로 사용할 때 우려되는 단점이 약간 궁금하긴 하네요.
| var name: AnalyticsEventName { get } | ||
| var parameters: [AnalyticsParamterKey: Any]? { get } |
There was a problem hiding this comment.
프로토콜 선언부에 저장 프로퍼티 식 선언도 가능한가요?
|
|
||
| import Foundation | ||
|
|
||
| public enum AnalyticsEventName: String { |
There was a problem hiding this comment.
p3
이 파일을 보고 문득 든 궁금증인데, 이런 식으로 이벤트마다 엔티티를 만들어서 분석하는 게 일반적인 방식일까요??
우리처럼 규모가 크지 않은 프로젝트는 이 파일처럼 모든 이벤트를 만들어두고, 추가될 때마다 추가하는 식으로 진행한다 하더라도, 규모가 큰 프로젝트나 실무같은 곳에서도 이런 식으로 할 지 궁금하네요
일일이 이벤트마다 엔티티를 만들고, 필요한 곳에 전부 붙인다?? 이벤트가 수백개가 되어도 그게 가능하려나??? 문뜩 든 의문..
There was a problem hiding this comment.
만약 수집하고자 하는 이벤트 종류가 굉장히 많아져서 하나의 열거형으로 표현하기 어려울 경우에는 Nested Type을 추가로 만들 것 같긴합니다. (네임스페이스가 주는 장점을 유지하고, 구분하기 쉽도록 분리)
enum AnalyticsEvent {
enum Pose {
case randomPose
// ....
}
}이런 식이면 이벤트 전송하는 코드에서는 AnalyticsEvent.Pose.randomPose 같은 식으로 작성할 수 있어서 가독성 떨어지는 문제를 해결할 수 있을듯해요.
| case brandName = "brand_name" | ||
| case entryPoint = "entry_point" | ||
| case mapType = "map_type" |
There was a problem hiding this comment.
p3
노션을 확인해보니 파라미터의 구체적인 value 값이 없는 것 같더라구요?
value값을 클라단에서 정해서 붙이게 되면 안드로이드와 iOS의 value값이 다르게 수집될 가능성이 있을 것 같아요.
그래서 기획단에게 가능하다면 구체적인 Value Case도 같이 제공해달라고 요구하는 게 좋을 것 같아요.
|
|
||
| public protocol AnalyticsEvent { | ||
| var name: AnalyticsEventName { get } | ||
| var parameters: [AnalyticsParamterKey: Any]? { get } |
There was a problem hiding this comment.
Any로 해둔 건 어떤 자료형으로 수집할 지 아직 안 정해져서 그런건가
There was a problem hiding this comment.
수집도구에는 Int, String 등등 다양한 자료형이 전달될 수 있습니다. 그걸 염두하여 Any 타입으로 표현했습니다.
실제로 Firebase Analytics의 이벤트 수집 메서드도 동일한 파라미터 타입을 받습니다!
| Task.detached(priority: .background) { await repository.setUserSession(with: userID) } | ||
| } logEvent: { event in | ||
| Task.detached(priority: .background) { await repository.logEvent(event) } |
There was a problem hiding this comment.
p3
메인스레드에서 할 필요가 없는 작업이니 Task.detached 사용 좋네요
| func sendEvent(name: String, parameters: [String: Any]?) async | ||
| func setUserID(_ userID: String?) async |
There was a problem hiding this comment.
p3
그냥 궁금해졌는데 AnalyticsRepository처럼 -> Void로 함수가 Void를 리턴한다고 명시해두는 것과
지금 AnalyticsService처럼 그냥 별 표시 안 하는 거랑 무슨 차이가 있을까요??
There was a problem hiding this comment.
아무 차이 없습니다.
Swift 문법일 뿐이지 않을까요.
이를테면, 클로저 블럭 내에 코드가 단 한줄이라면 return 키워드를 생략할 수 있는데 작성해도되고, 안해도되는 것과 같아요.
만약 (너무 극단적이지만) 우리 팀은 모든 코드에 return 키워드를 꼭 써! 라고 강제하는 미친 팀이 있다면 return을 써야겠지만요.
| import Foundation | ||
|
|
||
| public protocol AnalyticsRepository: Sendable { | ||
| func setUserSession(with userID: Int?) async -> Void |
There was a problem hiding this comment.
p3
혹시 여기는 _ 로 안하고 with로 해 둔 이유가???
There was a problem hiding this comment.
함수이름이 setUserSession, '사용자 로그인 정보를 설정한다'는 의미로 네이밍했습니다.
함수 호출부에 로그인 정보 설정은 사용자 식별자로 한다는 뜻을 전달하면서 with랑 by랑 여러 매개변수 레이블 네이밍을 고르다가 그냥 with로 했습니다.
언더바로 매개변수 레이블을 감추면 의미 전달이 어려울 것 같았어요
| #if DEBUG | ||
| Logger.data.log("[Firebase Service] Event: \(name) | Parameters: \(parameters ?? [:])") | ||
| #endif |
There was a problem hiding this comment.
p3
디버깅용으로 사용하는 거면 Logger.data.debug로 #if DEBUG 대신 사용하셔도 될 것 같슴다. level을 debug로 해줘도 되고!!
아니면 콘솔에도 찍히게는 하고 싶은데, 키 값이라던지 내용은 감춰지면 좋겠으면 privacy 설정을 private으로 해 주는 방법도 있겠네요
| } | ||
|
|
||
| public func setUserID(_ userID: String?) { | ||
| Analytics.setUserID(userID) |
There was a problem hiding this comment.
p3
Analytics를 선언한 곳이 안 보이는 걸 보니 FirebaseAnalytics에서 제공하는 것 같은데, 옵셔널인 userID를 사용해도 되는 걸 보면 Analytics.setUserID 이 null을 허용하는 메소드인가보네요?? setUserID인데 옵셔널을 허용한다니 먼가 신기하군
There was a problem hiding this comment.
Analytics에 기본적으로 구현되어 있는 자동수집 로직 떄문에 사용자를 식별할 수 있는 값으로 먼저 기기정보를 쓴다고 합니다. 얘는 앱 런치 시점에 확보가 되는 것 같고요.
그 후에 개발자가 직접 부여한 식별자(우리한테는 지금 UserID Int값)로 사용자를 구별하려거든 setUserID 메서드를 쓰라는데 이러면 그 이후부터는 기기정보가 아닌 개발자가 부여한 값이 식별자로써 로깅이 됩니다.
제 생각에는 아마도 위처럼 식별자가 부여된 상황에서 nil을 전달해 다시 호출하면 그것을 지우는 효과일듯해요
| if let parameters = event.parameters { | ||
| parsedParameters = [:] | ||
| for (key, value) in parameters { | ||
| parsedParameters?[key.value] = value |
There was a problem hiding this comment.
p3
딕셔너리[key.value] = value ???
이벤트가 AnalyticsEvent이고,
AnalyticsEvent는 name(AnalyticsEventName)과 parameters([AnalyticsParamterKey: Any]?)가 있고,
그럼 if let parameters = event.parameters 이거는 parameters가 [AnalyticsParamterKey: Any]? 이거가 되고,
AnalyticsParamterKey의 value값이 parsedParameters 딕셔너리의 Key 값이 되는 거군.
오키 파악완료
There was a problem hiding this comment.
이거 안그래도 약간 멈칫했는데, 열거형의 rawValue를 열거형의 바깥에서 직접 쓰기 싫어서 value라고 네이밍했는데, 덜 헷갈리는 측면에서 적절한 네이밍 다른거 없을까요..?
There was a problem hiding this comment.
아니면 저번 피알에서 리뷰해주신 내용처럼
let 어쩌구 = key.value 하는 과정을 추가해서 파악하기 좀 더 쉽게 해주는 것두..?? 불필요한 선언이 추가돼서 성능상 별로이려나
| var name: AnalyticsEventName { get } | ||
| var parameters: [AnalyticsParamterKey: Any]? { get } |
There was a problem hiding this comment.
프로토콜 선언부에 저장 프로퍼티 식 선언도 가능한가요?
|
|
||
| public protocol AnalyticsEvent { | ||
| var name: AnalyticsEventName { get } | ||
| var parameters: [AnalyticsParamterKey: Any]? { get } |
There was a problem hiding this comment.
수집도구에는 Int, String 등등 다양한 자료형이 전달될 수 있습니다. 그걸 염두하여 Any 타입으로 표현했습니다.
실제로 Firebase Analytics의 이벤트 수집 메서드도 동일한 파라미터 타입을 받습니다!
|
|
||
| import Foundation | ||
|
|
||
| public enum AnalyticsEventName: String { |
There was a problem hiding this comment.
만약 수집하고자 하는 이벤트 종류가 굉장히 많아져서 하나의 열거형으로 표현하기 어려울 경우에는 Nested Type을 추가로 만들 것 같긴합니다. (네임스페이스가 주는 장점을 유지하고, 구분하기 쉽도록 분리)
enum AnalyticsEvent {
enum Pose {
case randomPose
// ....
}
}이런 식이면 이벤트 전송하는 코드에서는 AnalyticsEvent.Pose.randomPose 같은 식으로 작성할 수 있어서 가독성 떨어지는 문제를 해결할 수 있을듯해요.
| func sendEvent(name: String, parameters: [String: Any]?) async | ||
| func setUserID(_ userID: String?) async |
There was a problem hiding this comment.
아무 차이 없습니다.
Swift 문법일 뿐이지 않을까요.
이를테면, 클로저 블럭 내에 코드가 단 한줄이라면 return 키워드를 생략할 수 있는데 작성해도되고, 안해도되는 것과 같아요.
만약 (너무 극단적이지만) 우리 팀은 모든 코드에 return 키워드를 꼭 써! 라고 강제하는 미친 팀이 있다면 return을 써야겠지만요.
| import Foundation | ||
|
|
||
| public protocol AnalyticsRepository: Sendable { | ||
| func setUserSession(with userID: Int?) async -> Void |
There was a problem hiding this comment.
함수이름이 setUserSession, '사용자 로그인 정보를 설정한다'는 의미로 네이밍했습니다.
함수 호출부에 로그인 정보 설정은 사용자 식별자로 한다는 뜻을 전달하면서 with랑 by랑 여러 매개변수 레이블 네이밍을 고르다가 그냥 with로 했습니다.
언더바로 매개변수 레이블을 감추면 의미 전달이 어려울 것 같았어요
| #if DEBUG | ||
| Logger.data.log("[Firebase Service] Event: \(name) | Parameters: \(parameters ?? [:])") | ||
| #endif |
| } | ||
|
|
||
| public func setUserID(_ userID: String?) { | ||
| Analytics.setUserID(userID) |
There was a problem hiding this comment.
Analytics에 기본적으로 구현되어 있는 자동수집 로직 떄문에 사용자를 식별할 수 있는 값으로 먼저 기기정보를 쓴다고 합니다. 얘는 앱 런치 시점에 확보가 되는 것 같고요.
그 후에 개발자가 직접 부여한 식별자(우리한테는 지금 UserID Int값)로 사용자를 구별하려거든 setUserID 메서드를 쓰라는데 이러면 그 이후부터는 기기정보가 아닌 개발자가 부여한 값이 식별자로써 로깅이 됩니다.
제 생각에는 아마도 위처럼 식별자가 부여된 상황에서 nil을 전달해 다시 호출하면 그것을 지우는 효과일듯해요
| if let parameters = event.parameters { | ||
| parsedParameters = [:] | ||
| for (key, value) in parameters { | ||
| parsedParameters?[key.value] = value |
There was a problem hiding this comment.
이거 안그래도 약간 멈칫했는데, 열거형의 rawValue를 열거형의 바깥에서 직접 쓰기 싫어서 value라고 네이밍했는데, 덜 헷갈리는 측면에서 적절한 네이밍 다른거 없을까요..?
|
다음의 수정사항이 발생하여 재검토 요청드립니다.
struct PhotoUploadEvent: AnalyticsEvent {
let name: AnalyticsEventName = .photoUpload
var parameters: [AnalyticsParameterKey: Any]?
init(method: UploadMethod, count: Int) {
parameters = [
.method: method.rawValue, // 이러면 "qr" 또는 "gallery"가 되겠죠?
.count: count
}
}위처럼 이벤트를 정의하고 리듀서에서 결과 액션 처리 시, 이벤트 수집 클라이언트를 부르면서 전달하면 됩니다. |
2 participants
🌴 작업한 브랜치
✅ 작업한 내용
개요
'사용자 행동 기반 기능 고도화 우선순위 결정'이라는 목표를 달성하기 위해 GA4 기반의 사용자 이벤트 수집 기능을 구현하는 첫 번째 단계입니다.
클린 아키텍처와 TCA 생태계에 부합하는 방향으로 Domain, Data, Infrastructure 계층의 기반 타입들을 설계하고 구현했습니다.
참고
Presentation Layer에 직접 이벤트를 붙이는 것은 별도의 Issue로 진행하려고 합니다.
이 PR내용이 머지된 이후, 각자의 담당 파트에서 병렬적으로 이벤트 수집 로직 붙이기 하면 될 것 같습니다.
Domain Layer
AnalyticsEvent: 모든 이벤트의 규격을 표현AnalyticsEventName,AnalyticsParameterKey: GA4 계획서 기반의 이벤트명과 파라미터 키를 네임스페이스화 하여 정리Data & Infra-Layer
AnalyticsService: Firebase 또는 그 외의 이벤트 수집 도구가 제공해야 할 인터페이스 선언AnalyticsRepository: 도메인 엔티티를 인프라 모델로 매핑하여 Service에게 데이터를 전달할 인터페이스❗️PR Point
주요 기술적 결정사항과 근거는 다음과 같습니다.
이벤트 및 파라미터의 열거형 관리
POP-DIP
유저 식별과 이벤트 로깅 인터페이스 분리
logEvent의 파라미터로userID를 매번 넘기지 않고, 최초 초기화 용도의configure메서드를 별도로 유지했습니다.그 외 주요 피드백 요청 사항
setUserId), 이벤트 로깅(logEvent) 밖에 없긴 합니다. 따라서 'FirebaseAnalytics 라이브러리를 어떻게 사용하는가' 보다는, 이제 각 Reducer에 어떻게 붙이고 이벤트 수집을 해야 좋을지 생각하는 것이 좋을 것 같습니다.Analytics라는 타입은 thread-safety한지 공식문서 설명을 못찾았어서(내부 Queue로 직렬화하도록 설계되어 있다는 스택오버플로우 글이 있긴하다만) 혹시 몰라서 actor로 선언했습니다.📸 스크린샷
구현 내용 상, 별도의 스크린샷은 없습니다.
📟 관련 이슈
Summary by CodeRabbit