Skip to content
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

ALTAPPS-1323: Shared research analytic logging issue #1145

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class AnalyticHyperskillCacheDataSourceImpl : AnalyticHyperskillCacheDataSource
events.add(event)
}

override suspend fun logEvents(events: List<AnalyticEvent>) {
this.events.addAll(events)
}

override suspend fun getEvents(): List<AnalyticEvent> =
events.toList()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import org.hyperskill.app.analytic.domain.model.AnalyticEvent
import org.hyperskill.app.analytic.domain.repository.AnalyticHyperskillRepository

internal class AnalyticHyperskillRepositoryImpl(
private val mutex: Mutex,
private val hyperskillRemoteDataSource: AnalyticHyperskillRemoteDataSource,
private val hyperskillCacheDataSource: AnalyticHyperskillCacheDataSource
) : AnalyticHyperskillRepository {

private val mutex = Mutex()

override suspend fun logEvent(event: AnalyticEvent) {
mutex.withLock {
hyperskillCacheDataSource.logEvent(event)
Expand All @@ -24,6 +26,12 @@ internal class AnalyticHyperskillRepositoryImpl(
eventsToFlush = hyperskillCacheDataSource.getEvents()
hyperskillCacheDataSource.clearEvents()
}
return hyperskillRemoteDataSource.flushEvents(eventsToFlush, isAuthorized)
return hyperskillRemoteDataSource
.flushEvents(eventsToFlush, isAuthorized)
.onFailure {
mutex.withLock {
hyperskillCacheDataSource.logEvents(eventsToFlush)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that there is a case where the current user profile can be changed between logEvent, and old analytic events can revert to being associated with the previous user profile.
However, it is not so critical right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivan-magda
Yes, you are right. Good point to think about.

}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.hyperskill.app.analytic.domain.model.AnalyticEvent

interface AnalyticHyperskillCacheDataSource {
suspend fun logEvent(event: AnalyticEvent)
suspend fun logEvents(events: List<AnalyticEvent>)
suspend fun getEvents(): List<AnalyticEvent>
suspend fun clearEvents()
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ internal class HyperskillAnalyticEngineImpl(

analyticHyperskillRepository
.flushEvents(isAuthorized)
.onSuccess {
logger.d { "Successfully flush events" }
}
.onFailure { logger.e(it) { "Failed to flush events" } }
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.hyperskill.app.analytic.injection

import kotlinx.coroutines.sync.Mutex
import org.hyperskill.app.analytic.cache.AnalyticHyperskillCacheDataSourceImpl
import org.hyperskill.app.analytic.data.repository.AnalyticHyperskillRepositoryImpl
import org.hyperskill.app.analytic.data.source.AnalyticHyperskillCacheDataSource
Expand All @@ -25,7 +24,6 @@ internal class HyperskillAnalyticEngineComponentImpl(appGraph: AppGraph) : Hyper
AnalyticHyperskillCacheDataSourceImpl()
private val hyperskillRepository: AnalyticHyperskillRepository =
AnalyticHyperskillRepositoryImpl(
Mutex(),
hyperskillRemoteDataSource,
hyperskillCacheDataSource
)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,23 @@ package org.hyperskill.app.analytic.presentation
import kotlinx.coroutines.CoroutineScope
import org.hyperskill.app.analytic.domain.interactor.AnalyticInteractor
import org.hyperskill.app.analytic.domain.model.AnalyticEvent
import org.hyperskill.app.core.presentation.CompletableCoroutineActionDispatcher
import org.hyperskill.app.core.presentation.CompletableCoroutineActionDispatcherConfig
import ru.nobird.app.presentation.redux.dispatcher.wrapWithActionDispatcher
import ru.nobird.app.presentation.redux.feature.Feature

/**
* Wraps the given [Feature] with an [AnalyticActionDispatcher].
* Wraps the given [Feature] with an [CompletableCoroutineActionDispatcher].
*
* @param analyticInteractor The [AnalyticInteractor] used for logging analytic events.
*
* @param parentScope The parent [CoroutineScope] to use for creating the logAnalyticScope. The Default value is null.
* @see [AnalyticActionDispatcher.ScopeConfigOptions] for more details.
* @see [CompletableCoroutineActionDispatcher.ScopeConfigOptions] for more details.
*
* @param getAnalyticEvent A function that takes an [Action] as input and returns an [AnalyticEvent].
* If the function returns null, no events will be logged.
*
* @see [AnalyticActionDispatcher], [AnalyticActionDispatcherConfig], [AnalyticActionDispatcher.ScopeConfigOptions]
* @see [CompletableCoroutineActionDispatcher], [CompletableCoroutineActionDispatcherConfig], [CompletableCoroutineActionDispatcher.ScopeConfigOptions]
*/
internal inline fun <State, Message, Action> Feature<State, Message, Action>.wrapWithAnalyticLogger(
analyticInteractor: AnalyticInteractor,
Expand All @@ -33,17 +35,17 @@ internal inline fun <State, Message, Action> Feature<State, Message, Action>.wra
)

/**
* Wraps the given [Feature] with an [AnalyticActionDispatcher].
* Wraps the given [Feature] with an [CompletableCoroutineActionDispatcher].
*
* @param analyticInteractor The [AnalyticInteractor] used for logging analytic events.
*
* @param parentScope The parent CoroutineScope to use for creating the logAnalyticScope. The Default value is null.
* @see [AnalyticActionDispatcher.ScopeConfigOptions] for more details.
* @see [CompletableCoroutineActionDispatcher.ScopeConfigOptions] for more details.
*
* @param getAnalyticEvent A function that takes an [Action] as input and returns a collection of [AnalyticEvent].
* If the function returns null, no events will be logged.
*
* @see [AnalyticActionDispatcher], [AnalyticActionDispatcherConfig], [AnalyticActionDispatcher.ScopeConfigOptions]
* @see [CompletableCoroutineActionDispatcher], [CompletableCoroutineActionDispatcherConfig], [CompletableCoroutineActionDispatcher.ScopeConfigOptions]
*/
internal fun <State, Message, Action> Feature<State, Message, Action>.wrapWithBatchAnalyticLogger(
analyticInteractor: AnalyticInteractor,
Expand All @@ -63,23 +65,30 @@ internal inline fun <Action, Message> SingleAnalyticEventActionDispatcher(
analyticInteractor: AnalyticInteractor,
parentScope: CoroutineScope? = null,
crossinline getAnalyticEvent: (Action) -> AnalyticEvent?
): AnalyticActionDispatcher<Action, Message> =
AnalyticActionDispatcher(
analyticInteractor = analyticInteractor,
logAnalyticScope = AnalyticActionDispatcherConfig(parentScope).createLogAnalyticScope()
) { action ->
val event = getAnalyticEvent(action)
if (event != null) listOf(event) else null
): CompletableCoroutineActionDispatcher<Action, Message> =
object : CompletableCoroutineActionDispatcher<Action, Message>(
coroutineScope = CompletableCoroutineActionDispatcherConfig(parentScope).createScope()
) {
override suspend fun handleNonCancellableAction(action: Action) {
val event = getAnalyticEvent(action)
if (event != null) {
analyticInteractor.logEvent(event)
}
}
}

@Suppress("FunctionName", "unused")
internal inline fun <Action, Message> BatchAnalyticEventActionDispatcher(
analyticInteractor: AnalyticInteractor,
parentScope: CoroutineScope? = null,
noinline getAnalyticEvent: (Action) -> Collection<AnalyticEvent>?
): AnalyticActionDispatcher<Action, Message> =
AnalyticActionDispatcher(
analyticInteractor = analyticInteractor,
logAnalyticScope = AnalyticActionDispatcherConfig(parentScope).createLogAnalyticScope(),
getAnalyticEvent = getAnalyticEvent
)
): CompletableCoroutineActionDispatcher<Action, Message> =
object : CompletableCoroutineActionDispatcher<Action, Message>(
coroutineScope = CompletableCoroutineActionDispatcherConfig(parentScope).createScope(),
) {
override suspend fun handleNonCancellableAction(action: Action) {
getAnalyticEvent(action)?.forEach { analyticEvent ->
analyticInteractor.logEvent(analyticEvent)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.hyperskill.app.core.presentation

import kotlinx.coroutines.CompletableJob
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.launch
import kotlinx.coroutines.plus
import org.hyperskill.app.analytic.domain.model.AnalyticEvent
import ru.nobird.app.presentation.redux.dispatcher.ActionDispatcher

/**
* Base class for ActionDispatcher dispatching actions, that should not be cancelled with feature cancellation.
* E.g. analytic event logging.
*
* @param coroutineScope is not cancelled on [cancel].
* Instead it is complete to wait for all launched coroutines to finish.
*/
internal abstract class CompletableCoroutineActionDispatcher<Action, Message>(
private val coroutineScope: CoroutineScope
) : ActionDispatcher<Action, Message> {

private var isCancelled: Boolean = false

abstract suspend fun handleNonCancellableAction(action: Action)

override fun handleAction(action: Action) {
if (isCancelled) return

coroutineScope.launch {
handleNonCancellableAction(action)
}
}

override fun setListener(listener: (message: Message) -> Unit) {
// no op
}

override fun cancel() {
isCancelled = true
(coroutineScope.coroutineContext[Job] as? CompletableJob)?.complete()
}

/**
* Represents [CoroutineScope] config for logging [AnalyticEvent].
* If [parentScope] is not presented uses a [Dispatchers.Main].immediate + [SupervisorJob].
*/
interface ScopeConfigOptions {
val parentScope: CoroutineScope?

val coroutineExceptionHandler: CoroutineExceptionHandler

fun createScope(): CoroutineScope {
val parentScope = parentScope
return if (parentScope != null) {
parentScope + SupervisorJob(parentScope.coroutineContext[Job]) + coroutineExceptionHandler
} else {
CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate + coroutineExceptionHandler)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package org.hyperskill.app.analytic.presentation
package org.hyperskill.app.core.presentation

import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.CoroutineScope
import org.hyperskill.app.core.domain.throwError

internal class AnalyticActionDispatcherConfig(
override val logAnalyticParentScope: CoroutineScope? = null
) : AnalyticActionDispatcher.ScopeConfigOptions {
override val logAnalyticCoroutineExceptionHandler: CoroutineExceptionHandler =
internal class CompletableCoroutineActionDispatcherConfig(
override val parentScope: CoroutineScope? = null
) : CompletableCoroutineActionDispatcher.ScopeConfigOptions {
override val coroutineExceptionHandler: CoroutineExceptionHandler =
CoroutineExceptionHandler { _, throwable ->
if (throwable !is CancellationException) {
throwError(throwable) // rethrow if not cancellation exception
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.hyperskill.app.analytic.presentation.wrapWithAnalyticLogger
import org.hyperskill.app.core.domain.BuildVariant
import org.hyperskill.app.core.domain.url.HyperskillUrlBuilder
import org.hyperskill.app.core.presentation.ActionDispatcherOptions
import org.hyperskill.app.core.presentation.CompletableCoroutineActionDispatcherConfig
import org.hyperskill.app.core.presentation.transformState
import org.hyperskill.app.learning_activities.domain.repository.NextLearningActivityStateRepository
import org.hyperskill.app.logging.presentation.wrapWithLogger
Expand Down Expand Up @@ -69,6 +70,11 @@ internal object StepFeatureBuilder {
logger = logger.withTag(LOG_TAG)
)

val viewStepActionDispatcher = ViewStepActionDispatcher(
config = CompletableCoroutineActionDispatcherConfig(),
stepInteractor = stepInteractor
)

val stepViewStateMapper = StepViewStateMapper(stepRoute)

return ReduxFeature(StepFeature.initialState(stepRoute), stepReducer)
Expand All @@ -89,6 +95,6 @@ internal object StepFeatureBuilder {
.wrapWithAnalyticLogger(analyticInteractor) {
(it as? InternalAction.LogAnalyticEvent)?.analyticEvent
}
.wrapWithActionDispatcher(ViewStepActionDispatcher(stepInteractor))
.wrapWithActionDispatcher(viewStepActionDispatcher)
}
}
Loading
Loading