-
Notifications
You must be signed in to change notification settings - Fork 49
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
fix: session replay respect feature flag variants #209
Changes from all commits
7ef43d6
2683cfb
a0d5623
0f3a2c5
dd126d2
33fd284
cc02e12
48f42f9
5a10f5b
9ed3c6a
c0996d7
5377865
c9adabe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,19 +36,46 @@ class PostHogFeatureFlags { | |
|
||
private func preloadSesssionReplayFlag() { | ||
var sessionReplay: [String: Any]? | ||
var featureFlags: [String: Any]? | ||
featureFlagsLock.withLock { | ||
sessionReplay = self.storage.getDictionary(forKey: .sessionReplay) as? [String: Any] | ||
featureFlags = self.getCachedFeatureFlags() | ||
} | ||
|
||
if let sessionReplay = sessionReplay { | ||
sessionReplayFlagActive = true | ||
sessionReplayFlagActive = isRecordingActive(featureFlags ?? [:], sessionReplay) | ||
|
||
if let endpoint = sessionReplay["endpoint"] as? String { | ||
config.snapshotEndpoint = endpoint | ||
} | ||
} | ||
} | ||
|
||
private func isRecordingActive(_ featureFlags: [String: Any], _ sessionRecording: [String: Any]) -> Bool { | ||
var recordingActive = true | ||
|
||
// check for boolean flags | ||
if let linkedFlag = sessionRecording["linkedFlag"] as? String, | ||
let value = featureFlags[linkedFlag] as? Bool | ||
{ | ||
recordingActive = value | ||
// check for specific flag variant | ||
} else if let linkedFlag = sessionRecording["linkedFlag"] as? [String: Any], | ||
let flag = linkedFlag["flag"] as? String, | ||
let variant = linkedFlag["variant"] as? String, | ||
let value = featureFlags[flag] as? String | ||
{ | ||
recordingActive = value == variant | ||
} | ||
// check for multi flag variant (any) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we clean up these comments here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so since it's a valid result from the API and might help while debugging things |
||
// if let linkedFlag = sessionRecording["linkedFlag"] as? String, | ||
// featureFlags[linkedFlag] != nil | ||
// is also a valid check bbut since we cannot check the value of the flag, | ||
// we consider session recording is active | ||
|
||
return recordingActive | ||
} | ||
|
||
func loadFeatureFlags( | ||
distinctId: String, | ||
anonymousId: String, | ||
|
@@ -94,7 +121,7 @@ class PostHogFeatureFlags { | |
if let endpoint = sessionRecording["endpoint"] as? String { | ||
self.config.snapshotEndpoint = endpoint | ||
} | ||
self.sessionReplayFlagActive = true | ||
self.sessionReplayFlagActive = self.isRecordingActive(featureFlags, sessionRecording) | ||
self.storage.setDictionary(forKey: .sessionReplay, contents: sessionRecording) | ||
} | ||
#endif | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,15 +120,20 @@ class PostHogStorage { | |
} | ||
|
||
/** | ||
There are cases where applications using posthog-ios want to share analytics data between host app and an app extension, Widget or App Clip. If there's a defined `appGroupIdentifier` in configuration, we want to use a shared container for storing data so that extensions correcly identify a user (and batch process events) | ||
There are cases where applications using posthog-ios want to share analytics data between host app and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just fixed a lint issue (line_length) |
||
an app extension, Widget or App Clip. If there's a defined `appGroupIdentifier` in configuration, | ||
we want to use a shared container for storing data so that extensions correcly identify a user (and batch process events) | ||
*/ | ||
private static func getAppFolderUrl(from configuration: PostHogConfig) -> URL { | ||
/** | ||
|
||
From Apple Docs: | ||
In iOS, the value is nil when the group identifier is invalid. In macOS, a URL of the expected form is always returned, even if the app group is invalid, so be sure to test that you can access the underlying directory before attempting to use it. | ||
In iOS, the value is nil when the group identifier is invalid. In macOS, a URL of the expected form is always | ||
returned, even if the app group is invalid, so be sure to test that you can access the underlying directory | ||
before attempting to use it. | ||
|
||
MacOS: The system also creates the Library/Application Support, Library/Caches, and Library/Preferences subdirectories inside the group directory the first time you use it | ||
MacOS: The system also creates the Library/Application Support, Library/Caches, and Library/Preferences | ||
subdirectories inside the group directory the first time you use it | ||
iOS: The system creates only the Library/Caches subdirectory automatically | ||
|
||
see: https://developer.apple.com/documentation/foundation/filemanager/1412643-containerurl/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,10 +141,8 @@ | |
private func finishAll() { | ||
var completedTasks: [URLSessionTask: NetworkSample] = [:] | ||
tasksLock.withLock { | ||
for item in samplesByTask { | ||
if item.key.state == .completed { | ||
completedTasks[item.key] = item.value | ||
} | ||
for item in samplesByTask where item.key.state == .completed { | ||
completedTasks[item.key] = item.value | ||
Comment on lines
+144
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just fixing a lint issue |
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default is active since the
sessionRecording
is a Map and not a boolean unless I find thelinkedFlag
and can compare things