feat: google analytics (APP-105) #41
Conversation
sergey48k
left a comment
sergey48k
left a comment
There was a problem hiding this comment.
This looks good overall, but I want to see another iteration.
| @@ -0,0 +1,95 @@ | |||
| package app | |||
src/main/kotlin/app/Analytics.kt
Outdated
| private val fuelManager = FuelManager() | ||
|
|
||
| var uuid: String = "" // Should be set on start of the app. | ||
| var username: String = "" // Should be set on success authorization. |
src/main/kotlin/app/Analytics.kt
Outdated
| post(params + defaultParams.filter { !params.contains(it) } | ||
| + idParams).responseString() | ||
| } catch (e: Throwable) { | ||
| // Never fail from analytics. |
There was a problem hiding this comment.
Add some sort of log/console output here, at least.
| try { | ||
| // Send event to GA with united params. | ||
| post(params + defaultParams.filter { !params.contains(it) } | ||
| + idParams).responseString() |
| return fuelManager.request(Method.POST, "/collect", params) | ||
| } | ||
|
|
||
| private fun trackEvent(event: String, params: List<Param> = listOf()) { |
There was a problem hiding this comment.
Can you add a comment that explains how exactly this is being logged? E.g. if it's virtual urls, can you explain how it will map to virtual urls? I also suggest all virtual urls start with /virtual prefix as a convention.
src/main/kotlin/app/Analytics.kt
Outdated
|
|
||
| fun trackError() { | ||
| // TODO(anatoly): Send message with exd, consider limit 150 bytes. | ||
| trackEvent("/error", listOf("t" to HIT_EXCEPTION)) |
There was a problem hiding this comment.
there should be some sort sort of error code as an argument for trackError, e.g.
/virtual/app/error/out-of-disk-space
src/main/kotlin/app/Analytics.kt
Outdated
| } | ||
|
|
||
| fun trackEnd() { | ||
| trackEvent("/end") |
src/main/kotlin/app/Analytics.kt
Outdated
| fun trackError() { | ||
| // TODO(anatoly): Send message with exd, consider limit 150 bytes. | ||
| trackEvent("/error", listOf("t" to HIT_EXCEPTION)) | ||
| trackEvent("error", listOf("t" to HIT_EXCEPTION)) |
There was a problem hiding this comment.
Normally, you don't want to change your logged events because those that are logged will stay in GA forever. So any change to log format, like renaming events, will impact funnels and their historical availability. I will let this one slip this one time, but please follow up with detailed error codes asap.
There was a problem hiding this comment.
I think it's better to use special error tracking software, e.g. https://sentry.io, because error code is not enough.
There was a problem hiding this comment.
For GA error urls probably best solution is to use exception names, agree?
No description provided.