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

Fixes #5243: Technical Analytics Milestone 3 - New App Health Metrics #5320

Merged
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
b8b1360
feat: Add code for the onboarding completed logs
kkmurerwa Jan 2, 2024
c78b85a
feat: Add event contexts for app-in-foregroun-time, retrofit calls an…
kkmurerwa Jan 2, 2024
60304b6
feat: Add ability to log app-in-foreground-time
kkmurerwa Jan 2, 2024
bf7534f
feat: Add implementations for the various logs created
kkmurerwa Jan 4, 2024
6c42dfe
fix: Fix linting issues
kkmurerwa Jan 4, 2024
c10fecd
chore: Add dependencies to ensure bazel builds
kkmurerwa Jan 4, 2024
cdfc748
Merge branch 'oppia:develop' into technical-analytics-milestone-3
kkmurerwa Jan 4, 2024
f45e165
backup: Push failing changes for backup purposes
kkmurerwa Jan 9, 2024
56d99e7
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Jan 23, 2024
f69d31d
feat: Add some tests for the network logging interceptor
kkmurerwa Jan 23, 2024
07a16d7
fix: False positives on the logging interceptor tests
kkmurerwa Jan 23, 2024
5961680
feat: Add comprehensive tests for the logging interceptor.
kkmurerwa Jan 23, 2024
b03b19f
feat: Add test file for the console logger
kkmurerwa Jan 23, 2024
466f2ce
feat: Add test file for the console logger file
kkmurerwa Jan 23, 2024
a97d6ea
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Jan 29, 2024
a02af61
feat: Add ability to log app-onboarding completed
kkmurerwa Jan 30, 2024
300b16b
Merge branch 'technical-analytics-milestone-3' of github.com:kkmurerw…
kkmurerwa Jan 30, 2024
14b6138
fix: Fix failing lint checks
kkmurerwa Jan 30, 2024
720178d
fix: Fix failing tests on app and data modules
kkmurerwa Jan 30, 2024
932b0e5
fix: Fix failing test on NetworkModuleTest
kkmurerwa Jan 30, 2024
acb4ff4
fix: Fix failing tests
kkmurerwa Jan 30, 2024
c80c78b
fix: Fix failing tests
kkmurerwa Jan 30, 2024
0adff1a
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Feb 3, 2024
67ddb4e
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Feb 8, 2024
bf9039a
fix: Make changes from preliminary review
kkmurerwa Feb 9, 2024
e14781c
merge changes from develop
kkmurerwa Feb 9, 2024
f8cd1b2
fix: Fix failing tests
kkmurerwa Feb 12, 2024
5cb647f
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Feb 13, 2024
e306487
fix: Fix flaky tests on ApplicationLifecycleObserverTest class
kkmurerwa Feb 15, 2024
db5e237
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Feb 15, 2024
6cb4104
chore: Make improvements to test classes
kkmurerwa Feb 15, 2024
58f44f2
test: Add integration tests for the flow observers in the AnalyticsCo…
kkmurerwa Feb 19, 2024
fed6626
fix: Fix lint issues
kkmurerwa Feb 19, 2024
1762518
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Feb 20, 2024
6b748cb
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Mar 6, 2024
778fe4d
chore: Reset erroneous change
kkmurerwa Mar 6, 2024
713f99e
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Mar 13, 2024
e0cbb02
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Mar 25, 2024
0474e1f
fix: Add fixes for latest suggestions after review
kkmurerwa Apr 1, 2024
773f0f2
fix: Fix lint issues
kkmurerwa Apr 1, 2024
4106575
chore: Remove commented out code
kkmurerwa Apr 7, 2024
78dc3e1
Merge branch 'develop' into technical-analytics-milestone-3
kkmurerwa Apr 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
) {
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
Loading