Skip to content

[andr] Move expensive operations on start to a bg thread #508

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jul 17, 2025
Merged

[andr] Move expensive operations on start to a bg thread #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
20053d4
Move expensive operations on start to a bg thread
murki Jul 17, 2025
55a0145
fmt
murki Jul 17, 2025
daae278
prefer default executor
murki Jul 17, 2025
3483077
lazify
murki Jul 17, 2025
9d2af08
fix thread name
murki Jul 17, 2025
4fa817c
fmt
murki Jul 17, 2025
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
8 changes: 6 additions & 2 deletions platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,10 @@ internal class LoggerImpl(
CaptureJniLibrary.startLogger(this.loggerId)
}

writeSdkStartLog(context, clientAttributes, initDuration = duration)
val captureStartThread = Thread.currentThread().name
eventListenerDispatcher.executorService.execute {
writeSdkStartLog(context, clientAttributes, duration, captureStartThread)
}
}

override val sessionId: String
Expand Down Expand Up @@ -488,6 +491,7 @@ internal class LoggerImpl(
appContext: Context,
clientAttributes: ClientAttributes,
initDuration: Duration,
captureStartThread: String,
) {
val installationSource =
clientAttributes
Expand All @@ -499,7 +503,7 @@ internal class LoggerImpl(
putAll(
mapOf(
"_app_installation_source" to installationSource,
"_capture_start_thread" to Thread.currentThread().name.toFieldValue(),
"_capture_start_thread" to captureStartThread.toFieldValue(),
),
)
putAll(fatalIssueReporter.getLogStatusFieldsMap())
Expand Down
16 changes: 12 additions & 4 deletions platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/attributes/NetworkAttributes.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ import android.telephony.TelephonyManager.NETWORK_TYPE_UNKNOWN
import androidx.core.content.ContextCompat
import io.bitdrift.capture.providers.FieldProvider
import io.bitdrift.capture.providers.Fields
import io.bitdrift.capture.threading.CaptureDispatchers
import java.util.concurrent.ExecutorService
import java.util.concurrent.atomic.AtomicReference

@SuppressLint("MissingPermission")
internal class NetworkAttributes(
private val context: Context,
executor: ExecutorService = CaptureDispatchers.CommonBackground.executorService,
) : ConnectivityManager.NetworkCallback(),
FieldProvider {
@SuppressLint("InlinedApi")
Expand All @@ -72,11 +75,17 @@ internal class NetworkAttributes(
NETWORK_TYPE_UNKNOWN to "unknown",
)
private val currentNetworkType: AtomicReference<String> = AtomicReference("unknown")
private val telephonyManager = context.getSystemService(Context.TELEPHONY_SERVICE) as TelephonyManager
private val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
private val telephonyManager by lazy {
context.getSystemService(Context.TELEPHONY_SERVICE) as TelephonyManager
}
private val connectivityManager by lazy {
context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
}

init {
monitorNetworkType()
executor.execute {
Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

Since monitorNetworkType() is deferred to a background thread, invoke() may return an outdated network_type if it runs before this task completes. Consider synchronizing initial execution or documenting this behavior so consumers understand the potential race window.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@murki murki Jul 17, 2025

Choose a reason for hiding this comment

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

This is fine since as a default value we have "unknown" which might happen for the first log or so if there's a race condition. In my tests above that wasn't the case though

monitorNetworkType()
}
}

override fun invoke(): Fields =
Expand All @@ -100,7 +109,6 @@ internal class NetworkAttributes(
} catch (e: Throwable) {
// Issue with some versions of Android: https://issuetracker.google.com/issues/175055271
// can sometime throw an exception: "package does not belong to 10006"
// We'll also exercise this path when api level < 23
updateNetworkType(NetworkCapabilities(null))
}
}
Expand Down
15 changes: 8 additions & 7 deletions platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/NetworkAttributesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import android.content.Context
import android.net.ConnectivityManager
import android.net.Network
import androidx.test.core.app.ApplicationProvider
import com.google.common.util.concurrent.MoreExecutors
import com.nhaarman.mockitokotlin2.verify
import io.bitdrift.capture.attributes.NetworkAttributes
import org.assertj.core.api.Assertions.assertThat
Expand All @@ -34,7 +35,7 @@ class NetworkAttributesTest {
fun carrier() {
val context = ApplicationProvider.getApplicationContext<Context>()

val networkAttributes = NetworkAttributes(context).invoke()
val networkAttributes = NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

assertThat(networkAttributes).containsEntry("carrier", "")
}
Expand All @@ -44,7 +45,7 @@ class NetworkAttributesTest {
grantPermissions(Manifest.permission.ACCESS_NETWORK_STATE)
val context = ApplicationProvider.getApplicationContext<Context>()

val networkAttributes = NetworkAttributes(context).invoke()
val networkAttributes = NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

assertThat(networkAttributes).containsEntry("network_type", "wwan")
}
Expand All @@ -53,7 +54,7 @@ class NetworkAttributesTest {
fun network_type_access_network_state_not_granted() {
val context = ApplicationProvider.getApplicationContext<Context>()

val networkAttributes = NetworkAttributes(context).invoke()
val networkAttributes = NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

assertThat(networkAttributes).containsEntry("network_type", "unknown")
}
Expand All @@ -66,7 +67,7 @@ class NetworkAttributesTest {
val mockedActiveNetwork = obtainMockedActiveNetwork(mockedConnectivityManager)
doReturn(null).`when`(mockedConnectivityManager).getNetworkCapabilities(eq(mockedActiveNetwork))

val networkAttributes = NetworkAttributes(context).invoke()
val networkAttributes = NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

assertThat(networkAttributes).containsEntry("network_type", "unknown")
}
Expand All @@ -77,7 +78,7 @@ class NetworkAttributesTest {
val context = spy(ApplicationProvider.getApplicationContext<Context>())
val mockedConnectivityManager = obtainMockedConnectivityManager(context)

NetworkAttributes(context).invoke()
NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

verify(mockedConnectivityManager).registerDefaultNetworkCallback(
any(ConnectivityManager.NetworkCallback::class.java),
Expand All @@ -89,7 +90,7 @@ class NetworkAttributesTest {
grantPermissions(Manifest.permission.READ_PHONE_STATE)
val context = ApplicationProvider.getApplicationContext<Context>()

val networkAttributes = NetworkAttributes(context).invoke()
val networkAttributes = NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

assertThat(networkAttributes).containsEntry("radio_type", "unknown")
}
Expand All @@ -98,7 +99,7 @@ class NetworkAttributesTest {
fun radio_type_read_phone_state_not_granted() {
val context = ApplicationProvider.getApplicationContext<Context>()

val networkAttributes = NetworkAttributes(context).invoke()
val networkAttributes = NetworkAttributes(context, MoreExecutors.newDirectExecutorService()).invoke()

assertThat(networkAttributes).containsEntry("radio_type", "forbidden")
}
Expand Down
Loading