-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add unified analytics and session tracking #745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Introduced a unified `AnalyticsManager` to handle multiple analytics providers (Firebase, Mixpanel, CleanInsights) using a facade pattern. This refactor ensures GDPR compliance through automatic PII sanitization.
Key changes include:
- A new `SessionManager` for tracking app lifecycle events like session start/end and foreground/background states.
- Automatic screen view and navigation tracking in `BaseActivity` and `BaseFragment`.
- Added analytics for key user actions:
- Feature toggles in Settings (Dark Mode, Tor, etc.).
- Upload lifecycle events (start, complete, fail, cancel).
- Backend configuration events.
- Enhanced `AppLogger` to integrate with Crashlytics for improved error reporting with breadcrumbs.
- Defined a structured set of `AnalyticsEvent` types for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Extracted all analytics logic, including Mixpanel, Firebase, and CleanInsights providers, into a new `:analytics` module. This modernizes the implementation by using coroutines, dependency injection with Koin, and reactive session tracking with StateFlow. Key changes: - Created a unified `AnalyticsManager` interface and `AnalyticsEvent` sealed interface for type-safe, asynchronous event tracking. - Introduced a `SessionTracker` to manage session lifecycle events reactively. - Replaced the old static `AnalyticsManager` and `SessionManager` with injected, testable components. - Renamed `UploadBatchStarted`/`Completed` events to `UploadSessionStarted`/`Completed` for clarity. - Removed direct analytics dependencies from the `:app` module.
| analyticsManager.setUserProperty("device_type", "android") | ||
|
|
||
| // Register app lifecycle observer AFTER analytics is initialized | ||
| ProcessLifecycleOwner.get().lifecycle.addObserver(this@SaveApp) |
Check warning
Code scanning / detekt
Reports lines with exceeded length Warning
| analyticsManager.setUserProperty("device_type", "android") | ||
|
|
||
| // Register app lifecycle observer AFTER analytics is initialized | ||
| ProcessLifecycleOwner.get().lifecycle.addObserver(this@SaveApp) |
Check warning
Code scanning / detekt
Reports incorrect argument list wrapping Warning
| analyticsManager.setUserProperty("device_type", "android") | ||
|
|
||
| // Register app lifecycle observer AFTER analytics is initialized | ||
| ProcessLifecycleOwner.get().lifecycle.addObserver(this@SaveApp) |
Check warning
Code scanning / detekt
Reports incorrect argument list wrapping Warning
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.SupervisorJob | ||
| import kotlinx.coroutines.launch | ||
| import net.opendasharchive.openarchive.analytics.api.AnalyticsEvent |
Check warning
Code scanning / detekt
Detects unused imports Warning
|
|
||
| try { | ||
| crashlytics = FirebaseCrashlytics.getInstance() | ||
| } catch (e: Exception) { |
Check warning
Code scanning / detekt
The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled. Warning
|
|
||
| // Track successful upload analytics | ||
| val uploadDuration = (System.currentTimeMillis() - uploadStartTime) / 1000 | ||
| val fileSizeKB = mMedia.contentLength / 1024 |
Check warning
Code scanning / detekt
Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning
| // Track failed upload analytics (GDPR-compliant - no PII) | ||
| val backendType = mMedia.space?.tType?.friendlyName ?: "Unknown" | ||
| val fileType = getFileType(mMedia.mimeType) | ||
| val fileSizeKB = mMedia.contentLength / 1024 |
Check warning
Code scanning / detekt
Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning
| if (initialBatch.isNotEmpty()) { | ||
| // Track upload session started (1+ files) | ||
| val sessionSize = initialBatch.size | ||
| val totalSizeMB = initialBatch.sumOf { it.contentLength } / (1024 * 1024) |
Check warning
Code scanning / detekt
Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning
| if (initialBatch.isNotEmpty()) { | ||
| // Track upload session started (1+ files) | ||
| val sessionSize = initialBatch.size | ||
| val totalSizeMB = initialBatch.sumOf { it.contentLength } / (1024 * 1024) |
Check warning
Code scanning / detekt
Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning
|
|
||
| // Track upload session completed (if any uploads were attempted) | ||
| if (totalCount > 0) { | ||
| val sessionDuration = (System.currentTimeMillis() - batchStartTime) / 1000 |
Check warning
Code scanning / detekt
Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning
Elelan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Introduced a unified
AnalyticsManagerto handle multiple analytics providers (Firebase, Mixpanel, CleanInsights) using a facade pattern. This refactor ensures GDPR compliance through automatic PII sanitization.Key changes include:
SessionManagerfor tracking app lifecycle events like session start/end and foreground/background states.BaseActivityandBaseFragment.AppLoggerto integrate with Crashlytics for improved error reporting with breadcrumbs.AnalyticsEventtypes for consistency.UPDATE 5th December 2025
Extracted all analytics logic, including Mixpanel, Firebase, and CleanInsights providers, into a new
:analyticsmodule. This modernizes the implementation by using coroutines, dependency injection with Koin, and reactive session tracking with StateFlow.Key changes:
AnalyticsManagerinterface andAnalyticsEventsealed interface for type-safe, asynchronous event tracking.SessionTrackerto manage session lifecycle events reactively.AnalyticsManagerandSessionManagerwith injected, testable components.UploadBatchStarted/Completedevents toUploadSessionStarted/Completedfor clarity.:appmodule.