Skip to content

Commit

Permalink
Technical Analytics: Milestone 3 - New App Health Metrics (#5320)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
When merged, this PR will;
- Introduce the ability to log all network calls as well as failed
network calls.
- Introduce the ability to log all console errors logged via the
`ConsoleLogger`.
- Introduce the ability to log the time spent by a user with the app in
the foreground.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [ ] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
  • Loading branch information
kkmurerwa authored Apr 9, 2024
1 parent 9e8553c commit bf80719
Show file tree
Hide file tree
Showing 36 changed files with 1,315 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.view.View
import android.view.ViewGroup
import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.Fragment
import androidx.lifecycle.Observer
import androidx.recyclerview.widget.GridLayoutManager
import org.oppia.android.R
import org.oppia.android.app.drawer.NAVIGATION_PROFILE_ID_ARGUMENT_KEY
Expand All @@ -13,6 +14,7 @@ import org.oppia.android.app.home.promotedlist.ComingSoonTopicListViewModel
import org.oppia.android.app.home.promotedlist.PromotedStoryListViewModel
import org.oppia.android.app.home.topiclist.AllTopicsViewModel
import org.oppia.android.app.home.topiclist.TopicSummaryViewModel
import org.oppia.android.app.model.AppStartupState
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.TopicSummary
import org.oppia.android.app.recyclerview.BindableAdapter
Expand All @@ -24,11 +26,14 @@ import org.oppia.android.databinding.HomeFragmentBinding
import org.oppia.android.databinding.PromotedStoryListBinding
import org.oppia.android.databinding.TopicSummaryViewBinding
import org.oppia.android.databinding.WelcomeBinding
import org.oppia.android.domain.onboarding.AppStartupStateController
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.oppialogger.analytics.AnalyticsController
import org.oppia.android.domain.profile.ProfileManagementController
import org.oppia.android.domain.topic.TopicListController
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import org.oppia.android.util.parser.html.StoryHtmlParserEntityType
import org.oppia.android.util.parser.html.TopicHtmlParserEntityType
import javax.inject.Inject
Expand All @@ -47,7 +52,8 @@ class HomeFragmentPresenter @Inject constructor(
private val resourceHandler: AppLanguageResourceHandler,
private val dateTimeUtil: DateTimeUtil,
private val translationController: TranslationController,
private val multiTypeBuilderFactory: BindableAdapter.MultiTypeBuilder.Factory
private val multiTypeBuilderFactory: BindableAdapter.MultiTypeBuilder.Factory,
private val appStartupStateController: AppStartupStateController
) {
private val routeToTopicPlayStoryListener = activity as RouteToTopicPlayStoryListener
private lateinit var binding: HomeFragmentBinding
Expand Down Expand Up @@ -96,9 +102,45 @@ class HomeFragmentPresenter @Inject constructor(
it.viewModel = homeViewModel
}

logAppOnboardedEvent()

return binding.root
}

private fun logAppOnboardedEvent() {
val startupStateProvider = appStartupStateController.getAppStartupState()
val liveData = startupStateProvider.toLiveData()
liveData.observe(
activity,
object : Observer<AsyncResult<AppStartupState>> {
override fun onChanged(startUpStateResult: AsyncResult<AppStartupState>?) {
when (startUpStateResult) {
is AsyncResult.Pending -> {
// Do nothing
}
is AsyncResult.Success -> {
liveData.removeObserver(this)

if (startUpStateResult.value.startupMode ==
AppStartupState.StartupMode.USER_NOT_YET_ONBOARDED
) {
analyticsController.logAppOnboardedEvent(
ProfileId.newBuilder().setInternalId(internalProfileId).build()
)
}
}
is AsyncResult.Failure -> {
oppiaLogger.e(
"HomeFragment",
"Failed to retrieve app startup state"
)
}
}
}
}
)
}

private fun createRecyclerViewAdapter(): BindableAdapter<HomeItemViewModel> {
return multiTypeBuilderFactory.create<HomeItemViewModel, ViewType> { viewModel ->
when (viewModel) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.oppia.android.app.home

import android.app.Application
import android.content.Context
import android.content.Intent
import androidx.appcompat.app.AppCompatActivity
import androidx.test.core.app.ActivityScenario.launch
Expand All @@ -26,6 +27,7 @@ import org.oppia.android.app.application.testing.TestingBuildFlavorModule
import org.oppia.android.app.devoptions.DeveloperOptionsModule
import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule
import org.oppia.android.app.model.EventLog
import org.oppia.android.app.model.EventLog.Context.ActivityContextCase.COMPLETE_APP_ONBOARDING
import org.oppia.android.app.model.EventLog.Context.ActivityContextCase.OPEN_HOME
import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule
import org.oppia.android.app.shim.IntentFactoryShimModule
Expand All @@ -51,6 +53,7 @@ import org.oppia.android.domain.exploration.ExplorationProgressModule
import org.oppia.android.domain.exploration.ExplorationStorageModule
import org.oppia.android.domain.hintsandsolution.HintsAndSolutionConfigModule
import org.oppia.android.domain.hintsandsolution.HintsAndSolutionProdModule
import org.oppia.android.domain.onboarding.AppStartupStateController
import org.oppia.android.domain.onboarding.ExpirationMetaDataRetrieverModule
import org.oppia.android.domain.oppialogger.LogStorageModule
import org.oppia.android.domain.oppialogger.LoggingIdentifierModule
Expand All @@ -65,6 +68,7 @@ import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.data.DataProviderTestMonitor
import org.oppia.android.testing.firebase.TestAuthenticationModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -106,12 +110,17 @@ class HomeActivityLocalTest {
@Inject
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers

@Inject
lateinit var appStartupStateController: AppStartupStateController

@Inject
lateinit var monitorFactory: DataProviderTestMonitor.Factory

private val internalProfileId: Int = 1

@Before
fun setUp() {
Intents.init()
setUpTestApplicationComponent()
}

@After
Expand All @@ -121,15 +130,69 @@ class HomeActivityLocalTest {

@Test
fun testHomeActivity_onLaunch_logsEvent() {
setUpTestApplicationComponent()

launch<HomeActivity>(createHomeActivityIntent(internalProfileId)).use {
testCoroutineDispatchers.runCurrent()
val event = fakeAnalyticsEventLogger.getOldestEvent()

assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
assertThat(event.context.activityContextCase).isEqualTo(OPEN_HOME)
}
}

@Test
fun testHomeActivity_onFirstLaunch_logsCompletedOnboardingEvent() {
setUpTestApplicationComponent()
launch<HomeActivity>(createHomeActivityIntent(internalProfileId)).use {
testCoroutineDispatchers.runCurrent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.priority).isEqualTo(EventLog.Priority.OPTIONAL)
assertThat(event.context.activityContextCase).isEqualTo(COMPLETE_APP_ONBOARDING)
}
}

@Test
fun testHomeActivity_onSubsequentLaunch_doesNotLogCompletedOnboardingEvent() {
executeInPreviousAppInstance { testComponent ->
testComponent.getAppStartupStateController().markOnboardingFlowCompleted()
testComponent.getTestCoroutineDispatchers().runCurrent()
}

setUpTestApplicationComponent()
launch<HomeActivity>(createHomeActivityIntent(internalProfileId)).use {
testCoroutineDispatchers.runCurrent()
val eventCount = fakeAnalyticsEventLogger.getEventListCount()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(eventCount).isEqualTo(1)
assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
assertThat(event.context.activityContextCase).isEqualTo(OPEN_HOME)
}
}

/**
* Creates a separate test application component and executes the specified block. This should be
* called before [setUpTestApplicationComponent] to avoid undefined behavior in production code.
* This can be used to simulate arranging state in a "prior" run of the app.
*
* Note that only dependencies fetched from the specified [TestApplicationComponent] should be
* used, not any class-level injected dependencies.
*/
private fun executeInPreviousAppInstance(block: (TestApplicationComponent) -> Unit) {
val testApplication = TestApplication()
// The true application is hooked as a base context. This is to make sure the new application
// can behave like a real Android application class (per Robolectric) without having a shared
// Dagger dependency graph with the application under test.
testApplication.attachBaseContext(ApplicationProvider.getApplicationContext())
block(
DaggerHomeActivityLocalTest_TestApplicationComponent.builder()
.setApplication(testApplication)
.build() as TestApplicationComponent
)
}

private fun createHomeActivityIntent(profileId: Int): Intent {
return HomeActivity.createHomeActivity(ApplicationProvider.getApplicationContext(), profileId)
}
Expand Down Expand Up @@ -175,6 +238,10 @@ class HomeActivityLocalTest {
interface Builder : ApplicationComponent.Builder

fun inject(homeActivityLocalTest: HomeActivityLocalTest)

fun getAppStartupStateController(): AppStartupStateController

fun getTestCoroutineDispatchers(): TestCoroutineDispatchers
}

class TestApplication : Application(), ActivityComponentFactory, ApplicationInjectorProvider {
Expand All @@ -188,6 +255,10 @@ class HomeActivityLocalTest {
component.inject(homeActivityLocalTest)
}

public override fun attachBaseContext(base: Context?) {
super.attachBaseContext(base)
}

override fun createActivityComponent(activity: AppCompatActivity): ActivityComponent {
return component.getActivityComponentBuilderProvider().get().setActivity(activity).build()
}
Expand Down
5 changes: 5 additions & 0 deletions data/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ TEST_DEPS = [
"//testing/src/main/java/org/oppia/android/testing/network",
"//testing/src/main/java/org/oppia/android/testing/network:test_module",
"//testing/src/main/java/org/oppia/android/testing/platformparameter:test_constants",
"//testing/src/main/java/org/oppia/android/testing/platformparameter:test_module",
"//testing/src/main/java/org/oppia/android/testing/robolectric:oppia_shadow_activity_manager",
"//testing/src/main/java/org/oppia/android/testing/robolectric:oppia_shadow_traffic_stats",
"//testing/src/main/java/org/oppia/android/testing/robolectric:test_module",
"//testing/src/main/java/org/oppia/android/testing/threading:test_module",
"//third_party:androidx_test_ext_junit",
Expand All @@ -40,6 +43,8 @@ TEST_DEPS = [
"//third_party:org_robolectric_annotations",
"//third_party:robolectric_android-all",
"//utility",
"//utility/src/main/java/org/oppia/android/util/logging:prod_module",
"//utility/src/main/java/org/oppia/android/util/threading:annotations",
]

# Qualified file paths for test classes that have been migrated over to their own packages &
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,25 @@ kt_android_library(
srcs = [
"JsonPrefixNetworkInterceptor.kt",
"NetworkApiKey.kt",
"NetworkLoggingInterceptor.kt",
"RemoteAuthNetworkInterceptor.kt",
],
visibility = ["//:oppia_testing_visibility"],
visibility = [
"//:oppia_api_visibility",
"//:oppia_testing_visibility",
],
deps = [
":constants",
":dagger",
":network_config_annotations",
"//model/src/main/proto:arguments_java_proto_lite",
"//model/src/main/proto:event_logger_java_proto_lite",
"//third_party:com_squareup_okhttp3_okhttp",
"//third_party:javax_inject_javax_inject",
"//third_party:org_jetbrains_kotlinx_kotlinx-coroutines-core",
"//utility/src/main/java/org/oppia/android/util/extensions:context_extensions",
"//utility/src/main/java/org/oppia/android/util/logging:prod_module",
"//utility/src/main/java/org/oppia/android/util/threading:annotations",
],
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package org.oppia.android.data.backends.gae

import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.launch
import okhttp3.Interceptor
import okhttp3.Response
import org.oppia.android.app.model.EventLog.RetrofitCallContext
import org.oppia.android.app.model.EventLog.RetrofitCallFailedContext
import org.oppia.android.util.threading.BackgroundDispatcher
import java.io.IOException
import java.lang.Exception
import javax.inject.Inject
import javax.inject.Singleton

/**
* Interceptor on top of Retrofit to log network requests and responses.
*/
@Singleton
class NetworkLoggingInterceptor @Inject constructor(
@BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher,
) : Interceptor {
private val _logNetworkCallFlow = MutableSharedFlow<RetrofitCallContext>()
/**
* A flow that emits a [RetrofitCallContext] when a network call is made.
*/
val logNetworkCallFlow: SharedFlow<RetrofitCallContext> = _logNetworkCallFlow

private val _logFailedNetworkCallFlow = MutableSharedFlow<RetrofitCallFailedContext>()

/**
* A flow that emits a [RetrofitCallFailedContext] when a network call fails.
*/
val logFailedNetworkCallFlow: SharedFlow<RetrofitCallFailedContext> = _logFailedNetworkCallFlow

@Throws(IOException::class)
override fun intercept(chain: Interceptor.Chain): Response {
val request = chain.request()

return try {
val response = chain.proceed(request)

val responseBody = response.body?.string()

CoroutineScope(backgroundDispatcher).launch {
_logNetworkCallFlow.emit(
RetrofitCallContext.newBuilder()
.setRequestUrl(request.url.toString())
.setHeaders(request.headers.toString())
.setResponseStatusCode(response.code)
.setBody(responseBody ?: "")
.build()
)
}

if (!response.isSuccessful) {
CoroutineScope(backgroundDispatcher).launch {
_logFailedNetworkCallFlow.emit(
RetrofitCallFailedContext.newBuilder()
.setRequestUrl(request.url.toString())
.setHeaders(request.headers.toString())
.setResponseStatusCode(response.code)
.setErrorMessage(responseBody ?: "")
.build()
)
}
}

response
} catch (exception: Exception) {
CoroutineScope(backgroundDispatcher).launch {
_logFailedNetworkCallFlow.emit(
RetrofitCallFailedContext.newBuilder()
.setRequestUrl(request.url.toString())
.setHeaders(request.headers.toString())
.setResponseStatusCode(0)
.setErrorMessage(exception.toString())
.build()
)
}
chain.proceed(request)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ class NetworkModule {
fun provideRetrofitInstance(
jsonPrefixNetworkInterceptor: JsonPrefixNetworkInterceptor,
remoteAuthNetworkInterceptor: RemoteAuthNetworkInterceptor,
networkLoggingInterceptor: NetworkLoggingInterceptor,
@BaseUrl baseUrl: String
): Optional<Retrofit> {
// TODO(#1720): Make this a compile-time dep once Hilt provides it as an option.
return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
val client = OkHttpClient.Builder()
.addInterceptor(jsonPrefixNetworkInterceptor)
.addInterceptor(remoteAuthNetworkInterceptor)
.addInterceptor(networkLoggingInterceptor)
.build()

Optional.of(
Expand Down
Loading

0 comments on commit bf80719

Please sign in to comment.