Repository navigation
feat(events): persist events to disk so they survive the process (tier 3) - #538
abelonogov-ld wants to merge 84 commits into
Conversation
Tiers 1 and 2 both need the process to survive. When it does not, everything recorded since the last delivery goes with it -- including the crash report an application was recording when it died. Tier 3 is the answer to that: events are written to a log under Application Support, and a later launch delivers whatever the previous run left behind. The write is a plain append into the page cache, never an fsync, so a commit point costs tens of microseconds rather than milliseconds. track and identify are commit points: at .immediate the write happens on the calling thread, so a process that dies the instant after track returns still reports that event. Evaluations are not -- they are held as objects and encoded in runs, so the hottest path in the SDK pays a counter bump and nothing else. This is off by default. Writing events changes what a track costs and leaves a directory of the application's data on the device, and a minor version cannot decide either on an application's behalf. LDConfig.eventPersistence has three levels: disabled, which is what Tiers 1 and 2 describe and is the default; deferred, which writes off the calling thread; and immediate, which is the whole of the above. Delivery is held for the round trip rather than just its start, because a batch now stays on disk until its response is handled and a second delivery beginning inside that window would send it again under the same payload ID. Spec: Event Durability §13, §14; Page-Cache Event Persistence. Co-authored-by: Cursor <cursoragent@cursor.com>
…y/event-durability-tier3-persistence # Conflicts: # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift
Tier 3 stages encoded bytes into the store instead of holding events in memory, so the assertion that nothing was buffered has to ask the store.
…y/event-durability-tier3-persistence # Conflicts: # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift
…y/event-durability-tier3-persistence Also renames handWrittenEncoder to eventJSONWriter, after its type.
…y/event-durability-tier3-persistence
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly.xcodeproj/project.pbxproj
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly/LaunchDarkly/ServiceObjects/EventJSONWriter.swift # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift # LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com>
…y/event-durability-tier3-persistence
…y/event-durability-tier3-persistence
…atomically; reserve capacity during encode - Without persistence, nothing but a delivery drains the held run: no threshold commit, and track, identify, flush and backgrounding no longer encode on the caller's thread. - An evaluation's summary counter and its full/debug events are recorded under pendingLock, and a commit takes both under it. - A commit reserves capacity for the run it takes, so events being encoded still count against eventCapacity. Co-authored-by: Cursor <cursoragent@cursor.com>
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly/LaunchDarkly/LDClient.swift # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift # LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly/LaunchDarkly/LDClient.swift # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift # LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly/LaunchDarkly/LDClient.swift # LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift # LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift
… failed write left in memory A read can fail on a file that is intact, as it does in a process out of file descriptors, and every failure was treated as a batch that can never be delivered and deleted. Listing now skips such a batch until a later listing can read it, delivery retries it, and startup recovery closes the previous run's log off uncounted rather than deleting it. Only a file that is gone, or one written in a format this version does not read, is nothing to keep. closeBatch also closed only the log where a failed write had left events both there and in memory, so a flush could report success and close() discard the rest. Co-authored-by: Cursor <cursoragent@cursor.com>
…d not be kept A batch LaunchDarkly refuses in a way that may pass is kept and retried, so it is not a loss. What is lost is what the store never received: an event or a summary that could not be serialized when it was committed, or that serialized too large to store. A delivery that finds nothing to send cannot tell those apart from events that arrived, so flushAndWait reported success for them. Co-authored-by: Cursor <cursoragent@cursor.com>
Android pins the failed write at the processor level, where the bug was visible: the flush reports success and both the written and the held events arrive. It also pins that a body reads correctly once reads work again, and that a previous run's log moved aside leaves this run the name to append to. Co-authored-by: Cursor <cursoragent@cursor.com>
…y/event-durability-tier3-persistence Tier 2 replaced the delivery gate this branch was built on, so the two designs had to be reconciled rather than taken side by side. Coalescing comes from tier 2: flushes arriving before a delivery starts join it, and one that has already started is never joined, so nothing holds the drain back while a retry runs. What tier 2 did not need is a way to keep an in-flight batch from being sent twice. Tier 2 empties its buffer before publishing, so a second delivery finds nothing; here a batch stays on disk until its response arrives, which is what `isDelivering` used to guard. `batchesInFlight` takes its place. A delivery claims the batches it lists and releases them when it has an answer, so a later delivery still runs and still sends what it closed for itself, and only the batches already on their way are left out of it. A caller whose own events are all that is left unsent is answered `true`, which is the contract tier 2 documents: an answer is about the batch the caller's own delivery took. The loss flag is now taken once per delivery rather than once per caller, so everyone joined to it hears about a loss instead of only whichever was answered first, and a scheduled delivery with no waiters leaves it for the caller who is owed it. `flush` stays a commit point on both paths. This branch had also fixed the duplicate event reporter on its own, as `eventSyncLogger`; that now comes from the precursor branch instead, so the local copy goes. Co-authored-by: Cursor <cursoragent@cursor.com>
…y/event-durability-tier3-persistence
Functions that must run under a lock now have plain names, keep "Requires `xLock`" in their docs, and check it with UnfairLock.assertOwned(), which traps in debug builds when the calling thread does not hold the lock. Co-authored-by: Cursor <cursoragent@cursor.com>
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # LaunchDarkly/LaunchDarkly/UnfairLock.swift
…-durability-tier2-bounded-flush
…y/event-durability-tier3-persistence
| if let value = context.getTopLevelAddressableAttributeSingleKind(key) { | ||
| if redactAll { | ||
| redactedAttributes.append(reference.raw()) | ||
| redactedAttributes.append(key) |
There was a problem hiding this comment.
All-private redaction names do not reparse
Medium Severity
When every attribute is private, both context encoders append the raw attribute name to redactedAttributes. A name such as /foo is a path reference to foo, not a reference to an attribute named /foo, so the redaction list identifies a different attribute than the one that was removed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8bf63c6. Configure here.
Closing a batch from memory cleared the buffer, assembled the body outside the lock, and only then counted the events as closed. In between they were counted nowhere, so a capacity check in that window admitted that many events more than capacity allows. Counted through writingEvents, which the commit path already uses for the same window around a write, and which Android has always used for both. Co-authored-by: Cursor <cursoragent@cursor.com>
A failed delete was swallowed: the counts came off, the file stayed, and the next listing found it and sent it again, and the one after that. The counts still come off, because LaunchDarkly has the events and holding capacity for a file this process may never be allowed to delete would cost the session events it could still have recorded. The batch goes into a per-session set that every listing skips, and each listing retries the delete, so a directory that stops being read-only clears itself. Co-authored-by: Cursor <cursoragent@cursor.com>
Also documents that the same number bounds how many distinct contexts flag evaluations are summarized for between two reports. Co-authored-by: Cursor <cursoragent@cursor.com>
A flush is what an application calls before ending the process on purpose, so it now writes on the calling thread wherever events persist rather than only queueing the write. The flush and .deferred docs now say what a crash loses and when evaluations are written. Co-authored-by: Cursor <cursoragent@cursor.com>
The store scheduled its own write once 16 KB were staged, on a queue of its own. The processor is the only caller of stage, and every path that stages ends in a commit or a close, so that write only ever split or repeated one already under way. The store now writes only when told to, and pendingCommitThreshold is the one background trigger. Co-authored-by: Cursor <cursoragent@cursor.com>
Batches are now named ready-<payloadId>-<count>, so listing one this process did not close - every batch at startup, and those of another process sharing the environment - costs a directory entry rather than a read of the whole file under ioLock. Batches named without a count by an earlier version are still read, once, and delivered under the name they have. Co-authored-by: Cursor <cursoragent@cursor.com>
didEnterBackground only queued the write at .deferred, so on macOS and watchOS, where nothing is flushed afterwards, a process ended in the background could lose what was recorded. It now commits before returning wherever events persist, by the same rule a flush uses, exposed as EventProcessor.commitWherePersisting(). commitAtCommitPoint() had no other caller outside the processor and is now private to it. Co-authored-by: Cursor <cursoragent@cursor.com>
DirectEventProcessorSpec read eventBuffer.contextSummarizer without recordLock, which a scheduled commit holds while it takes the counters. The reads now go through withContextSummarizer, a DEBUG accessor that takes the lock. Co-authored-by: Cursor <cursoragent@cursor.com>
A batch named without a count by an earlier version ends in a UUID group, which Int32 accepts once its leading zeros are dropped. A count is never written with a leading zero, so a suffix that has one is part of the id. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
An identify's completion runs before its after-identify hooks, so a test that only waits on the completion could check the stages before the hook was told. RecordingHook is also written from the identify's thread, so it now guards its stages with a lock. Co-authored-by: Cursor <cursoragent@cursor.com>
The check for what a previous run left can run after the spec closes its batch. It then delivers on going online, and where that delivery starts before the spec's flush, the flush finds nothing left and reports true. Co-authored-by: Cursor <cursoragent@cursor.com>
…eduled It required more than one summary, which only held where a commit landed while the evaluations were still being recorded. A starved commit queue and main thread left one. Recording now pauses halfway until the first half is committed, so there are always at least two. Co-authored-by: Cursor <cursoragent@cursor.com>
## Problem `ConnectionInformation.onlineSetCheck` called `NetworkReporter.isConnectedToNetwork()`, a static `SCNetworkReachability` probe of the default route with no seam for a test to reach. `LDClientSpec`'s mock graph could say the client was online, but not that there was anywhere for it to reach. So this test depended on the machine running it being genuinely online: ``` ConnectionInformation / when client was started in foreground ``` Offline it fails, expecting `.establishingStreamingConnection` and getting `.offline`. The other two cases in that block expect `.offline`, so offline they pass — for the wrong reason. ## Change `NetworkReporter` becomes a `NetworkReporting` protocol reached through `ClientServiceCreating.makeNetworkReporter()`, like every other service, and `LDClient` holds one as `networkReporter`. `onlineSetCheck` reads it off the client, which is how `effectiveStreamingMode` already gets `isInSupportedRunMode`, so neither call site changes. The mock is Sourcery-generated from the usual `// sourcery: autoMockable` annotation rather than hand-written, so it cannot drift. `ClientServiceMockFactory` defaults it to connected, and `TestContext` exposes `networkReporterMock` for tests that want otherwise. No behavior change for the SDK: the production factory returns the same reachability check as before. ## Verification - Stubbed the real `SCNetworkReachability` implementation to return `false` and confirmed it compiled in (via the unreachable-code warning). All 220 `LDClientSpec` tests still pass, so the suite no longer consults the machine. Before this change, the same stub fails the foreground test. - Full macOS suite: 707 passing. - SwiftLint: 71, identical to the `v11` baseline. - New test `when the device has no route to the network` covers the branch that had no deterministic coverage: the client reports `isOnline == true` with a connection mode of `.offline`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Replaces the static `NetworkReporter.isConnectedToNetwork()` probe with an injectable **`NetworkReporting`** service created by **`ClientServiceCreating.makeNetworkReporter()`** and held on **`LDClient`**. **`ConnectionInformation.onlineSetCheck`** now asks **`ldClient.networkReporter`** whether there is a route before choosing streaming/polling vs **`.offline`**, so tests can stub connectivity instead of inheriting the host machine’s network state. > > Test support adds a Sourcery **`NetworkReportingMock`**, defaults the mock factory to “connected,” exposes **`networkReporterMock`** on **`TestContext`**, and adds a case where **`startOnline`** yields **`isOnline == true`** but **`currentConnectionMode == .offline`** when there is no network route. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a4818e1. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 690f52e. Configure here.
| os_log("%s aborted. DirectEventProcessor is offline", log: service.config.logger, type: .debug, typeName(and: #function)) | ||
| reportSyncComplete(.isOffline) | ||
| completion?(false, 0) | ||
| return |
There was a problem hiding this comment.
Close drops unsent in-memory events
High Severity
close() flushes then immediately sets the processor offline. With default .disabled persistence, commitWherePersisting does not encode pending events, and the queued delivery often hits the isOnline guard before commitAndCloseBatch. Those events are never staged or sent, and they disappear when the client is released. Restarting the SDK in-process loses them.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 690f52e. Configure here.
| public func flush() { | ||
| LDClient.instancesQueue.sync(flags: .barrier) { | ||
| LDClient.instances?.forEach { $1.internalFlush() } | ||
| } |
There was a problem hiding this comment.
Flush blocks client map during disk I/O
Medium Severity
flush() and close() keep instancesQueue while commitWherePersisting encodes and writes every environment. LDClient.get(), start, and other instance-map calls wait on that I/O. flush(completion:) already snapshots the clients first, so the no-completion path is doing much heavier work under the global lock than it used to.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 690f52e. Configure here.


Third in the event-durability stack, on top of #536. Recorded events go to an on-disk log rather than an in-memory buffer, so an application that dies moments after recording one still reports it: the next run delivers what the last one left behind.
What changes
EventStoreis a new on-disk log, with capacity, framing, interrupted-log recovery, and multi-process safety.NullEventStorestands in where the platform gave the SDK nowhere to write, which is the behaviour the SDK had before.Recording no longer encodes on the caller's thread. An evaluation appends to a held run and updates a counter; a commit turns the whole run into bytes at once. Recording a custom or identify event is a commit point, because an application reporting something it chose to report is saying it matters more than the microseconds it costs. Evaluations commit once
pendingCommitThresholdof them accumulate, when a delivery starts, or on flush.LDConfig.eventPersistencelets the application choose whether a commit point runs on the caller's thread (.immediate), off it (.deferred), or not at all (.disabled).Delivery sends batches oldest first and stops at the first retryable failure, so the rest keep their place in the log instead of the SDK spending the session's requests on a service that is refusing them.
Reconciling with tier 2
Tier 2 replaced the delivery gate this branch was built on, so the merge had to pick one design rather than keep both.
Coalescing comes from tier 2: flushes arriving before a delivery starts join it, one already started is never joined, and nothing holds the drain back while a retry runs. What tier 2 did not need is a way to keep an in-flight batch from being sent twice — it empties its buffer before publishing, so a second delivery finds nothing. Here a batch stays on disk until its response arrives, which is what
isDeliveringguarded.batchesInFlighttakes its place. A delivery claims the batches it lists and releases them once it has an answer, so a later delivery still runs and still sends what it closed for itself; only the batches already on their way are left out. A caller whose own events are all that remain unsent is answeredtrue, which is exactly the contract tier 2 documents: an answer is about the batch the caller's own delivery took, never one an earlier delivery lost.The loss flag is now taken once per delivery rather than once per caller, so everyone joined to it hears about a loss instead of only whoever was answered first, and a scheduled delivery with no waiters leaves it for the caller who is owed it. This branch had also fixed the duplicate event reporter independently; that now comes from #537, so the local copy is gone.
Verification
does not send a batch again when a delivery is asked for while one is in flight.DiagnosticReporterSpec.testRetriesflaked once under load and passed on re-run; it has a 0.1 s timeout and is unrelated to these changes.Note
Overview
Replaces in-memory
EventReporterwith anEventProcessorpipeline (DirectEventProcessor,NullEventProcessor,AnalyticsEventSender) backed by an on-diskEventStoreso recorded analytics can survive process death and be delivered on the next launch.Recording and persistence: Evaluations are batched and encoded on commit (threshold, delivery, or flush);
track/identifyare commit points. NewLDConfig.eventPersistence(.disabled/.deferred/.immediate) controls whether commits run on the caller thread. DefaulteventCapacityrises from 100 to 2000 and also caps distinct contexts inContextSummarizer.Delivery and flush: Batches are claimed while in flight, sent oldest-first, and
flush(completion:)/flushAndWaitreport per-environment outcomes. On iOS/tvOS, backgroundingcommitWherePersistingand attempts delivery underBackgroundActivity.Encoding: Custom
EventJSONWriter/LDContextJSONWriterwithContextEncodingCachereplace copying contexts for anonymous redaction; feature vs debug/summary redaction is driven by event kind, not mutatingLDContext.Reviewed by Cursor Bugbot for commit 690f52e. Bugbot is set up for automated code reviews on this repo. Configure here.