From 5fc82d5e677d4e41f83f0f4307e4dd3b17cc5662 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:26:33 +0000 Subject: [PATCH] fix(analytics): drop the package from RUM view names (#7525) --- .../meshtastic/core/navigation/RumViewName.kt | 22 ++++++++----- .../core/navigation/RumViewNameTest.kt | 31 ++++++++++++++----- .../core/repository/PlatformAnalytics.kt | 2 +- .../ui/component/ScreenViewTrackerTest.kt | 18 +++++------ 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/RumViewName.kt b/core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/RumViewName.kt index 4ac78333bc..51391fb559 100644 --- a/core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/RumViewName.kt +++ b/core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/RumViewName.kt @@ -19,11 +19,19 @@ package org.meshtastic.core.navigation import androidx.navigation3.runtime.NavKey /** - * Derives the analytics view name for a navigation destination. - * - * The name is the route's fully-qualified class name (e.g. `org.meshtastic.core.navigation.NodesRoute.Nodes`), matching - * the convention historically recorded by Datadog RUM before the Navigation 3 migration, so new per-screen data lines - * up with existing dashboards. Falls back to the simple name (and finally `toString()`) on the rare platform where - * [kotlin.reflect.KClass.qualifiedName] is unavailable. + * Derives the analytics view name for a navigation destination: the route's class name without its package, keeping the + * enclosing route interface so leaf names stay unique (e.g. `NodesRoute.Nodes`, `SettingsRoute.Bluetooth`). */ -fun NavKey.rumViewName(): String = this::class.qualifiedName ?: this::class.simpleName ?: toString() +fun NavKey.rumViewName(): String = rumViewName(this::class.qualifiedName ?: this::class.simpleName ?: toString()) + +/** + * Strips the package from [className], treating leading lowercase segments as the package. Minified builds can report + * the JVM binary name (`NodesRoute$Nodes`), so `$` is normalised to `.` to give every build type the same name. + */ +internal fun rumViewName(className: String): String = className + .split('.', '$') + .dropWhile { it.firstOrNull()?.isLowerCase() == true } + .joinToString(".") + .ifEmpty { + className + } diff --git a/core/navigation/src/commonTest/kotlin/org/meshtastic/core/navigation/RumViewNameTest.kt b/core/navigation/src/commonTest/kotlin/org/meshtastic/core/navigation/RumViewNameTest.kt index 60f2d72a26..a4537f12c0 100644 --- a/core/navigation/src/commonTest/kotlin/org/meshtastic/core/navigation/RumViewNameTest.kt +++ b/core/navigation/src/commonTest/kotlin/org/meshtastic/core/navigation/RumViewNameTest.kt @@ -16,25 +16,40 @@ */ package org.meshtastic.core.navigation +import androidx.navigation3.runtime.NavKey import kotlin.test.Test import kotlin.test.assertEquals -/** - * Guards the RUM view-name convention consumed by the analytics layer. View names must be the route's fully-qualified - * class name so per-screen RUM data lines up with historical Datadog dashboards. A rename of a route interface or the - * package would break cross-platform data continuity, so this test pins the format. - */ +/** Pins the RUM view-name format that Datadog dashboards and monitors filter `@view.name` on. */ class RumViewNameTest { @Test - fun `rumViewName is the fully qualified route name for data objects`() { - assertEquals("org.meshtastic.core.navigation.NodesRoute.Nodes", NodesRoute.Nodes.rumViewName()) + fun `rumViewName drops the package and keeps the enclosing route`() { + assertEquals("NodesRoute.Nodes", NodesRoute.Nodes.rumViewName()) + assertEquals("SettingsRoute.Bluetooth", SettingsRoute.Bluetooth.rumViewName()) } @Test fun `rumViewName is stable across argument values for data classes`() { - val expected = "org.meshtastic.core.navigation.NodeDetailRoute.DeviceMetrics" + val expected = "NodeDetailRoute.DeviceMetrics" assertEquals(expected, NodeDetailRoute.DeviceMetrics(destNum = 1).rumViewName()) assertEquals(expected, NodeDetailRoute.DeviceMetrics(destNum = 2).rumViewName()) } + + @Test + fun `rumViewName of a top level key is its simple name`() { + assertEquals("TopLevelKey", TopLevelKey.rumViewName()) + } + + @Test + fun `binary names from minified builds match the qualified form`() { + assertEquals("NodesRoute.Nodes", rumViewName("org.meshtastic.core.navigation.NodesRoute\$Nodes")) + } + + @Test + fun `a name with no uppercase segment is kept whole`() { + assertEquals("a.b", rumViewName("a.b")) + } } + +private data object TopLevelKey : NavKey diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PlatformAnalytics.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PlatformAnalytics.kt index 733c70c7a5..5cb80cabc1 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PlatformAnalytics.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PlatformAnalytics.kt @@ -71,7 +71,7 @@ interface PlatformAnalytics { * [stopScreenView] using the same [key] when the screen is left. * * @param key A stable identifier that pairs this start with its matching [stopScreenView]. - * @param name The route-derived view name (e.g. `org.meshtastic.core.navigation.NodesRoute.Nodes`). + * @param name The route-derived view name (e.g. `NodesRoute.Nodes`). */ fun startScreenView(key: String, name: String) { // Default no-op for platforms that don't support RUM (fdroid, desktop) diff --git a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/ScreenViewTrackerTest.kt b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/ScreenViewTrackerTest.kt index 748ad3a849..c468ddbbd8 100644 --- a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/ScreenViewTrackerTest.kt +++ b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/ScreenViewTrackerTest.kt @@ -35,9 +35,9 @@ class ScreenViewTrackerTest { assertEquals( listOf( - "start:org.meshtastic.core.navigation.NodesRoute.Nodes:name=org.meshtastic.core.navigation.NodesRoute.Nodes", - "stop:org.meshtastic.core.navigation.NodesRoute.Nodes", - "start:org.meshtastic.core.navigation.NodeDetailRoute.DeviceMetrics:name=org.meshtastic.core.navigation.NodeDetailRoute.DeviceMetrics", + "start:NodesRoute.Nodes:name=NodesRoute.Nodes", + "stop:NodesRoute.Nodes", + "start:NodeDetailRoute.DeviceMetrics:name=NodeDetailRoute.DeviceMetrics", ), analytics.events, ) @@ -54,8 +54,8 @@ class ScreenViewTrackerTest { assertEquals( listOf( - "start:org.meshtastic.core.navigation.NodesRoute.Nodes:name=org.meshtastic.core.navigation.NodesRoute.Nodes", - "stop:org.meshtastic.core.navigation.NodesRoute.Nodes", + "start:NodesRoute.Nodes:name=NodesRoute.Nodes", + "stop:NodesRoute.Nodes", ), analytics.events, ) @@ -70,9 +70,7 @@ class ScreenViewTrackerTest { tracker.onCurrentKeyChanged(NodeDetailRoute.DeviceMetrics(destNum = 2)) assertEquals( - listOf( - "start:org.meshtastic.core.navigation.NodeDetailRoute.DeviceMetrics:name=org.meshtastic.core.navigation.NodeDetailRoute.DeviceMetrics", - ), + listOf("start:NodeDetailRoute.DeviceMetrics:name=NodeDetailRoute.DeviceMetrics"), analytics.events, ) } @@ -87,8 +85,8 @@ class ScreenViewTrackerTest { assertEquals( listOf( - "start:org.meshtastic.core.navigation.NodesRoute.Nodes:name=org.meshtastic.core.navigation.NodesRoute.Nodes", - "stop:org.meshtastic.core.navigation.NodesRoute.Nodes", + "start:NodesRoute.Nodes:name=NodesRoute.Nodes", + "stop:NodesRoute.Nodes", ), analytics.events, )