Skip to content

Commit 1bd9913

Browse files
authored
Update sampling to calculate per-run-of-app instead of per-session (#10769)
1 parent 6faa9c3 commit 1bd9913

File tree

4 files changed

+52
-13
lines changed

4 files changed

+52
-13
lines changed

FirebaseSessions/Sources/FirebaseSessions.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ private enum GoogleDataTransportConfig {
6969
installations: installations
7070
)
7171

72-
let sessionGenerator = SessionGenerator(settings: settings)
72+
let sessionGenerator = SessionGenerator(collectEvents: Sessions
73+
.shouldCollectEvents(settings: settings))
7374
let coordinator = SessionCoordinator(
7475
installations: installations,
7576
fireLogger: fireLogger
@@ -195,6 +196,15 @@ private enum GoogleDataTransportConfig {
195196
}
196197
}
197198

199+
// MARK: - Sampling
200+
201+
static func shouldCollectEvents(settings: SettingsProtocol) -> Bool {
202+
// Calculate whether we should sample events using settings data
203+
// Sampling rate of 1 means we do not sample.
204+
let randomValue = Double.random(in: 0 ... 1)
205+
return randomValue <= settings.samplingRate
206+
}
207+
198208
// MARK: - Data Collection
199209

200210
var isAnyDataCollectionEnabled: Bool {

FirebaseSessions/Sources/SessionGenerator.swift

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,16 @@ struct SessionInfo {
3939
///
4040
class SessionGenerator {
4141
private var thisSession: SessionInfo?
42-
private var settings: SettingsProtocol
4342

4443
private var firstSessionId = ""
4544
private var sessionIndex: Int32
45+
private var collectEvents: Bool
4646

47-
init(settings: SettingsProtocol) {
48-
self.settings = settings
47+
init(collectEvents: Bool) {
4948
// This will be incremented to 0 on the first generation
5049
sessionIndex = -1
50+
51+
self.collectEvents = collectEvents
5152
}
5253

5354
// Generates a new Session ID. If there was already a generated Session ID
@@ -61,12 +62,6 @@ class SessionGenerator {
6162

6263
sessionIndex += 1
6364

64-
var collectEvents = true
65-
let randomValue = Double.random(in: 0 ... 1)
66-
if randomValue > settings.samplingRate {
67-
collectEvents = false
68-
}
69-
7065
let newSession = SessionInfo(sessionId: newSessionId,
7166
firstSessionId: firstSessionId,
7267
dispatchEvents: collectEvents,

FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ class FirebaseSessionsTestsBase: XCTestCase {
9696
// initialization of other classes
9797
preSessionsInit(mockSettings)
9898

99-
generator = SessionGenerator(settings: mockSettings)
99+
generator = SessionGenerator(collectEvents: Sessions
100+
.shouldCollectEvents(settings: mockSettings))
100101
initiator = SessionInitiator(settings: mockSettings, currentTimeProvider: {
101102
self.pausedClock
102103
})

FirebaseSessions/Tests/Unit/SessionGeneratorTests.swift

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ class SessionGeneratorTests: XCTestCase {
5353
localOverrides: localOverrideSettings,
5454
remoteSettings: remoteSettings
5555
)
56-
generator = SessionGenerator(settings: sessionSettings)
56+
generator = SessionGenerator(collectEvents: Sessions
57+
.shouldCollectEvents(settings: sessionSettings))
5758
}
5859

5960
func isValidSessionID(_ sessionID: String) -> Bool {
@@ -138,6 +139,10 @@ class SessionGeneratorTests: XCTestCase {
138139
remoteSettings: remoteSettings
139140
)
140141

142+
// Rebuild the SessionGenerator with the new settings
143+
generator = SessionGenerator(collectEvents: Sessions
144+
.shouldCollectEvents(settings: sessionSettings))
145+
141146
let sessionInfo = generator.generateNewSession()
142147
XCTAssertTrue(sessionInfo.shouldDispatchEvents)
143148
}
@@ -150,7 +155,6 @@ class SessionGeneratorTests: XCTestCase {
150155
"session_timeout_seconds": 50,
151156
],
152157
]
153-
154158
cache.removeCache()
155159
downloader = MockSettingsDownloader(successResponse: someSettings)
156160
remoteSettings = RemoteSettings(appInfo: appInfo, downloader: downloader, cache: cache)
@@ -164,7 +168,36 @@ class SessionGeneratorTests: XCTestCase {
164168
remoteSettings: remoteSettings
165169
)
166170

171+
// Rebuild the SessionGenerator with the new settings
172+
generator = SessionGenerator(collectEvents: Sessions
173+
.shouldCollectEvents(settings: sessionSettings))
174+
175+
let sessionInfo = generator.generateNewSession()
176+
XCTAssertFalse(sessionInfo.shouldDispatchEvents)
177+
}
178+
179+
func test_sessionsSampling_persistsPerRun() throws {
180+
let mockSettings = MockSettingsProtocol()
181+
mockSettings.samplingRate = 0
182+
183+
// Rebuild the SessionGenerator with the new settings
184+
generator = SessionGenerator(collectEvents: Sessions
185+
.shouldCollectEvents(settings: mockSettings))
186+
167187
let sessionInfo = generator.generateNewSession()
168188
XCTAssertFalse(sessionInfo.shouldDispatchEvents)
189+
190+
// Try again
191+
let sessionInfo2 = generator.generateNewSession()
192+
XCTAssertFalse(sessionInfo2.shouldDispatchEvents)
193+
194+
// Start returning true from the calculator
195+
mockSettings.samplingRate = 1
196+
197+
// Try again after the calculator starts returning true.
198+
// It should still be false in the sessionInfo because the
199+
// sampling rate is calculated per-run
200+
let sessionInfo3 = generator.generateNewSession()
201+
XCTAssertFalse(sessionInfo3.shouldDispatchEvents)
169202
}
170203
}

0 commit comments

Comments
 (0)