Skip to content

fix(rum): keep remote settings across app upgrades #37

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
Fiona2016 merged 1 commit into from
Oct 18, 2026
Merged

fix(rum): keep remote settings across app upgrades #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion ...m/src/main/kotlin/com/datadog/android/rum/internal/remoteconfig/RemoteConfigController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ internal class RemoteConfigController(

private val inFlight = AtomicBoolean(false)

/** Whether the entries of app versions this device no longer runs have been cleared yet. */
/** Whether the entries this device no longer reads have been cleared yet. */
private val swept = AtomicBoolean(false)
private var failedAttempts = 0
private var pendingRetry: ScheduledFuture<*>? = null
Expand Down
40 changes: 24 additions & 16 deletions ...id-rum/src/main/kotlin/com/datadog/android/rum/internal/remoteconfig/RemoteConfigStore.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,13 @@ internal class RemoteConfigStore(
}

/**
* Removes the entries of app versions this device is no longer running.
* Removes the entries this device no longer reads: those of an endpoint, application, service
* or environment the app has stopped using, and those written by releases that still keyed the
* entry by app version, which no current key can reach.
*
* The key covers the app version, so every release the device installs leaves one behind, and
* nothing read or removed them again — they accumulated for good inside a preferences file
* that is parsed in full at every launch. Age is what separates an abandoned entry from a live
* one: a store that is still being read is also being written, at the latest by [touch] when
* Nothing else reads or removes them, so they would accumulate for good inside a preferences
* file that is parsed in full at every launch. Age is what separates an abandoned entry from a
* live one: a store that is still being read is also being written, at the latest by [touch] when
* its answer comes back unchanged.
*
* This store's own entry is never a candidate, whatever its age says. It is the one entry that
Expand Down Expand Up @@ -212,7 +213,7 @@ internal class RemoteConfigStore(
/**
* The entry a stored key belongs to, or null when the key is not one of ours. Splitting on the
* known suffixes rather than on the last separator is what keeps a store key free to contain
* one: a service name and an app version both routinely do.
* one: a service name and an environment name both routinely do.
*/
private fun entryKeyOf(key: String): String? {
if (!key.startsWith(STORE_KEY_PREFIX)) return null
Expand Down Expand Up @@ -325,15 +326,23 @@ internal class RemoteConfigStore(
"Unable to open the remote configuration store; the values passed to init will apply."

/**
* Identifies whose configuration this is. It covers everything that can change the answer —
* which endpoint the app asks, which application, in which environment, at which app
* version — so an app that ships a new version, or two applications sharing a device, never
* read each other's values.
* Identifies whose configuration this is: which endpoint the app asks, which application,
* which service, in which environment — so two applications or two environments sharing a
* device never read each other's values.
*
* It deliberately leaves out the SDK version: including it would discard the stored values
* on every SDK upgrade and put the first session after an upgrade back on the init values.
* The storage format version lives in [STORE_KEY_PREFIX] instead, so only a real format
* change orphans the cache.
* It deliberately leaves out the app version. An installed app runs one version at a time,
* and keying by version would send the first session after every upgrade to the init values
* with no settings version to report, even though the previous release's settings are a
* better answer than that until the new fetch is stored. The console does not target by app
* version either; if it ever did, the response stored here is replaced by the first fetch
* of the new version anyway.
*
* It also leaves out the SDK version, for the same reason. The storage format version lives
* in [STORE_KEY_PREFIX] instead, so only a real format change orphans the cache.
*
* Entries written under the earlier layout, which ended in the app version, are not reached
* by this key and are left for [sweepAbandoned]; the format did not change, so there is
* nothing for new code to misparse and no reason to bump the prefix.
*/
fun buildStoreKey(context: DatadogContext, intakeUrl: String, applicationId: String): String {
val host = try {
Expand All @@ -348,8 +357,7 @@ internal class RemoteConfigStore(
host,
applicationId,
context.service,
context.env,
context.version
context.env
).joinToString("|")
}
}
Expand Down
88 changes: 69 additions & 19 deletions ...um/src/test/kotlin/com/datadog/android/rum/internal/remoteconfig/RemoteConfigStoreTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,53 @@ internal class RemoteConfigStoreTest {
assertThat(key).contains("app-1")
assertThat(key).contains(SERVICE)
assertThat(key).contains(ENV)
assertThat(key).contains(APP_VERSION)
}

@Test
fun `M leave the app version out of the key W buildStoreKey()`() {
// Including it would send the first session after every app upgrade to the init values.
val key = RemoteConfigStore.buildStoreKey(
context = datadogContext(),
intakeUrl = "https://rum.example.com/api/v2/rum",
applicationId = "app-1"
)

assertThat(key).doesNotContain(APP_VERSION)
}

@Test
fun `M produce the same key W buildStoreKey() { app version changed }`() {
val intakeUrl = "https://rum.example.com/api/v2/rum"

val before = RemoteConfigStore.buildStoreKey(datadogContext(version = "1.2.3"), intakeUrl, "app-1")
val after = RemoteConfigStore.buildStoreKey(datadogContext(version = "1.3.0"), intakeUrl, "app-1")

assertThat(after).isEqualTo(before)
}

@Test
fun `M produce different keys W buildStoreKey() { application, environment or service differs }`() {
val intakeUrl = "https://rum.example.com/api/v2/rum"
val base = RemoteConfigStore.buildStoreKey(datadogContext(), intakeUrl, "app-1")

assertThat(RemoteConfigStore.buildStoreKey(datadogContext(), intakeUrl, "app-2")).isNotEqualTo(base)
assertThat(RemoteConfigStore.buildStoreKey(datadogContext(env = "prod"), intakeUrl, "app-1"))
.isNotEqualTo(base)
assertThat(RemoteConfigStore.buildStoreKey(datadogContext(service = "other"), intakeUrl, "app-1"))
.isNotEqualTo(base)
}

@Test
fun `M read a stored value W snapshot() { app version changed between launches }`() {
val intakeUrl = "https://rum.example.com/api/v2/rum"
val oldKey = RemoteConfigStore.buildStoreKey(datadogContext(version = "1.2.3"), intakeUrl, "app-1")
val newKey = RemoteConfigStore.buildStoreKey(datadogContext(version = "1.3.0"), intakeUrl, "app-1")
val values = RemoteConfigValues(25f, 4, etag = "\"v4\"")
RemoteConfigStore(appContext, oldKey, mock<InternalLogger>()).store(values)

val snapshot = RemoteConfigStore(appContext, newKey, mock<InternalLogger>()).snapshot()

assertThat(snapshot).isEqualTo(values)
}

@Test
Expand Down Expand Up @@ -165,39 +211,39 @@ internal class RemoteConfigStoreTest {

// endregion

// region sweeping the entries of versions this device no longer runs
// region sweeping the entries this device no longer reads

@Test
fun `M remove an entry nothing has refreshed for longer than the threshold W sweepAbandoned()`() {
// The key covers the app version, so every release the device installs leaves one behind.
storeUnder(OTHER_VERSION_KEY, atMs = NOW - THREE_DAYS_MS)
// An entry written under the earlier layout, which ended in the app version.
storeUnder(LEGACY_VERSIONED_KEY, atMs = NOW - THREE_DAYS_MS)
val store = testedStore(nowMs = NOW)

store.sweepAbandoned()

assertThat(preferences.all.keys.filter { it.startsWith(OTHER_VERSION_KEY) }).isEmpty()
assertThat(preferences.all.keys.filter { it.startsWith(LEGACY_VERSIONED_KEY) }).isEmpty()
}

@Test
fun `M keep an entry something refreshed recently W sweepAbandoned()`() {
storeUnder(OTHER_VERSION_KEY, atMs = NOW - ONE_DAY_MS)
storeUnder(LEGACY_VERSIONED_KEY, atMs = NOW - ONE_DAY_MS)
val store = testedStore(nowMs = NOW)

store.sweepAbandoned()

assertThat(preferences.all.keys.filter { it.startsWith(OTHER_VERSION_KEY) }).isNotEmpty()
assertThat(preferences.all.keys.filter { it.startsWith(LEGACY_VERSIONED_KEY) }).isNotEmpty()
}

@Test
fun `M remove every key of an abandoned entry W sweepAbandoned()`() {
// An entry is spread over several keys, and leaving any of them behind leaks just as well.
storeUnder(OTHER_VERSION_KEY, atMs = NOW - THREE_DAYS_MS)
val before = preferences.all.keys.count { it.startsWith(OTHER_VERSION_KEY) }
storeUnder(LEGACY_VERSIONED_KEY, atMs = NOW - THREE_DAYS_MS)
val before = preferences.all.keys.count { it.startsWith(LEGACY_VERSIONED_KEY) }

testedStore(nowMs = NOW).sweepAbandoned()

assertThat(before).isGreaterThan(1)
assertThat(preferences.all.keys.none { it.startsWith(OTHER_VERSION_KEY) }).isTrue()
assertThat(preferences.all.keys.none { it.startsWith(LEGACY_VERSIONED_KEY) }).isTrue()
}

@Test
Expand All @@ -215,7 +261,7 @@ internal class RemoteConfigStoreTest {
@Test
fun `M leave alone every key it did not write W sweepAbandoned()`() {
preferences.edit().putString("a-key-the-host-app-owns", "not ours").apply()
storeUnder(OTHER_VERSION_KEY, atMs = NOW - THREE_DAYS_MS)
storeUnder(LEGACY_VERSIONED_KEY, atMs = NOW - THREE_DAYS_MS)

testedStore(nowMs = NOW).sweepAbandoned()

Expand All @@ -224,9 +270,9 @@ internal class RemoteConfigStoreTest {

@Test
fun `M take a store key containing separators apart correctly W sweepAbandoned()`() {
// A service name and an app version both routinely contain dots, so the suffix cannot be
// A service name and an environment name both routinely contain dots, so the suffix cannot be
// found by splitting on the last one.
val dotted = "${RemoteConfigStore.STORE_KEY_PREFIX}host|app|com.example.shop|prod|1.2.3"
val dotted = "${RemoteConfigStore.STORE_KEY_PREFIX}host|app|com.example.shop|prod.eu"
storeUnder(dotted, atMs = NOW - THREE_DAYS_MS)

testedStore(nowMs = NOW).sweepAbandoned()
Expand Down Expand Up @@ -273,11 +319,15 @@ internal class RemoteConfigStoreTest {
RemoteConfigStore(appContext, STORE_KEY, mock<InternalLogger>()) { nowMs }
}

private fun datadogContext(): DatadogContext {
private fun datadogContext(
service: String = SERVICE,
env: String = ENV,
version: String = APP_VERSION
): DatadogContext {
val context = mock<DatadogContext>()
whenever(context.service).thenReturn(SERVICE)
whenever(context.env).thenReturn(ENV)
whenever(context.version).thenReturn(APP_VERSION)
whenever(context.service).thenReturn(service)
whenever(context.env).thenReturn(env)
whenever(context.version).thenReturn(version)
whenever(context.sdkVersion).thenReturn(SDK_VERSION)
return context
}
Expand Down Expand Up @@ -346,8 +396,8 @@ internal class RemoteConfigStoreTest {
// Shaped like a real store key, and carrying the prefix, so the sweep considers it at all:
// a key it skips for the wrong reason would let the "never remove its own entry" test pass
// with the guard gone.
private val STORE_KEY = "${RemoteConfigStore.STORE_KEY_PREFIX}host|app|shop|prod|1.0.0"
private val OTHER_VERSION_KEY = "${RemoteConfigStore.STORE_KEY_PREFIX}host|app|shop|prod|0.9.0"
private val STORE_KEY = "${RemoteConfigStore.STORE_KEY_PREFIX}host|app|shop|prod"
private val LEGACY_VERSIONED_KEY = "${RemoteConfigStore.STORE_KEY_PREFIX}host|app|shop|prod|0.9.0"

private const val NOW = 1_800_000_000_000L
private const val ONE_DAY_MS = 24L * 60 * 60 * 1000
Expand Down
Loading