Skip to content

Commit

Permalink
Fixes #4335: Addition of support for uploading performance metric logs (
Browse files Browse the repository at this point in the history
#4399)

* qualifiers and constants for metric record and upload times

* comments

* dagger provides for flags

* rename to enablePerformanceMetricCollection

* initial proto

* nit fixes

* nit fixes

* comments.

* nits

* nit

* updates.

* updates.

* nits.

* metric log inclusion.

* base setup.

* name correction.

* nits.

* nits.

* storage comment

* pss comment

* network usage comment.

* network usage comment - part 2.

* metric addition in proto definitions.

* metricLog --> loggableMetric

* proto definitions in oppiaLogger

* wording update for transmission

* dependency/api updates.

* nits.

* metric controller and oppiaLogger

* nits

* logger-controller pattern, single utils

* nits

* comments

* log uploading support

* nits

* nits

* performance metric event logger nits.

* nits.

* logger reinstated.

* nits

* deleting unused interfaces.

* log generator to metric log scheduler

* nits

* nits -- kdoc, code placing, readability

* nits -- kdoc, code placing, readability

* worker functions to schedule logging, renaming

* nits.

* clearing up build errors.

* nits.

* fixing build errors.

* removal of dependency from oppiaLogger and basing the entire implementation on Logger-Controller

* performanceMetricController tests + fakeLogger

* fakeLogger tests

* nits

* tests for the new logger.

* tests for the worker + renaming of package to logScheduler

* tests for work manager initializer

* initial base setup for utils test

* nits

* nits

* nit

* worker and initializer tests

* event bundle creator tests

* module tests

* nits

* performanceMetricsUtils tests

* nits

* renamed logUpload to logReport + nits

* nits

* nits

* review updates.

* nit

* review updates - 2

* isAppInForeground revamp

* isAppInForeground revamp

* nits

* domain tests repair

* app tests repair

* nits.

* nits.

* nits.

* nits.

* nit

* test update.

* test update.

* nit

* todo issue number update -- static check

* comments.

* nits.

* utils test + nits

* utils test + nits

* nits

* test rearrangement

* nits

* test fixes.

* nits

* nits

* nits

* memory and storage tier updates

* memory and storage tier updates

* updates.

* nits

* test file exemptions refactor due to file renaming.

* additional tests, nit fixes.

* activity manager shadow and assessor test

* nit

* nit

* shadow traffic stats + assessor test fix.

* custom shadow tests

* module deps

* tests

* nits

* metricLogScheduler refactor and test

* exemption removal

* nits

* updates.

* variable nits + parameterized test exemption

* app component dependencies.

* nits

* logging module bazel build, config module creation

* testing robolectric bazel module update

* domain bazel build fixes

* working oppia bazel build

* nits.

* nits.

* nit for bazel building

* addition of test file for module

* bazel tests fixes

* more fixes.

* reformatting

* nits

* nits

* dep fix

* deps fix

* deps fix

* nit

* nits

* nits

* gradle test fix.

* bazel oppia build

* test fixes -- renaming.

* deps addition

* previous merge correction

* nits

* updates.

* nits

* bazel update.

* nit

* nits.
  • Loading branch information
Sarthak2601 authored Sep 5, 2022
1 parent a7a8c33 commit 3839846
Show file tree
Hide file tree
Showing 13 changed files with 871 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ kt_android_library(
],
deps = [
"//domain/src/main/java/org/oppia/android/domain/oppialogger/analytics:controller",
"//domain/src/main/java/org/oppia/android/domain/oppialogger/analytics:performance_metrics_controller",
"//domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions:controller",
"//domain/src/main/java/org/oppia/android/domain/util:extensions",
"//third_party:androidx_work_work-runtime-ktx",
"//utility/src/main/java/org/oppia/android/util/logging:console_logger",
"//utility/src/main/java/org/oppia/android/util/logging:event_logger",
"//utility/src/main/java/org/oppia/android/util/logging:exception_logger",
"//utility/src/main/java/org/oppia/android/util/logging/performancemetrics:performance_metrics_event_logger",
"//utility/src/main/java/org/oppia/android/util/threading:annotations",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ class LogReportWorkManagerInitializer @Inject constructor(
)
.build()

private val workerCaseForUploadingPerformanceMetrics: Data = Data.Builder()
.putString(
LogUploadWorker.WORKER_CASE_KEY,
LogUploadWorker.PERFORMANCE_METRICS_WORKER
)
.build()

private val workerCaseForSchedulingPeriodicBackgroundMetricLogs: Data = Data.Builder()
.putString(
MetricLogSchedulingWorker.WORKER_CASE_KEY,
Expand Down Expand Up @@ -83,6 +90,12 @@ class LogReportWorkManagerInitializer @Inject constructor(
.setConstraints(logReportWorkerConstraints)
.build()

private val workRequestForUploadingPerformanceMetrics: PeriodicWorkRequest = PeriodicWorkRequest
.Builder(LogUploadWorker::class.java, 6, TimeUnit.HOURS)
.setInputData(workerCaseForUploadingPerformanceMetrics)
.setConstraints(logReportWorkerConstraints)
.build()

private val workRequestForSchedulingPeriodicBackgroundMetricLogs: PeriodicWorkRequest =
PeriodicWorkRequest.Builder(
MetricLogSchedulingWorker::class.java,
Expand Down Expand Up @@ -117,6 +130,10 @@ class LogReportWorkManagerInitializer @Inject constructor(
val workManager = WorkManager.getInstance(context)
logUploader.enqueueWorkRequestForEvents(workManager, workRequestForUploadingEvents)
logUploader.enqueueWorkRequestForExceptions(workManager, workRequestForUploadingExceptions)
logUploader.enqueueWorkRequestForPerformanceMetrics(
workManager,
workRequestForUploadingPerformanceMetrics
)
metricLogScheduler.enqueueWorkRequestForPeriodicBackgroundMetrics(
workManager,
workRequestForSchedulingPeriodicBackgroundMetricLogs
Expand All @@ -140,6 +157,9 @@ class LogReportWorkManagerInitializer @Inject constructor(
/** Returns the [UUID] of the work request that is enqueued for uploading exception logs. */
fun getWorkRequestForExceptionsId(): UUID = workRequestForUploadingExceptions.id

/** Returns the [UUID] of the work request that is enqueued for uploading performance metrics logs. */
fun getWorkRequestForPerformanceMetricsId(): UUID = workRequestForUploadingPerformanceMetrics.id

/**
* Returns the [UUID] of the work request that is enqueued for scheduling memory usage
* performance metrics collection.
Expand Down Expand Up @@ -172,6 +192,9 @@ class LogReportWorkManagerInitializer @Inject constructor(
*/
fun getWorkRequestDataForExceptions(): Data = workerCaseForUploadingExceptions

/** Returns the [Data] that goes into the work request that is enqueued for uploading performance metric logs. */
fun getWorkRequestDataForPerformanceMetrics(): Data = workerCaseForUploadingPerformanceMetrics

/**
* Returns the [Data] that goes into the work request that is enqueued for scheduling storage
* usage performance metrics collection.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import org.oppia.android.domain.oppialogger.analytics.AnalyticsController
import org.oppia.android.domain.oppialogger.analytics.PerformanceMetricsController
import org.oppia.android.domain.oppialogger.exceptions.ExceptionsController
import org.oppia.android.domain.oppialogger.exceptions.toException
import org.oppia.android.domain.util.getStringFromData
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.logging.EventLogger
import org.oppia.android.util.logging.ExceptionLogger
import org.oppia.android.util.logging.SyncStatusManager
import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsEventLogger
import org.oppia.android.util.threading.BackgroundDispatcher
import javax.inject.Inject

Expand All @@ -26,8 +28,10 @@ class LogUploadWorker private constructor(
params: WorkerParameters,
private val analyticsController: AnalyticsController,
private val exceptionsController: ExceptionsController,
private val performanceMetricsController: PerformanceMetricsController,
private val exceptionLogger: ExceptionLogger,
private val eventLogger: EventLogger,
private val performanceMetricsEventLogger: PerformanceMetricsEventLogger,
private val consoleLogger: ConsoleLogger,
private val syncStatusManager: SyncStatusManager,
@BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher
Expand All @@ -38,6 +42,7 @@ class LogUploadWorker private constructor(
const val TAG = "LogUploadWorker.tag"
const val EVENT_WORKER = "event_worker"
const val EXCEPTION_WORKER = "exception_worker"
const val PERFORMANCE_METRICS_WORKER = "performance_metrics_worker"
}

@ExperimentalCoroutinesApi
Expand All @@ -47,6 +52,7 @@ class LogUploadWorker private constructor(
when (inputData.getStringFromData(WORKER_CASE_KEY)) {
EVENT_WORKER -> uploadEvents()
EXCEPTION_WORKER -> uploadExceptions()
PERFORMANCE_METRICS_WORKER -> uploadPerformanceMetrics()
else -> Result.failure()
}
}
Expand Down Expand Up @@ -97,12 +103,29 @@ class LogUploadWorker private constructor(
}
}

/** Extracts performance metric logs from the cache store and logs them to the remote service. */
private suspend fun uploadPerformanceMetrics(): Result {
return try {
val performanceMetricsLogs = performanceMetricsController.getMetricLogStoreList()
performanceMetricsLogs.forEach { performanceMetricsLog ->
performanceMetricsEventLogger.logPerformanceMetric(performanceMetricsLog)
performanceMetricsController.removeFirstMetricLogFromStore()
}
Result.success()
} catch (e: Exception) {
consoleLogger.e(TAG, e.toString(), e)
Result.failure()
}
}

/** Creates an instance of [LogUploadWorker] by properly injecting dependencies. */
class Factory @Inject constructor(
private val analyticsController: AnalyticsController,
private val exceptionsController: ExceptionsController,
private val performanceMetricsController: PerformanceMetricsController,
private val exceptionLogger: ExceptionLogger,
private val eventLogger: EventLogger,
private val performanceMetricsEventLogger: PerformanceMetricsEventLogger,
private val consoleLogger: ConsoleLogger,
private val syncStatusManager: SyncStatusManager,
@BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher
Expand All @@ -114,8 +137,10 @@ class LogUploadWorker private constructor(
params,
analyticsController,
exceptionsController,
performanceMetricsController,
exceptionLogger,
eventLogger,
performanceMetricsEventLogger,
consoleLogger,
syncStatusManager,
backgroundDispatcher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import javax.inject.Singleton
class FakeLogUploader @Inject constructor() : LogUploader {
private val eventRequestIdList = mutableListOf<UUID>()
private val exceptionRequestIdList = mutableListOf<UUID>()
private val performanceMetricsRequestIdList = mutableListOf<UUID>()

override fun enqueueWorkRequestForEvents(
workManager: WorkManager,
Expand All @@ -27,9 +28,19 @@ class FakeLogUploader @Inject constructor() : LogUploader {
exceptionRequestIdList.add(workRequest.id)
}

override fun enqueueWorkRequestForPerformanceMetrics(
workManager: WorkManager,
workRequest: PeriodicWorkRequest
) {
performanceMetricsRequestIdList.add(workRequest.id)
}

/** Returns the most recent work request id that's stored in the [eventRequestIdList]. */
fun getMostRecentEventRequestId() = eventRequestIdList.last()

/** Returns the most recent work request id that's stored in the [exceptionRequestIdList]. */
fun getMostRecentExceptionRequestId() = exceptionRequestIdList.last()

/** Returns the most recent work request id that's stored in the [performanceMetricsRequestIdList]. */
fun getMostRecentPerformanceMetricsRequestId() = performanceMetricsRequestIdList.last()
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class LogReportWorkManagerInitializerTest {
val enqueuedEventWorkRequestId = logReportWorkManagerInitializer.getWorkRequestForEventsId()
val enqueuedExceptionWorkRequestId =
logReportWorkManagerInitializer.getWorkRequestForExceptionsId()
val enqueuedPerformanceMetricsWorkRequestId =
logReportWorkManagerInitializer.getWorkRequestForPerformanceMetricsId()
val enqueuedSchedulingStorageUsageMetricWorkRequestId =
logReportWorkManagerInitializer.getWorkRequestForSchedulingStorageUsageMetricLogsId()
val enqueuedSchedulingPeriodicUiMetricWorkRequestId =
Expand All @@ -134,6 +136,9 @@ class LogReportWorkManagerInitializerTest {
assertThat(fakeLogUploader.getMostRecentExceptionRequestId()).isEqualTo(
enqueuedExceptionWorkRequestId
)
assertThat(fakeLogUploader.getMostRecentPerformanceMetricsRequestId()).isEqualTo(
enqueuedPerformanceMetricsWorkRequestId
)
assertThat(fakeLogScheduler.getMostRecentStorageUsageMetricLoggingRequestId()).isEqualTo(
enqueuedSchedulingStorageUsageMetricWorkRequestId
)
Expand Down Expand Up @@ -186,6 +191,20 @@ class LogReportWorkManagerInitializerTest {
).isEqualTo(workerCaseForUploadingExceptions)
}

@Test
fun testWorkRequest_verifyWorkRequestDataForPerformanceMetrics() {
val workerCaseForUploadingPerformanceMetrics: Data = Data.Builder()
.putString(
LogUploadWorker.WORKER_CASE_KEY,
LogUploadWorker.PERFORMANCE_METRICS_WORKER
)
.build()

assertThat(logReportWorkManagerInitializer.getWorkRequestDataForPerformanceMetrics()).isEqualTo(
workerCaseForUploadingPerformanceMetrics
)
}

@Test
fun testWorkRequest_verifyWorkRequestData_forSchedulingStorageUsageMetricLogs() {
val workerCaseForSchedulingStorageUsageMetricLogs: Data = Data.Builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,22 @@ import org.mockito.Mockito.`when`
import org.mockito.Mockito.mock
import org.mockito.Mockito.reset
import org.oppia.android.app.model.EventLog
import org.oppia.android.app.model.OppiaMetricLog
import org.oppia.android.domain.oppialogger.EventLogStorageCacheSize
import org.oppia.android.domain.oppialogger.ExceptionLogStorageCacheSize
import org.oppia.android.domain.oppialogger.LoggingIdentifierModule
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.oppialogger.PerformanceMetricsLogStorageCacheSize
import org.oppia.android.domain.oppialogger.analytics.AnalyticsController
import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule
import org.oppia.android.domain.oppialogger.analytics.PerformanceMetricsController
import org.oppia.android.domain.oppialogger.exceptions.ExceptionsController
import org.oppia.android.domain.platformparameter.PlatformParameterModule
import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule
import org.oppia.android.domain.testing.oppialogger.loguploader.FakeLogUploader
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeExceptionLogger
import org.oppia.android.testing.FakePerformanceMetricsEventLogger
import org.oppia.android.testing.logging.FakeSyncStatusManager
import org.oppia.android.testing.logging.SyncStatusTestModule
import org.oppia.android.testing.mockito.anyOrNull
Expand All @@ -58,6 +62,9 @@ import org.oppia.android.util.logging.SyncStatusManager.SyncStatus.DATA_UPLOADED
import org.oppia.android.util.logging.SyncStatusManager.SyncStatus.DATA_UPLOADING
import org.oppia.android.util.logging.SyncStatusManager.SyncStatus.NETWORK_ERROR
import org.oppia.android.util.logging.SyncStatusManager.SyncStatus.NO_CONNECTIVITY
import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsAssessorModule
import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsConfigurationsModule
import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsEventLogger
import org.oppia.android.util.networking.NetworkConnectionDebugUtil
import org.oppia.android.util.networking.NetworkConnectionUtil.ProdConnectionStatus.NONE
import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule
Expand All @@ -69,6 +76,7 @@ import javax.inject.Singleton

private const val TEST_TIMESTAMP = 1556094120000
private const val TEST_TOPIC_ID = "test_topicId"
private const val TEST_APK_SIZE = Long.MAX_VALUE

/** Tests for [LogUploadWorker]. */
// FunctionName: test names are conventionally named with underscores.
Expand All @@ -80,9 +88,11 @@ class LogUploadWorkerTest {
@Inject lateinit var networkConnectionUtil: NetworkConnectionDebugUtil
@Inject lateinit var fakeEventLogger: FakeEventLogger
@Inject lateinit var fakeExceptionLogger: FakeExceptionLogger
@Inject lateinit var fakePerformanceMetricsEventLogger: FakePerformanceMetricsEventLogger
@Inject lateinit var oppiaLogger: OppiaLogger
@Inject lateinit var analyticsController: AnalyticsController
@Inject lateinit var exceptionsController: ExceptionsController
@Inject lateinit var performanceMetricsController: PerformanceMetricsController
@Inject lateinit var logUploadWorkerFactory: LogUploadWorkerFactory
@Inject lateinit var dataProviders: DataProviders
@Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers
Expand All @@ -105,6 +115,13 @@ class LogUploadWorkerTest {
.setTimestamp(TEST_TIMESTAMP)
.build()

private val apkSizeTestLoggableMetric = OppiaMetricLog.LoggableMetric.newBuilder()
.setApkSizeMetric(
OppiaMetricLog.ApkSizeMetric.newBuilder()
.setApkSizeBytes(TEST_APK_SIZE)
.build()
).build()

private val exception = Exception("TEST")

@Before
Expand Down Expand Up @@ -203,6 +220,46 @@ class LogUploadWorkerTest {
assertThat(loggedExceptionStackTraceElems).isEqualTo(expectedExceptionStackTraceElems)
}

@Test
fun testWorker_logPerformanceMetric_withoutNetwork_enqueueRequest_verifySuccess() {
networkConnectionUtil.setCurrentConnectionStatus(NONE)
performanceMetricsController.logPerformanceMetricsEvent(
TEST_TIMESTAMP,
OppiaMetricLog.CurrentScreen.SCREEN_UNSPECIFIED,
apkSizeTestLoggableMetric,
OppiaMetricLog.Priority.LOW_PRIORITY
)

val workManager = WorkManager.getInstance(ApplicationProvider.getApplicationContext())

val inputData = Data.Builder().putString(
LogUploadWorker.WORKER_CASE_KEY,
LogUploadWorker.PERFORMANCE_METRICS_WORKER
).build()

val request: OneTimeWorkRequest = OneTimeWorkRequestBuilder<LogUploadWorker>()
.setInputData(inputData)
.build()
workManager.enqueue(request)
testCoroutineDispatchers.runCurrent()

val workInfo = workManager.getWorkInfoById(request.id)
val loggedPerformanceMetric =
fakePerformanceMetricsEventLogger.getMostRecentPerformanceMetricsEvent()
assertThat(workInfo.get().state).isEqualTo(WorkInfo.State.SUCCEEDED)
assertThat(loggedPerformanceMetric.loggableMetric.loggableMetricTypeCase).isEqualTo(
OppiaMetricLog.LoggableMetric.LoggableMetricTypeCase.APK_SIZE_METRIC
)
assertThat(loggedPerformanceMetric.currentScreen).isEqualTo(
OppiaMetricLog.CurrentScreen.SCREEN_UNSPECIFIED
)
assertThat(loggedPerformanceMetric.priority).isEqualTo(OppiaMetricLog.Priority.LOW_PRIORITY)
assertThat(loggedPerformanceMetric.timestampMillis).isEqualTo(TEST_TIMESTAMP)
assertThat(loggedPerformanceMetric.loggableMetric.apkSizeMetric.apkSizeBytes).isEqualTo(
TEST_APK_SIZE
)
}

@Test
fun testWorker_logEvent_withoutNetwork_enqueueRequest_verifyCorrectSyncStatusSequence() {
networkConnectionUtil.setCurrentConnectionStatus(NONE)
Expand Down Expand Up @@ -310,6 +367,11 @@ class LogUploadWorkerTest {

@Provides
fun bindFakeExceptionLogger(fakeLogger: FakeExceptionLogger): ExceptionLogger = fakeLogger

@Provides
fun bindFakePerformanceMetricsLogger(
fakePerformanceMetricsEventLogger: FakePerformanceMetricsEventLogger
): PerformanceMetricsEventLogger = fakePerformanceMetricsEventLogger
}

@Module
Expand All @@ -322,6 +384,10 @@ class LogUploadWorkerTest {
@Provides
@ExceptionLogStorageCacheSize
fun provideExceptionLogStorageSize(): Int = 2

@Provides
@PerformanceMetricsLogStorageCacheSize
fun providePerformanceMetricsLogStorageCacheSize(): Int = 2
}

@Module
Expand All @@ -340,7 +406,9 @@ class LogUploadWorkerTest {
TestFirebaseLogUploaderModule::class, FakeOppiaClockModule::class,
NetworkConnectionUtilDebugModule::class, LocaleProdModule::class, LoggerModule::class,
AssetModule::class, PlatformParameterModule::class, PlatformParameterSingletonModule::class,
LoggingIdentifierModule::class, SyncStatusTestModule::class, ApplicationLifecycleModule::class
LoggingIdentifierModule::class, SyncStatusTestModule::class,
PerformanceMetricsAssessorModule::class, ApplicationLifecycleModule::class,
PerformanceMetricsConfigurationsModule::class
]
)
interface TestApplicationComponent : DataProvidersInjector {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ kt_android_library(
visibility = ["//:oppia_api_visibility"],
deps = [
"//model/src/main/proto:event_logger_java_proto_lite",
"//model/src/main/proto:performance_metrics_event_logger_java_proto_lite",
"//third_party:javax_inject_javax_inject",
"//utility",
],
Expand Down
Loading

0 comments on commit 3839846

Please sign in to comment.