Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -6,6 +6,7 @@

package com.datadog.android.profiling.internal

import com.datadog.android.api.InternalLogger
import com.datadog.android.api.context.DatadogContext
import com.datadog.android.api.feature.Feature
import com.datadog.android.api.feature.FeatureSdkCore
Expand All @@ -32,32 +33,31 @@ internal class ProfilingDataWriter(
anrEvents: List<ProfilerEvent.RumAnrEvent>,
vitalEvents: List<ProfilerEvent.RumVitalEvent>
) {
writeWithContext { context ->
buildRawBatchEvent(
context = context,
profilingResult = profilingResult,
longTaskEvents = longTasks,
anrEvents = anrEvents,
vitalEvents = vitalEvents
)
val feature = sdkCore.getFeature(Feature.PROFILING_FEATURE_NAME)
if (feature == null) {
safeDelete(profilingResult.resultFilePath)
return
}
}

private fun writeWithContext(rawBatchEventBuilder: (DatadogContext) -> RawBatchEvent?) {
sdkCore.getFeature(Feature.Companion.PROFILING_FEATURE_NAME)
?.withWriteContext { context, writeScope ->
writeScope { writer ->
rawBatchEventBuilder(context)?.let {
synchronized(this) {
writer.write(
event = it,
batchMetadata = null,
eventType = EventType.DEFAULT
)
}
feature.withWriteContext { context, writeScope ->
writeScope { writer ->
synchronized(this) {
buildRawBatchEvent(
context = context,
profilingResult = profilingResult,
longTaskEvents = longTasks,
anrEvents = anrEvents,
vitalEvents = vitalEvents
)?.let {
writer.write(
event = it,
batchMetadata = null,
eventType = EventType.DEFAULT
)
}
safeDelete(profilingResult.resultFilePath)
}
}
}
Comment on lines +41 to +60

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently we don't have a retry mechanism for the writing failure, it will be implemented later as described above, in this case, instead of filling up the user's storage, we need to delete the file even though the write fails.

}

private fun buildRawBatchEvent(
Expand Down Expand Up @@ -210,7 +210,29 @@ internal class ProfilingDataWriter(
return File(profilingPath).readBytesSafe(internalLogger = sdkCore.internalLogger)
}

private fun safeDelete(path: String) {
try {
@Suppress("UnsafeThirdPartyFunctionCall")
val deleted = File(path).delete()
if (!deleted) {
sdkCore.internalLogger.log(
InternalLogger.Level.WARN,
InternalLogger.Target.MAINTAINER,
{ LOG_FILE_DELETE_FAILED.format(path) }
)
}
} catch (@Suppress("TooGenericExceptionCaught") t: Throwable) {
sdkCore.internalLogger.log(
InternalLogger.Level.WARN,
InternalLogger.Target.MAINTAINER,
{ LOG_FILE_DELETE_FAILED.format(path) },
t
)
}
}

companion object {
private const val LOG_FILE_DELETE_FAILED = "Failed to delete Perfetto trace file: %s"
private const val TAG_KEY_SERVICE = "service"
private const val TAG_KEY_VERSION = "version"
private const val TAG_KEY_BUILD_ID = "build_id"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ internal class ProfilingFeature(
}
}

@Suppress("ReturnCount")
private fun tryWriteProfilingEvent() {
val result = perfettoResult ?: return
when (result.startReason) {
Expand All @@ -261,23 +260,23 @@ internal class ProfilingFeature(
val scheduler = continuousProfilingScheduler ?: return
scheduler.onActiveWindowEnded()
val (longTasks, anrEvents, vitalEvents) = pendingRumEvents.drain()
if (longTasks.isEmpty() && anrEvents.isEmpty() && vitalEvents.isEmpty()) {
logToUser(LOG_CONTINUOUS_PROFILING_DROPPED_NO_RUM_EVENTS)
return
}
dataWriter.write(
profilingResult = result,
longTasks = longTasks,
anrEvents = anrEvents,
vitalEvents = vitalEvents
)
logToUser(
LOG_CONTINUOUS_PROFILING_WRITTEN.format(
Locale.US,
longTasks.size,
anrEvents.size
if (longTasks.isEmpty() && anrEvents.isEmpty() && vitalEvents.isEmpty()) {
logToUser(LOG_CONTINUOUS_PROFILING_NOT_UPLOADED_NO_RUM_EVENTS)
} else {
logToUser(
LOG_CONTINUOUS_PROFILING_WRITTEN.format(
Locale.US,
longTasks.size,
anrEvents.size
)
)
)
}
}

else -> {
Expand Down Expand Up @@ -311,8 +310,8 @@ internal class ProfilingFeature(
"Profiling feature received an event of unsupported type=%s."
private const val LOG_LAUNCH_PROFILING_STOPPED_AT_TTID =
"Launch profiling stopped at TTID."
private const val LOG_CONTINUOUS_PROFILING_DROPPED_NO_RUM_EVENTS =
"Continuous profiling result dropped: no pending RUM events."
private const val LOG_CONTINUOUS_PROFILING_NOT_UPLOADED_NO_RUM_EVENTS =
"Continuous profiling result not uploaded: no pending RUM events."
private const val LOG_CONTINUOUS_PROFILING_WRITTEN =
"Continuous profiling result written: %d long task(s), %d ANR event(s)."
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ internal class ProfilingFeatureTest {
@Mock
private lateinit var mockProfiler: Profiler

@Mock
private lateinit var mockProfilingFeatureScope: FeatureScope

@Mock
private lateinit var mockRumFeatureScope: FeatureScope

Expand Down Expand Up @@ -559,30 +556,44 @@ internal class ProfilingFeatureTest {
}

@Test
fun `M skip writing W continuous profiling result received {no RUM events}`(
fun `M write with empty events W continuous profiling result received {no RUM events}`(
@Forgery fakePerfettoResult: PerfettoResult
) {
// Given
testedFeature = ProfilingFeature(mockSdkCore, fakeAllSampledConfiguration, mockProfiler)
testedFeature.dataWriter = mockDataWriter
whenever(mockProfiler.isRunning(fakeInstanceName)) doReturn true
whenever(mockSdkCore.getFeature(Feature.PROFILING_FEATURE_NAME)) doReturn mockProfilingFeatureScope
val callbackCaptor = argumentCaptor<ProfilerCallback>()
testedFeature.onInitialize(mockContext)
testedFeature.dataWriter = mockDataWriter
verify(mockProfiler).registerProfilingCallback(
eq(mockContext),
eq(fakeInstanceName),
callbackCaptor.capture()
)
testedFeature.onReceive(fakeTTID)

// When
callbackCaptor.firstValue.onSuccess(
fakePerfettoResult.copy(startReason = ProfilingStartReason.CONTINUOUS)
)

// Then
verifyNoInteractions(mockDataWriter)
verify(mockDataWriter).write(
profilingResult = fakePerfettoResult.copy(startReason = ProfilingStartReason.CONTINUOUS),
longTasks = emptyList(),
anrEvents = emptyList(),
vitalEvents = emptyList()
)
val logCaptor = argumentCaptor<() -> String>()
verify(mockInternalLogger, atLeastOnce()).log(
eq(InternalLogger.Level.DEBUG),
eq(InternalLogger.Target.USER),
logCaptor.capture(),
isNull(),
eq(false),
isNull()
)
assertThat(logCaptor.allValues.map { it.invoke() })
.contains("Continuous profiling result not uploaded: no pending RUM events.")
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.eq
import org.mockito.kotlin.isNull
import org.mockito.kotlin.verifyNoInteractions
import org.mockito.kotlin.verifyNoMoreInteractions
import org.mockito.kotlin.whenever
import org.mockito.quality.Strictness
Expand Down Expand Up @@ -233,28 +234,37 @@ internal class ProfilingDataWriterTest {
assertThat(vital).hasDurationNs(fakeVital.durationNs)
}
verifyNoMoreInteractions(mockEventBatchWriter)
assertThat(file.exists()).isFalse()
}

@Test
fun `M skip writing W write {can't read perfetto File}`(
fun `M skip writing and log warn on delete W write {perfetto file not found}`(
@Forgery fakeResult: PerfettoResult,
@Forgery fakeVitals: List<ProfilerEvent.RumVitalEvent>,
@Forgery fakeLongTasks: List<ProfilerEvent.RumLongTaskEvent>,
@Forgery fakeAnrs: List<ProfilerEvent.RumAnrEvent>
) {
// Given
// Don't create the tmp file so it can't be found
// Given — file path exists in TempDir but file is never created
val nonExistentFile = File(tmp, "nonexistent.perfetto-stack-sample")

// When
testedDataWriterTest.write(
profilingResult = fakeResult,
profilingResult = fakeResult.copy(resultFilePath = nonExistentFile.absolutePath),
vitalEvents = fakeVitals,
anrEvents = fakeAnrs,
longTasks = fakeLongTasks
)

// Then
verifyNoMoreInteractions(mockInternalLogger, mockEventBatchWriter)
verify(mockInternalLogger).log(
eq(InternalLogger.Level.WARN),
eq(InternalLogger.Target.MAINTAINER),
any<() -> String>(),
isNull(),
eq(false),
isNull()
)
verifyNoMoreInteractions(mockEventBatchWriter)
}

@Test
Expand All @@ -277,6 +287,7 @@ internal class ProfilingDataWriterTest {
)

// Then
assertThat(file.exists()).isFalse()
verifyNoMoreInteractions(mockInternalLogger, mockEventBatchWriter)
}

Expand All @@ -298,6 +309,7 @@ internal class ProfilingDataWriterTest {
)

// Then
assertThat(file.exists()).isFalse()
verifyNoMoreInteractions(mockInternalLogger, mockEventBatchWriter)
}

Expand Down Expand Up @@ -362,6 +374,62 @@ internal class ProfilingDataWriterTest {
}
assertThat(actualMetadataEvents.none { it.type == RumMetadataEvent.Type.ERROR }).isTrue()
assertThat(actualMetadataEvents.none { it.type == RumMetadataEvent.Type.LONG_TASK }).isTrue()
assertThat(file.exists()).isFalse()
verifyNoMoreInteractions(mockEventBatchWriter)
}

@Test
fun `M delete result file W write {feature not initialized}`(
@Forgery fakeResult: PerfettoResult,
forge: Forge
) {
// Given
whenever(mockSdkCore.getFeature(Feature.PROFILING_FEATURE_NAME)) doReturn null
val file = File(tmp, "fake_profile.perfetto-stack-sample")
file.writeBytes(forge.aString().toByteArray())

// When
testedDataWriterTest.write(
profilingResult = fakeResult.copy(resultFilePath = file.absolutePath),
vitalEvents = emptyList(),
anrEvents = emptyList(),
longTasks = emptyList()
)

// Then
assertThat(file.exists()).isFalse()
verifyNoInteractions(mockEventBatchWriter)
}

@Test
fun `M delete result file W write {events present}`(
@Forgery fakeResult: PerfettoResult,
@Forgery fakeVitals: List<ProfilerEvent.RumVitalEvent>,
forge: Forge
) {
// Given
val file = File(tmp, "fake_profile.perfetto-stack-sample")
file.writeBytes(forge.aString().toByteArray())
val rumContext = fakeVitals.first().rumContext
val alignedVitals = fakeVitals.map {
it.copy(
rumContext = it.rumContext.copy(
applicationId = rumContext.applicationId,
sessionId = rumContext.sessionId
)
)
}

// When
testedDataWriterTest.write(
profilingResult = fakeResult.copy(resultFilePath = file.absolutePath),
vitalEvents = alignedVitals,
anrEvents = emptyList(),
longTasks = emptyList()
)

// Then
assertThat(file.exists()).isFalse()
verify(mockEventBatchWriter).write(any(), isNull(), eq(EventType.DEFAULT))
}
}
Loading