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

Avoid sending empty profiles #2232

Merged
merged 2 commits into from
Sep 6, 2022
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Avoid sending empty profiles ([#2232](https://github.com/getsentry/sentry-java/pull/2232))

## 6.4.1

### Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import io.sentry.SentryEvent
import io.sentry.SentryOptions
import io.sentry.protocol.SentryTransaction
import org.junit.runner.RunWith
import java.io.File
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue
Expand Down Expand Up @@ -65,6 +66,32 @@ class EnvelopeTests : BaseUiTest() {
}
}

@Test
fun checkProfileNotSentIfEmpty() {

initSentry(true) { options: SentryOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
}
relayIdlingResource.increment()
val transaction = Sentry.startTransaction("e2etests", "test empty")
transaction.finish()
// Let's modify the trace file to be empty, so that the profile will actually be empty.
val profilesDirPath = Sentry.getCurrentHub().options.profilingTracesDirPath
val origProfileFile = File(profilesDirPath!!).listFiles()?.maxByOrNull { f -> f.lastModified() }
origProfileFile?.writeBytes(ByteArray(0))

relay.assert {
assertEnvelope {
it.assertItem<SentryTransaction>()
// Since the profile is empty, it is discarded and not sent to Sentry
it.assertNoOtherItems()
}
assertNoOtherEnvelopes()
assertNoOtherRequests()
}
}

@Test
fun sendProfiledTransaction() {
// This is a dogfooding test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ProfilingActivity : AppCompatActivity() {

private lateinit var binding: ActivityProfilingBinding
private val executors = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors())
private var profileFinished = false
private var profileFinished = true

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
Expand Down
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/SentryEnvelopeItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ public static SentryEnvelopeItem fromAttachment(
// base64
byte[] traceFileBytes = readBytesFromFile(traceFile.getPath(), maxTraceFileSize);
String base64Trace = Base64.encodeToString(traceFileBytes, NO_WRAP | NO_PADDING);
if (base64Trace.isEmpty()) {
throw new SentryEnvelopeException("Profiling trace file is empty");
}
profilingTraceData.setSampledProfile(base64Trace);
profilingTraceData.readDeviceCpuFrequencies();

Expand Down
9 changes: 9 additions & 0 deletions sentry/src/test/java/io/sentry/SentryClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class SentryClientTest {

fun getSut(optionsCallback: ((SentryOptions) -> Unit)? = null): SentryClient {
optionsCallback?.invoke(sentryOptions)
profilingTraceFile.writeText("sampledProfile")
return SentryClient(sentryOptions)
}
}
Expand Down Expand Up @@ -1055,6 +1056,14 @@ class SentryClientTest {
verifyProfilingTraceInEnvelope(transaction.eventId)
}

@Test
fun `when captureTransaction with empty trace file, profiling data is not sent`() {
val transaction = SentryTransaction(fixture.sentryTracer)
fixture.getSut().captureTransaction(transaction, null, null, null, fixture.profilingTraceData)
fixture.profilingTraceFile.writeText("")
assertFails { verifyProfilingTraceInEnvelope(transaction.eventId) }
}

@Test
fun `when captureTransaction with non existing profiling trace file, profiling trace data is not sent`() {
val transaction = SentryTransaction(fixture.sentryTracer)
Expand Down