From 4078dbe11e5368f59f64519c6f4220d183465997 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:58:37 +0200 Subject: [PATCH] recording: session replay respect feature flag variants (#197) --- CHANGELOG.md | 2 + .../posthog/internal/PostHogFeatureFlags.kt | 66 ++++++++++--- .../internal/PostHogFeatureFlagsTest.kt | 92 +++++++++++++++++++ ...decide-recording-bool-linked-disabled.json | 35 +++++++ ...-decide-recording-bool-linked-enabled.json | 35 +++++++ ...e-recording-bool-linked-variant-match.json | 38 ++++++++ ...cording-bool-linked-variant-not-match.json | 38 ++++++++ 7 files changed, 293 insertions(+), 13 deletions(-) create mode 100644 posthog/src/test/resources/json/basic-decide-recording-bool-linked-disabled.json create mode 100644 posthog/src/test/resources/json/basic-decide-recording-bool-linked-enabled.json create mode 100644 posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-match.json create mode 100644 posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-not-match.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 44caae7f..6fd2de65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next +- recording: session replay respect feature flag variants ([#197](https://github.com/PostHog/posthog-android/pull/197)) + ## 3.8.1 - 2024-10-09 - recording: `OnTouchEventListener` try catch guard to swallow unexpected errors take 2 ([#196](https://github.com/PostHog/posthog-android/pull/196)) diff --git a/posthog/src/main/java/com/posthog/internal/PostHogFeatureFlags.kt b/posthog/src/main/java/com/posthog/internal/PostHogFeatureFlags.kt index f1223b92..b7022fdf 100644 --- a/posthog/src/main/java/com/posthog/internal/PostHogFeatureFlags.kt +++ b/posthog/src/main/java/com/posthog/internal/PostHogFeatureFlags.kt @@ -36,6 +36,37 @@ internal class PostHogFeatureFlags( preloadSessionReplayFlag() } + private fun isRecordingActive( + featureFlags: Map, + sessionRecording: Map, + ): Boolean { + var recordingActive = true + + // Check for boolean flags + val linkedFlag = sessionRecording["linkedFlag"] + if (linkedFlag is String) { + val value = featureFlags[linkedFlag] + if (value is Boolean) { + recordingActive = value + } + } else if (linkedFlag is Map<*, *>) { + // Check for specific flag variant + val flag = linkedFlag["flag"] as? String + val variant = linkedFlag["variant"] as? String + if (flag != null && variant != null) { + val value = featureFlags[flag] as? String + recordingActive = value == variant + } + } + // check for multi flag variant (any) + // val linkedFlag = sessionRecording["linkedFlag"] as? String, + // featureFlags[linkedFlag] != nil + // is also a valid check but since we cannot check the value of the flag, + // we consider session recording is active + + return recordingActive + } + fun loadFeatureFlags( distinctId: String, anonymousId: String?, @@ -74,26 +105,30 @@ internal class PostHogFeatureFlags( this.featureFlagPayloads = normalizedPayloads } - when (response.sessionRecording) { + when (val sessionRecording = response.sessionRecording) { is Boolean -> { // if sessionRecording is a Boolean, its always disabled // so we don't enable sessionReplayFlagActive here - sessionReplayFlagActive = false + sessionReplayFlagActive = sessionRecording - config.cachePreferences?.remove(SESSION_REPLAY) + if (!sessionRecording) { + config.cachePreferences?.remove(SESSION_REPLAY) + } else { + // do nothing + } } is Map<*, *> -> { @Suppress("UNCHECKED_CAST") - (response.sessionRecording as? Map)?.let { sessionRecording -> + (sessionRecording as? Map)?.let { // keeps the value from config.sessionReplay since having sessionRecording // means its enabled on the project settings, but its only enabled // when local config.sessionReplay is also enabled - config.snapshotEndpoint = sessionRecording["endpoint"] as? String + config.snapshotEndpoint = it["endpoint"] as? String ?: config.snapshotEndpoint - sessionReplayFlagActive = true - config.cachePreferences?.setValue(SESSION_REPLAY, sessionRecording) + sessionReplayFlagActive = isRecordingActive(this.featureFlags ?: mapOf(), it) + config.cachePreferences?.setValue(SESSION_REPLAY, it) // TODO: // consoleLogRecordingEnabled -> Boolean or null @@ -131,14 +166,19 @@ internal class PostHogFeatureFlags( private fun preloadSessionReplayFlag() { synchronized(featureFlagsLock) { - @Suppress("UNCHECKED_CAST") - val sessionRecording = config.cachePreferences?.getValue(SESSION_REPLAY) as? Map + config.cachePreferences?.let { preferences -> + @Suppress("UNCHECKED_CAST") + val sessionRecording = preferences.getValue(SESSION_REPLAY) as? Map + + @Suppress("UNCHECKED_CAST") + val flags = preferences.getValue(FEATURE_FLAGS) as? Map - if (sessionRecording != null) { - sessionReplayFlagActive = true + if (sessionRecording != null) { + sessionReplayFlagActive = isRecordingActive(flags ?: mapOf(), sessionRecording) - config.snapshotEndpoint = sessionRecording["endpoint"] as? String - ?: config.snapshotEndpoint + config.snapshotEndpoint = sessionRecording["endpoint"] as? String + ?: config.snapshotEndpoint + } } } } diff --git a/posthog/src/test/java/com/posthog/internal/PostHogFeatureFlagsTest.kt b/posthog/src/test/java/com/posthog/internal/PostHogFeatureFlagsTest.kt index 5d96bc8d..12e5341f 100644 --- a/posthog/src/test/java/com/posthog/internal/PostHogFeatureFlagsTest.kt +++ b/posthog/src/test/java/com/posthog/internal/PostHogFeatureFlagsTest.kt @@ -342,4 +342,96 @@ internal class PostHogFeatureFlagsTest { assertTrue(sut.isSessionReplayFlagActive()) assertEquals("/b/", config?.snapshotEndpoint) } + + @Test + fun `returns isSessionReplayFlagActive true if bool linked flag is enabled`() { + val file = File("src/test/resources/json/basic-decide-recording-bool-linked-enabled.json") + + val http = + mockHttp( + response = + MockResponse() + .setBody(file.readText()), + ) + val url = http.url("/") + + val sut = getSut(host = url.toString()) + + sut.loadFeatureFlags("my_identify", anonymousId = "anonId", emptyMap(), null) + + executor.shutdownAndAwaitTermination() + + assertTrue(sut.isSessionReplayFlagActive()) + + sut.clear() + } + + @Test + fun `returns isSessionReplayFlagActive false if bool linked flag is disabled`() { + val file = File("src/test/resources/json/basic-decide-recording-bool-linked-disabled.json") + + val http = + mockHttp( + response = + MockResponse() + .setBody(file.readText()), + ) + val url = http.url("/") + + val sut = getSut(host = url.toString()) + + sut.loadFeatureFlags("my_identify", anonymousId = "anonId", emptyMap(), null) + + executor.shutdownAndAwaitTermination() + + assertFalse(sut.isSessionReplayFlagActive()) + + sut.clear() + } + + @Test + fun `returns isSessionReplayFlagActive true if multi variant linked flag is a match`() { + val file = File("src/test/resources/json/basic-decide-recording-bool-linked-variant-match.json") + + val http = + mockHttp( + response = + MockResponse() + .setBody(file.readText()), + ) + val url = http.url("/") + + val sut = getSut(host = url.toString()) + + sut.loadFeatureFlags("my_identify", anonymousId = "anonId", emptyMap(), null) + + executor.shutdownAndAwaitTermination() + + assertTrue(sut.isSessionReplayFlagActive()) + + sut.clear() + } + + @Test + fun `returns isSessionReplayFlagActive false if multi variant linked flag is not a match`() { + val file = File("src/test/resources/json/basic-decide-recording-bool-linked-variant-not-match.json") + + val http = + mockHttp( + response = + MockResponse() + .setBody(file.readText()), + ) + val url = http.url("/") + + val sut = getSut(host = url.toString()) + + sut.loadFeatureFlags("my_identify", anonymousId = "anonId", emptyMap(), null) + + executor.shutdownAndAwaitTermination() + + assertFalse(sut.isSessionReplayFlagActive()) + + sut.clear() + } } diff --git a/posthog/src/test/resources/json/basic-decide-recording-bool-linked-disabled.json b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-disabled.json new file mode 100644 index 00000000..66514cb3 --- /dev/null +++ b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-disabled.json @@ -0,0 +1,35 @@ +{ + "autocaptureExceptions": false, + "toolbarParams": {}, + "errorsWhileComputingFlags": false, + "capturePerformance": true, + "autocapture_opt_out": false, + "isAuthenticated": false, + "supportedCompression": [ + "gzip", + "gzip-js" + ], + "config": { + "enable_collect_everything": true + }, + "featureFlagPayloads": { + "thePayload": true + }, + "featureFlags": { + "4535-funnel-bar-viz": true, + "session-replay-flag": false + }, + "sessionRecording": { + "endpoint": "/b/", + "linkedFlag": "session-replay-flag" + }, + "siteApps": [ + { + "id": 21039.0, + "url": "/site_app/21039/EOsOSePYNyTzHkZ3f4mjrjUap8Hy8o2vUTAc6v1ZMFP/576ac89bc8aed72a21d9b19221c2c626/" + } + ], + "editorParams": { + + } +} diff --git a/posthog/src/test/resources/json/basic-decide-recording-bool-linked-enabled.json b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-enabled.json new file mode 100644 index 00000000..5bd8e4e4 --- /dev/null +++ b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-enabled.json @@ -0,0 +1,35 @@ +{ + "autocaptureExceptions": false, + "toolbarParams": {}, + "errorsWhileComputingFlags": false, + "capturePerformance": true, + "autocapture_opt_out": false, + "isAuthenticated": false, + "supportedCompression": [ + "gzip", + "gzip-js" + ], + "config": { + "enable_collect_everything": true + }, + "featureFlagPayloads": { + "thePayload": true + }, + "featureFlags": { + "4535-funnel-bar-viz": true, + "session-replay-flag": true + }, + "sessionRecording": { + "endpoint": "/b/", + "linkedFlag": "session-replay-flag" + }, + "siteApps": [ + { + "id": 21039.0, + "url": "/site_app/21039/EOsOSePYNyTzHkZ3f4mjrjUap8Hy8o2vUTAc6v1ZMFP/576ac89bc8aed72a21d9b19221c2c626/" + } + ], + "editorParams": { + + } +} diff --git a/posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-match.json b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-match.json new file mode 100644 index 00000000..79bc074a --- /dev/null +++ b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-match.json @@ -0,0 +1,38 @@ +{ + "autocaptureExceptions": false, + "toolbarParams": {}, + "errorsWhileComputingFlags": false, + "capturePerformance": true, + "autocapture_opt_out": false, + "isAuthenticated": false, + "supportedCompression": [ + "gzip", + "gzip-js" + ], + "config": { + "enable_collect_everything": true + }, + "featureFlagPayloads": { + "thePayload": true + }, + "featureFlags": { + "4535-funnel-bar-viz": true, + "session-replay-flag": "variant-1" + }, + "sessionRecording": { + "endpoint": "/b/", + "linkedFlag": { + "flag": "session-replay-flag", + "variant": "variant-1" + } + }, + "siteApps": [ + { + "id": 21039.0, + "url": "/site_app/21039/EOsOSePYNyTzHkZ3f4mjrjUap8Hy8o2vUTAc6v1ZMFP/576ac89bc8aed72a21d9b19221c2c626/" + } + ], + "editorParams": { + + } +} diff --git a/posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-not-match.json b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-not-match.json new file mode 100644 index 00000000..142de337 --- /dev/null +++ b/posthog/src/test/resources/json/basic-decide-recording-bool-linked-variant-not-match.json @@ -0,0 +1,38 @@ +{ + "autocaptureExceptions": false, + "toolbarParams": {}, + "errorsWhileComputingFlags": false, + "capturePerformance": true, + "autocapture_opt_out": false, + "isAuthenticated": false, + "supportedCompression": [ + "gzip", + "gzip-js" + ], + "config": { + "enable_collect_everything": true + }, + "featureFlagPayloads": { + "thePayload": true + }, + "featureFlags": { + "4535-funnel-bar-viz": true, + "session-replay-flag": "variant-2" + }, + "sessionRecording": { + "endpoint": "/b/", + "linkedFlag": { + "flag": "session-replay-flag", + "variant": "variant-1" + } + }, + "siteApps": [ + { + "id": 21039.0, + "url": "/site_app/21039/EOsOSePYNyTzHkZ3f4mjrjUap8Hy8o2vUTAc6v1ZMFP/576ac89bc8aed72a21d9b19221c2c626/" + } + ], + "editorParams": { + + } +}