Skip to content

Commit

Permalink
Merge pull request #534 from WideChat/instrument-send-message-failures
Browse files Browse the repository at this point in the history
log sendMessage exception to google analytics
  • Loading branch information
bizzbyster authored Jul 25, 2019
2 parents 123a1ce + 98179d0 commit 76f488e
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 22 deletions.
13 changes: 11 additions & 2 deletions app/src/main/java/chat/rocket/android/analytics/Analytics.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,19 @@ interface Analytics {
/**
* Logs the message sent event.
*
* @param event The [SubscriptionTypeEvent] to log.
* @param messageType The type of message to log
* @param serverUrl The server URL to log.
*/
fun logMessageSent(messageType: String, serverUrl: String) {}

/**
* Logs the exception that occurs when sendMessage fails
*
* @param countToSend The number of messages still outstanding that need to be sent when exception occurs
* @param exceptionDescription The string associated with the exception
* @param serverUrl The server URL to log.
*/
fun logMessageSent(event: SubscriptionTypeEvent, serverUrl: String) {}
fun logSendMessageException(countToSend: Int, exceptionDescription: String, serverUrl: String) {}

/**
* Logs the media upload event.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,15 @@ class AnalyticsManager @Inject constructor(
}
}

fun logMessageSent(event: SubscriptionTypeEvent) {
fun logMessageSent(messageType: String, serverUrl: String) {
if (analyticsTrackingInteractor.get() && serverUrl != null) {
analytics.forEach { it.logMessageSent(event, serverUrl) }
analytics.forEach { it.logMessageSent(messageType, serverUrl) }
}
}

fun logSendMessageException(countToSend: Int, exceptionDescription: String, serverUrl: String) {
if (analyticsTrackingInteractor.get()) {
analytics.forEach { it.logSendMessageException(countToSend, exceptionDescription, serverUrl) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ class ChatRoomPresenter @Inject constructor(
private val messagesChannel = Channel<Message>()

private var chatRoomId: String? = null
private lateinit var chatRoomType: String
private lateinit var chatRoomName: String
lateinit var chatRoomType: String
lateinit var chatRoomName: String
private var chatIsBroadcast: Boolean = false
private var chatRoles = emptyList<ChatRoomRole>()
private val stateChannel = Channel<State>()
Expand Down Expand Up @@ -410,8 +410,11 @@ class ChatRoomPresenter @Inject constructor(
)
client.sendMessage(id, chatRoomId, text)
messagesRepository.save(newMessage.copy(synced = true))
logMessageSent()
analyticsManager.logMessageSent(newMessage.type.toString(), currentServer)

} catch (ex: Exception) {
analyticsManager.logSendMessageException(0, ex.toString(), currentServer)

// Ok, not very beautiful, but the backend sends us a not valid response
// When someone sends a message on a read-only channel, so we just ignore it
// and show a generic error message
Expand Down Expand Up @@ -1166,16 +1169,6 @@ class ChatRoomPresenter @Inject constructor(
}
}

private fun logMessageSent() {
when {
roomTypeOf(chatRoomType) is RoomType.DirectMessage ->
analyticsManager.logMessageSent(SubscriptionTypeEvent.DirectMessage)
roomTypeOf(chatRoomType) is RoomType.Channel ->
analyticsManager.logMessageSent(SubscriptionTypeEvent.Channel)
else -> analyticsManager.logMessageSent(SubscriptionTypeEvent.Group)
}
}

fun showReactions(messageId: String) {
view.showReactionsPopup(messageId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package chat.rocket.android.chatroom.service

import android.app.job.JobParameters
import android.app.job.JobService
import chat.rocket.android.analytics.AnalyticsManager
import chat.rocket.android.db.DatabaseManagerFactory
import chat.rocket.android.server.domain.GetAccountsInteractor
import chat.rocket.android.server.infraestructure.ConnectionManagerFactory
Expand All @@ -23,6 +24,8 @@ class MessageService : JobService() {
lateinit var dbFactory: DatabaseManagerFactory
@Inject
lateinit var getAccountsInteractor: GetAccountsInteractor
@Inject
lateinit var analyticsManager: AnalyticsManager

override fun onCreate() {
super.onCreate()
Expand Down Expand Up @@ -62,10 +65,13 @@ class MessageService : JobService() {
alias = message.senderAlias
)
messageRepository.save(message.copy(synced = true))
analyticsManager.logMessageSent(message.type.toString(), serverUrl)
Timber.d("Sent scheduled message given by id: ${message.id}")
} catch (ex: Exception) {
analyticsManager.logSendMessageException(temporaryMessages.size, ex.toString(), serverUrl)
Timber.e(ex)
// TODO - remove the generic message when we implement :userId:/message subscription

if (ex is IllegalStateException) {
Timber.e(ex, "Probably a read-only problem...")
// TODO: For now we are only going to reschedule when api is fixed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,22 @@ class AnswersAnalytics : Analytics {
.logCustom(CustomEvent("screen_view").putCustomAttribute("screen", event.screenName))


override fun logMessageSent(event: SubscriptionTypeEvent, serverUrl: String) =
override fun logMessageSent(messageType: String, serverUrl: String) =
Answers.getInstance()
.logCustom(
CustomEvent("message_actionsent")
.putCustomAttribute("subscription_type", event.subscriptionTypeName)
.putCustomAttribute("message_type", messageType)
.putCustomAttribute("server", serverUrl)
)

override fun logSendMessageException(countToSend: Int, exceptionDescription: String, serverUrl: String) =
Answers.getInstance()
.logCustom(
CustomEvent("send_message_exception")
.putCustomAttribute("description", exceptionDescription)
.putCustomAttribute("count_to_send", countToSend)
.putCustomAttribute("server", serverUrl)
)

override fun logMediaUploaded(event: SubscriptionTypeEvent, mimeType: String) =
Answers.getInstance()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,28 @@ class GoogleAnalyticsForFirebase @Inject constructor(val context: Context) :
}

// WIDECHAT tracking BSSID
override fun logMessageSent(event: SubscriptionTypeEvent, serverUrl: String) {
override fun logMessageSent(messageType: String, serverUrl: String) {
var bssid = SharedPreferenceHelper.getString(Constants.CURRENT_BSSID, "none")
if (bssid == "none") {
bssid = SharedPreferenceHelper.getString(Constants.LOCATION_PERMISSION, "none")

}
firebaseAnalytics.logEvent("message_sent", Bundle(2).apply {
putString("subscription_type", event.subscriptionTypeName)
// putString("server", serverUrl)
putString("message_type", messageType)
putString("wifi_bssid", bssid)
})
}

override fun logSendMessageException(countToSend: Int, exceptionDescription: String, serverUrl: String) {
var bssid = SharedPreferenceHelper.getString(Constants.CURRENT_BSSID, "none")
if (bssid == "none") {
bssid = SharedPreferenceHelper.getString(Constants.LOCATION_PERMISSION, "none")
}

firebaseAnalytics.logEvent("send_message_exception", Bundle(3).apply {
putString("wifi_bssid", bssid)
putString("exception_description", exceptionDescription.take(100))
putInt("count_to_send", countToSend)
})
}

Expand Down

0 comments on commit 76f488e

Please sign in to comment.