Repository navigation
feat: Add experimental support for flag overrides - #496
Merged
Merged
Conversation
<!-- TERMAG_PR_BODY_BEGIN --> ## Summary This is the first of a series that adds flag overrides, as defined by the OVERRIDE specification, to the Ruby server SDK. The existing FDv1 and FDv2 file data sources keep their current behavior: this PR does not change `lib/ldclient-rb/impl/integrations/file_data_source.rb`, `file_data_source_v2.rb`, or their specs, and the override feature is additive throughout the series. The file-based override source must tolerate a missing file, a file that is being rewritten in place, and a burst of change notifications, and it must keep the last good data when a reload fails. This change adds `LaunchDarkly::Impl::FileData`, the file reading, parsing, merging, and reloading code that source needs: - Document parsing. The format comes from the file name: a name that ends in `.yaml` or `.yml`, in any case (a file named `.yaml` included), is YAML, and any other file, including one with no extension, is JSON. The parser checks that the document and its `flags`, `flagValues`, and `segments` members are objects; a JSON `null`, array or scalar is an error. - Whether an empty file is valid depends on the format. An empty or white-space JSON file fails to parse (`the file has no content, which is not a JSON document`). Empty or white-space YAML (a tab included), or YAML that is only a comment, `---` or a null, is a document with no entries. - Files are read as bytes and taken as UTF-8, without a UTF-8 byte order mark, whatever the process's default encoding. Content that is not valid UTF-8 fails to parse in either format; a file with a UTF-16 or UTF-32 byte order mark is reported as such (`encoded in UTF-16 or UTF-32; data files must be UTF-8`). - JSON is parsed with `JSON.parse`, so JSON numbers are numbers (`1e3` is `1000.0`), and content after the document, a `#` comment or a trailing comma is an error. Comments and duplicate keys inside one object follow the application's `json` gem: 3.x rejects both. - An ordered merge of several documents with duplicate keys handling of `fail` or `ignore`, expansion of each `flagValues` entry into a flag that is on and serves the value by fallthrough (so an evaluation reports the FALLTHROUGH reason kind, as the specification describes), deserialization into the data model classes, and per-document entry counts. The key under which an entry appears is its key for every purpose: the item is stored under it, duplicates are detected by it, and the definition's own `key` is set to it, so evaluations report the flag by that key. - A reloader that serializes reloads, debounces change signals, keeps the last good result when a reload fails, retries after a bounded delay, reports each distinct failure once, and can skip a reload whose paths, presence and content are identical to the last applied ones. A configured file that does not exist can be made to contribute no data. An exception raised by the consumer's apply is treated as a failed reload: nothing from that load is remembered, the failure is reported, and the reload is retried. An error that a load does not expect is logged and retried, and the worker thread survives it. - In-place rewrites. Applications and editors truncate a file and then write the new content, and a load in between finds the file empty. A load that finds a file with no content (empty, white space, or YAML read as nothing) while the data in effect has content from that file holds the file back: it keeps the current data, logs at debug level, and arms the retry, without reporting a failure. - A file is held back until it has stayed so, unwritten, for one retry delay (1 second), timed from the clock read after that file's own read. A change of its state (modification time and size, read through any symbolic link with the poller's helper) is a new rewrite and starts the delay over, as does a read that finds it with content, even content that fails to parse, or missing. - Each file is held for at most five retry delays from its first hold; its bound starts over only at a load that holds nothing back, whether that load applies, is skipped, or fails. - A load reads and classifies every file, even after one fails to read or parse, so files emptied together are held and applied together; it then fails with the first failure, holds files back, or merges, in that order. - The retry of a hold is armed for the rest of the first due file's delay or bound, plus a tenth of the retry delay. - A file that contributed no content to the data in effect (a placeholder that has always been empty, a file that was missing, a file whose emptying has been applied) is parsed at once, and so is every file when retries are off. - After the delay, an empty JSON file fails to parse and keeps the data; an empty YAML file has no entries. - A stat-based poller that compares existence, modification time, and size, through symbolic links. A file that appears or disappears is a change. - A watcher that uses `rb-inotify` on Linux (a dependency of the optional `listen` gem) to watch the directory of each file without descending into subdirectories, and `listen` elsewhere. It picks up a file that does not exist yet when it appears, and it watches each directory on its own: a directory that does not exist is retried once per second while the others stay watched, and one change is signaled when a retry sets up a watch. Non-recursive watching matters because `listen` scans the whole tree under a watched directory and fails when a subdirectory cannot be read, as happens for a file in the system temporary directory. On Linux, a watched directory that is deleted is watched again once it exists, and one change is signaled so the reload picks up what was written meanwhile. A configured file that is a symbolic link is followed: the directory of the link's target is watched for the target's name, and the links are resolved again after every reported change, after a retry sets up watches, and when a directory that holds a target is deleted, as a Kubernetes ConfigMap update does. A link that cannot be resolved, for example because its target's directory does not exist yet or was deleted, is resolved again once per second until its target is watched, and one change is then signaled. A relative link is expanded against the real path of the directory that holds it, so a `..` in it is right when a directory on the way is itself a link. A new spec, `spec/integrations/file_data_source_compatibility_spec.rb`, pins the behaviors of the existing file data sources that their other specs did not assert (per-file version numbering, keying by the entry's own key, the on-plus-fallthrough expansion of `flagValues`, no retry of a failed load, log messages, modification-time-only polling, no reaction to a deleted file when polling, the FDv1 poller's reload cadence after a change, and use of `listen` when it is present), so that they stay as they are while this code exists beside them. Not changed: YAML that is cut off mid-write can still parse, with fewer entries or a value cut short, so the override source docs recommend replacing a YAML file in one operation. A writer that pauses longer than the retry delay gets the empty file applied (YAML) or reported (JSON). ## Testing - Unit specs for the document parser (format by name, empty JSON and YAML, UTF-8 and byte order marks, JSON numbers and trailing content), merge, poller (including symbolic links), reloader (debounce, retry, last-good retention, sequential reloads, stop, and the hold of an emptied file), and watcher (including a directory with an unreadable subdirectory, the inotify thread lifecycle, symbolic links to files in the same and in another directory, a link that is replaced, a ConfigMap-style update, a relative link in a directory reached through another link, and a target directory that appears late or is recreated), the new compatibility pins, and the unchanged existing file data source specs. - The reloader specs rewrite their files in place with real, deliberately slow writes (`InPlaceWrites.rewrite` in `spec/in_place_writes.rb`), and the hold's delays, bounds and retries are tested on a controlled clock with synchronous loads, with a few real-clock tests for the retry firing. - The in-place rewrite handling was reviewed in eight rounds as #479, with multi-agent and Bugbot reviews, and is shared with the other SDKs' ports. On that code: a mutation check reverted each part of the fix on its own (67 mutants); 60 fail at least one test, and the other 7 are equivalent (the retries-off guard, the clearing of held-file records on apply or skip, starting over every file instead of the files read, and the path or the presence alone in the digest). The changed spec files ran 10 times in a row on Ruby 3.2 and on JRuby 9.4 with no failures. - Full suite and RuboCop are clean, apart from `can be automatically loaded by Bundler.require`, which needs write access to the system gem directory on the test machine. Each new spec was checked against a deliberate defect in the code it covers, and each compatibility pin against a deliberate change to the existing source it pins. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=Overrides --> <sub>Opened from termag</sub>
<!-- TERMAG_PR_BODY_BEGIN --> ## Summary This PR is stacked on #449 so that the flag overrides series applies in order. It does not use that PR's code. Flag overrides, as defined by the OVERRIDE specification, need three pieces below the store: a marker on the flag and segment model that says a definition came from the override store, evaluator marking that follows every definition an evaluation reads, and an indicator on the evaluation reason that reports the marking to callers. This change adds those three pieces. Nothing sets the marker yet, so the SDK behaves exactly as before. The override store, the overlay, and the data system wiring follow in the next PR. The model classes `FeatureFlag` and `Segment` carry the marker as an attribute that is never serialized, because `as_json` returns the flag data hash. `as_override` returns a marked shallow copy and leaves the original unchanged. Only the SDK components that manage override entries will set it. The evaluator marks an evaluation as override-affected when the evaluated flag, a prerequisite flag at any depth, or a segment read during clause matching carries the marker. A segment counts when it is read, so a segment that does not match, or one read through a negated clause, still marks the evaluation. The marking propagates upward only. A prerequisite's own record reflects the definitions its own subtree read, so a plain prerequisite inside a marked evaluation is recorded as usual, and a prerequisite of an overridden flag is not marked by its parent. An error result is marked when a marked definition was read before the failure, including a failure raised while a prerequisite was being evaluated. An evaluation that reads no marked definition keeps the shared precomputed detail instances. `EvaluationReason#override_affected` carries the indicator, following the `inExperiment` and `bigSegmentsStatus` precedents. It is written to JSON as `overrideAffected` only when true, so the wire format of ordinary evaluations is unchanged. `with_override_affected` returns the same instance when nothing changes, and `with_big_segments_status` keeps the indicator. Verification: specs for the model marker (default, copy, original unchanged, not serialized), the reason indicator (default, JSON when true and when false, `[]`, equality, interaction with the big segments status), and the evaluator marking rules (direct, prerequisite at any depth, upward-only propagation, unaffected prerequisite record, segment read with and without a match, negated clause, nested segment, unresolved definition, prerequisite failure, malformed override, error raised during a prerequisite evaluation, big segments status kept). Full suite and RuboCop are clean. Each new spec was checked against a deliberate defect in the code it covers. The existing FDv1 and FDv2 file data sources keep their current behavior. This series does not change them; the override feature is additive. This PR also carries a spec-only fix for the Windows job, which failed or passed depending on the random example order. Ruby 3.1 through 3.3 report file modification times in whole seconds on Windows, so a truncation 20 ms after another one does not look like a new write. The reloader specs that need a rewrite to look new now set the file's modification time themselves, and the watcher spec that replaces a symbolic link waits for the next second first on such a platform. No code under `lib/` changes, and the specs came in with #449, so the fix sits at the bottom of the stack. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=Overrides --> <sub>Opened from termag</sub>
<!-- TERMAG_PR_BODY_BEGIN --> ## Summary This PR is stacked on #450 because it uses the override marker, the evaluator marking, and the reason indicator that PR adds. This change adds the override layer that the OVERRIDE specification describes: a runtime-mutable collection of flag and segment definitions that takes precedence over LaunchDarkly data at the store read boundary, populated by an override source through a sink. With no override source configured the SDK behaves exactly as before. The file-based source, the events changes, and the contract test service follow in later PRs. `LaunchDarkly::Interfaces::Overrides::OverrideSource` and `OverrideSink` define how a source supplies complete snapshots to the SDK. The sink accepts data model objects or hashes in the flag and segment data model. The SDK marks the entries itself. `Impl::Overrides::Layer` holds marked entries in an immutable hash that is swapped on update, so the layer holds exactly one snapshot at any instant and reads are lock-free. `Impl::Overrides::Overlay` sits at the store read boundary: a read for a key returns the override entry when one exists and the LaunchDarkly entry otherwise, an enumeration is the union with the override entry winning, and initialization status is the base store's alone. `Impl::Overrides::Sink` applies snapshots and notifies flag change listeners of every flag whose merged-view evaluation may have changed, including flags that depend on an overridden prerequisite or segment. It computes dependency edges over both the old and the new merged view, as the Go reference does. `DataSystem::ConfigBuilder#overrides` accepts one override source builder, the same `build(sdk_key, config)` protocol as the data source builders. The FDv2 data system builds the source at construction, so an invalid configuration raises from `LDClient.new` like other invalid component configuration. It starts the source before its run loop, so the initial load is in effect before the constructor returns. An exception from the source's `start` does not prevent the client from starting: the SDK logs it, stops the source, removes any snapshot the source supplied, and the client runs without overrides (the SDK's own file source does not raise from `start`; a custom source could). It serves reads through the overlay. When the client closes, the sink stops accepting snapshots before the source is stopped, and an exception from `stop` is logged. An offline client builds no source and logs a warning that the configured source is not started. The client consults the override store before the not-initialized short-circuit. An overridden flag is served before LaunchDarkly data arrives, read from the override store itself, and a flag that is not overridden still returns the client-not-ready default, even when a store without an init marker holds LaunchDarkly data for it. `all_flags_state` reads through the overlay. Before initialization, when the store is not initialized and the override store holds flags, it returns a state of those flags alone, read from the override store, with a once-per-client warning; with no override flags the state is invalid, as it is without a source. Without a source this state is empty, because the SDK reports no flags before the store is initialized, so the override flags only add to what it reports; overrides never remove a flag from the state. A failure of the base store to list its flags propagates through the overlay, and `all_flags_state` logs it as without a source. Without a source the state is then invalid and empty; with override flags it holds those flags and stays invalid, so the overrides only add to it. A client-side SDK bootstrapped from the state uses its flags whatever `$valid` says (js-client-sdk 3.x and the 4.x browser SDK only log a warning), so the overridden values still reach it. A wrong-type migration result keeps the marking of the evaluation it replaces. When the evaluator raises for a flag that came from the override store, `variation_detail` and `all_flags_state` report the `EXCEPTION` error reason with `override_affected` set, from the flag's own marker. This also fixes a pre-existing crash: `all_flags_state` raised `NoMethodError` when an evaluation raised, because the failed flag has no evaluation state; it now reports that flag with no prerequisites. The marker is read only from the SDK's model objects: a custom store can return a flag as a plain hash, as the `FeatureStore` interface documents, and such a flag never came from the override store, so its failed evaluation still returns the unmarked `EXCEPTION` result instead of raising. A value-only override expands, through the file loading code from #449, into a flag that is on and serves its single value by fallthrough, which reports the FALLTHROUGH reason kind as the specification describes. The OVERRIDE specification's test vectors (`spec/fixtures/override-vectors/vectors.json`) run as a spec through the full client stack, checking value, variation index, and reason under the vectors' comparison rules. The vectors' per-evaluation summary marker is asserted once the event processor carries the marking, in the events PR. LaunchDarkly changes reach override dependents. `Layer` keeps a dependency graph of its entries in each snapshot, and `Store` expands every changed LaunchDarkly item through both its own graph and the layer's, to a fixpoint. A change to a prerequisite flag or a segment that an override entry references therefore notifies that entry's flag, the same way dependent flags are notified for LaunchDarkly data, and a chain may cross between LaunchDarkly data and overrides any number of times. The FDv2 store's defect that seeded its dependency fan-out with symbol keys, while `Impl::DependencyTracker` indexes dependents under string keys, is fixed on `main` by #470, which the stack's base now includes. The fan-out helper here also compares keys as strings, which the fixpoint needs in any case, and the store spec keeps the #470 specs beside this PR's specs for the override layer. Verification: specs for the layer (marking, replacement, snapshot atomicity under concurrent writers), the overlay (precedence, union, deleted base items, base failure, initialization), the sink (added, removed, changed, and unchanged entries, dependency fan-out through prerequisites and nested segments, both merged views, no-listener path, base failure, serialized updates), the FDv2 wiring (construction, start order, overlay reads, stop, offline, availability unaffected), the client (not-ready gate, all-flags state, change notifications through the flag tracker, migration marker, lifecycle), and the specification vectors. Full suite and RuboCop are clean, and the full suite passes on JRuby 9.4 (the specs wait on queues without `Queue#pop`'s timeout, which Ruby 3.1 and JRuby 9.4 lack). Each new spec was checked against a deliberate defect in the code it covers. The existing FDv1 and FDv2 file data sources keep their current behavior. This series does not change them; the override feature is additive. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **feat: Add the override store, overlay, and data system wiring** > > Adds experimental, runtime flag/segment overrides: `DataSystem::ConfigBuilder#overrides` accepts an `OverrideSource` builder, FDv2 builds `Layer` + `Overlay` + `Sink`, wires `Store` to cross-graph dependency notifications, and extends evaluator/reason plumbing so prerequisites, segments, and client APIs respect override markers while still documenting that the surface is experimental. > > **Highlights** > > - New override layer modules (`Impl::Overrides::{Layer,Overlay,Sink}`) plus `Interfaces::Overrides` contracts mark model objects, merge overrides ahead of LD data, and fan-out flag-change listeners using merged dependency graphs. > - `EvaluationReason`, evaluator state, prerequisite records, migrations, and `LDClient` paths (`variation_detail`, `all_flags_state`) now propagate `overrideAffected`, handle uninitialized stores by consulting pure override snapshots, and keep JSON compatibility by emitting the bit only when true. > - FDv2 data system starts/stops override sources safely (offline skip, resilient `start`/`stop`), exposes `override_items`, and coordinates with `Store` via external dependency graphs; specs/vectors cover layer semantics, notifier behavior, client flows, and file-data reloader timing fixes on coarse clocks. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1005ed0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> The all-flags report keeps an override-affected flag's `trackEvents`, `trackReason`, and `debugEventsUntilDate` in this PR; #452 turns them off (OVERRIDE Requirement 1.4.1.1) together with the rest of the event policy for override-affected evaluations. <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=Overrides --> <sub>Opened from termag</sub>
…ide-affected evaluations (#452) <!-- TERMAG_PR_BODY_BEGIN --> ## Summary This PR is stacked on #451 because it uses the override store and the marking that PR wires into the client. This change carries the override-affected marking from evaluation into analytics events, as the OVERRIDE specification requires. Event handling keys on the marking scalar that the client hands to the event processor, not on the evaluation reason. `EvalEvent` and `record_eval_event` gain an `override_affected` flag. The client sets it from the evaluation result for the evaluated flag, from each prerequisite record's own marking for prerequisite evaluations, and from the flag's own marker when an evaluation raises. An unknown flag is never marked. The event dispatcher produces no individual feature event and no debug event for a marked evaluation, whatever the flag's configuration requests. Marked evaluations appear in summary events only. The summarizer keys counters by version, variation, and the marker, so override-affected and other evaluations of the same flag, variation, and version accumulate into separate counters. The summary event writes `overrideAffected: true` on the marked counters only, following the existing `unknown` marker. Event ingestion must accept that property on summary counters before release. `all_flags_state` presents an override-affected flag with `trackEvents` false, `trackReason` false, and no `debugEventsUntilDate`, so a consumer bootstrapped from the state sends no individual events for it. The flag, its value, its version, and its reason stay in the state. When an evaluation raises, there is no evaluation result, so the flag's own override marker decides, as it does for the reason and for the event of a single evaluation. The specification vectors now also check the summary marker the client hands to the event processor for each evaluation. Verification: processor specs (no feature event, no debug event, marked counter, counter split, unmarked counters unchanged, marked prerequisite record), summarizer spec (split by marker), client specs (record arguments for direct, segment, prerequisite, unknown, and error cases; all-flags tracking fields including experiment tracking and details-only-for-tracked), and end-to-end payload specs through the real event processor (index plus marked summary only for an overridden flag, individual events for an unaffected prerequisite inside a marked evaluation, ordinary events when nothing is overridden). Full suite and RuboCop are clean. Each new spec was checked against a deliberate defect in the code it covers. The existing FDv1 and FDv2 file data sources keep their current behavior. This series does not change them; the override feature is additive. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Analytics now threads an **`override_affected`** marker from evaluations into `EvalEvent` / `record_eval_event`. The client sets it from the evaluation result (and per-prerequisite marking for prereq records; flag-store marker on errors). > > **Event pipeline:** Override-affected evals still hit summary counters and index events, but **no feature or debug events** are emitted even when tracking or debug mode is on. The summarizer buckets counts by version, variation, and the marker; summary payloads add **`overrideAffected: true`** only on marked counters. > > **`all_flags_state`:** For override-affected flags, **`trackEvents`**, **`trackReason`**, and **`debugEventsUntilDate`** are cleared so bootstrapped clients do not send individual events; values/reasons stay present. > > Specs cover processor, summarizer, client wiring, OVERRIDE vectors (`summaryOverrideAffected`), and end-to-end payloads, plus **`RecordingEventProcessor`** for tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e2e01d8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=projects --> <sub>Opened from termag</sub>
<!-- TERMAG_PR_BODY_BEGIN --> ## Summary This PR is stacked on #452 because it supplies the override store that the earlier PRs add, and its client-level spec relies on the marked reasons and events they produce. This change adds `LaunchDarkly::Integrations::FileData.override_source`, the file-based override source that the OVERRIDE specification defines. It is configured with `LaunchDarkly::DataSystem::ConfigBuilder#overrides`: ```ruby overrides = LaunchDarkly::Integrations::FileData.override_source(paths: ["/etc/launchdarkly/overrides.json"]) config = LaunchDarkly::Config.new(data_system_config: LaunchDarkly::DataSystem.default.overrides(overrides).build) ``` The source reads one or more JSON or YAML files in the file data source document format (`flags`, `flagValues`, `segments`) and supplies complete snapshots to the SDK's override store. Its options are `paths` (required, non-empty strings in precedence order; a path listed more than once is logged and the repeats are ignored; on Windows, where file names ignore case, paths that differ only in case count as the same path), `duplicate_keys_handling` (`:fail`, the default, or `:ignore`, which keeps the first file's entry), `change_detection` (`:polling`, the default, or `:watching` through the optional `listen` gem), and `poll_interval` (default 1 second, raised to the 1 second minimum with a warning). Invalid options raise `ArgumentError` when the client is created, the same way other invalid component configuration is reported. Unlike the Go reference, an unrecognized `duplicate_keys_handling` value and an unknown option key are rejected rather than defaulted, because the specification treats invalid configuration as a construction error and an emergency mechanism should fail loudly. Behavior comes from the shared file data code added in #449: - The format comes from each file's name, as sdk-specs #272 recommends: a name that ends in `.yaml` or `.yml`, in any case, is YAML, and any other file, including one with no extension, is JSON. The existing file data sources read every file with the YAML parser, which also reads most JSON; the override source does not, so a YAML document in a `.json` file, or JSON with a `#` comment or a trailing comma, does not load. Files must be UTF-8; a UTF-16 file, as Windows PowerShell 5.1 writes with `>`, fails to load with a clear message and keeps the overrides in effect. - An empty JSON file is not a valid document: it fails to load and the overrides in effect stay. To remove a file's overrides, delete the file or write `{}`. - Override files can be rewritten in place. When the source finds a file whose overrides are in effect empty (or, for YAML, with only a comment or `---`), it holds the file back and keeps the overrides in effect until the new content is written: until the file has stayed so, unwritten, for one retry delay (1 second), and for at most five retry delays while it keeps being rewritten. Changes to the other files wait with it, since a reload applies all files or none. The mechanism is described in #449; the public docs say only what an application needs. - A configured file that does not exist contributes no overrides, so a file can be created later and deleting a file removes its overrides; a file that exists but cannot be read or parsed fails that reload and keeps the last good overrides, with a logged failure and a retry; a rewrite with identical content is not reapplied; the initial load completes before the client constructor returns. A `flagValues` entry expands into a flag that is on and serves the value by fallthrough, so the evaluation reports the FALLTHROUGH reason kind. Every applied change is logged at Info with the overrides in effect and what each file supplied, and the reloader's log lines for this source, including the hold's debug lines, carry a `FileOverrideSource` prefix. The `override_source` docs say how the format is chosen, that files must be UTF-8, that comments and repeated keys in JSON depend on the application's `json` gem, that files can be rewritten in place, how to remove a file's overrides, that an empty JSON file is not a valid document, and that replacing a YAML file in one operation avoids briefly applying a partly written file. ## Testing - Specs for the builder (defaults, absolute paths, every invalid option, `listen` requirement, poll interval clamping), the initial load (merge, fallthrough-form expansion, YAML, missing file, all files missing, malformed file, duplicate keys under both handlings), the Info log format, polling change detection (change, appearance, deletion, malformed edit with recovery, identical rewrite, stop, stop before start), watching change detection (change, appearance, deletion), and a client-level check of serving and reloading overrides through `LDClient`. - The specs rewrite override files in place with real, deliberately slow writes. End-to-end specs check that the source never supplies a snapshot without the override, and that a client never serves the LaunchDarkly value, while a JSON or YAML file is rewritten in place with a 300 ms pause, with polling and watching change detection; also for a YAML file whose leading comment is written before the pause, for five rewrites about a second apart with the default timing, for a JSON file and a YAML file that stay empty, and for files that are UTF-16 or not valid UTF-8, at startup and at runtime. - The in-place rewrite handling was reviewed in eight rounds as #479 (multi-agent and Bugbot reviews); see #449 for the mutation check (67 mutants, 60 killed, 7 equivalent) and the loops (10 runs in a row on Ruby 3.2 and on JRuby 9.4, no failures). - The `flag overrides` suite of sdk-test-harness (3.2.0-alpha.13, built from #515, whose override files are rewritten in place and have no extension) passes against the test service of #454: 79 tests ran, all passed. - Full suite and RuboCop are clean, apart from `can be automatically loaded by Bundler.require`, which needs write access to the system gem directory on the test machine. Each new spec was checked against a deliberate defect in the code it covers. The existing FDv1 and FDv2 file data sources keep their current behavior. This series does not change them; the override feature is additive. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=Overrides --> <sub>Opened from termag</sub> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Adds **`LaunchDarkly::Integrations::FileData.override_source`** so operators can load **flag and segment overrides from local JSON/YAML files** that take precedence over LaunchDarkly at evaluation time, wired through **`DataSystem::ConfigBuilder#overrides`**. The new **`FileOverrideSource`** reuses the shared **`FileData::Reloader`** (with **`skip_missing_paths`**, **`skip_unchanged`**, polling or optional `listen` watching) and logs applied overrides with a **`FileOverrideSource:`** prefix. > > **`Reloader`** gains a configurable **`log_prefix`** (default `[LDClient]`) and logs unexpected errors directly instead of **`Util.log_exception`**. > > Documentation examples for custom data systems now use **`data_system_config:`** and **`.build`** on **`DataSystem`** builders instead of the old **`data_system:`** shape. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 46b384f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Summary This PR is stacked on #453 because the test service configures the file-based override source that PR adds. The contract test service now advertises the `flag-overrides` capability and accepts the `overrides` block of the SDK configuration described in the test harness service specification. `filePaths`, `duplicateKeysHandling`, `changeDetection`, and `pollIntervalMs` map to the options of `LaunchDarkly::Integrations::FileData.override_source`, which is set on the FDv2 data system builder. An `overrides` block without a `dataSystem` block is an error, as in the Go reference service. The `listen` gem joins the contract test bundle so that the harness's watching change detection tests can run. The service port can be set with the `PORT` environment variable and still defaults to 9000, so the existing Makefile and CI steps are unchanged. Verification against the prebuilt harness (sdk-test-harness branch rlamb/sdk-2657/flag-overrides-tests at 9ad082c, `flag-overrides` capability): `-run 'flag overrides'` ran 38 tests with 0 failures, and the full suite with no filter ran 4733 tests with 0 failures (13 skipped by pre-existing capability and long-running gates). The first run of the watching mode tests failed because the watcher relied on `listen`, which could not scan the system temporary directory where the harness writes its files; the watcher change in #449 corrects that, and both runs above were made with it in place. The existing FDv1 and FDv2 file data sources keep their current behavior. This series does not change them; the override feature is additive. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Enables the Ruby contract test client to wire the FDv2 `overrides` block to `LaunchDarkly::Integrations::FileData.override_source`, mapping harness fields to paths, duplicate-keys handling, change-detection mode, and optional polling interval, and rejects overrides when no `dataSystem` is configured to match harness expectations. > > Advertises the `flag-overrides` capability from the test service, allows the bind port to be overridden via `PORT` (default 9000), and adds the `listen` gem to support watching change detection for file-based overrides. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit bb7a5ca. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…nd the key (#493) <!-- TERMAG_PR_BODY_BEGIN --> **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [ ] I have validated my changes against all supported platform versions (run on Ruby 3.2 only) **Related issues** - Spec: launchdarkly/sdk-specs#288 (Requirements 1.1.1.1, 1.2.8.1 and 1.3.6.2: the override layer never holds a deleted item). - [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) This PR is stacked on #454 so that the PRs below it do not change. Its base is `rlamb/overrides-ruby-contract-tests`. **Describe the solution you've provided** Before: - The override sink already rejected a snapshot with a deleted flag or segment with `ArgumentError`, before the layer changed. Nothing logged the rejection when a custom source supplied the snapshot after it had started. The message used the internal kind namespace: `the override features entry f is deleted`. - The file override source failed the reload only through that sink check. The log named neither the file nor the kind as users know it: `Unable to load flags: the override features entry f is deleted`. After: - The file override source checks each file as it reads it, before it supplies a snapshot. A file with a flag or segment entry marked as deleted fails the reload in the same way as a file that cannot be parsed: the previous overrides stay, the existing retry applies, and the failure is logged at Error level with the file and the key, for example `[LDClient] FileOverrideSource: Unable to load flags: flag "f" in /path/overrides.json is marked as deleted; remove the entry to remove its override`. The check runs through a new `check_document` option of the shared reloader (`impl/file_data/reloader.rb`), and only the override source sets it. - The sink still raises `ArgumentError` for a snapshot with a deleted flag or segment. It now also logs the rejection at Error level, with "flag" or "segment" and the key: `[LDClient] Rejected an override snapshot, keeping the overrides in effect: override flag "f" is marked as deleted; leave the flag out of the snapshot to remove its override`. The whole snapshot is rejected, the layer is unchanged, and no change notifications are sent. - `deleted: false` is an ordinary definition and still loads. - Public docs: `FileData.override_source` and `OverrideSink#set_overrides` each gain one sentence about entries marked as deleted. The ordinary file data sources (v1 `FileDataSourceImpl` and FDv2 `FileDataSourceV2`) are unchanged on purpose. Rejecting deleted entries there would be a breaking change, which is planned for a later major version. Neither of them uses the shared `impl/file_data` engine. The new specs pin that both of them, and the reloader without a document check, still load entries marked as deleted. Tests added or changed: - `spec/impl/overrides/sink_spec.rb`: a deleted flag, and a deleted segment given as a model object, each reject the whole snapshot. The other entries are not applied, the layer is unchanged, nothing is notified, and the Error log names the kind and the key. Entries with `deleted: false` are accepted. The existing test's message expectations moved into these tests. - `spec/integrations/file_override_source_spec.rb`: a deleted flag fails the initial load, and the log names the file and the key. On a running source, a deleted flag keeps the previous overrides, and the fix is applied. A deleted segment in a YAML file keeps every file's overrides, including a change to the other file, until the segment entry is fixed. `deleted: false` loads. With a client, the override is still served, the log names the file, and the sink's rejection never appears because the source fails the load first. - `spec/impl/file_data/reloader_spec.rb`: a document check that raises fails the load and keeps the last good result. The other files are still checked. Without a check, deleted entries are combined as before. - `spec/integrations/file_data_source_v2_spec.rb`, `spec/integrations/file_data_source_spec.rb`: pins that the data sources still load entries marked as deleted. How I ran them (Ruby 3.2.3): - `bundle exec rspec spec/impl/overrides spec/impl/file_data spec/integrations/file_override_source_spec.rb spec/integrations/file_data_source_v2_spec.rb spec/integrations/file_data_source_spec.rb`: 312 examples, 0 failures. - `bundle exec rspec spec` (database store specs skip by default): 1681 examples, 1 failure. The failure is `spec/launchdarkly-server-sdk_spec.rb:5`, the `Bundler.require` autoload test. Its inline bundler gets `Permission denied` on `/var/lib/gems/3.2.0/bundler.lock` on this machine, so it is a local environment problem and unrelated to this change. - I ran the new specs against the parent commit with only the spec files copied. The 7 behavior tests fail there, and the 5 pins and `deleted: false` tests pass. Two mutants also fail the expected tests: removing the sink's log line, and skipping segments in the file check. - `bundle exec rubocop` on the changed files: no offenses. **Describe alternatives you've considered** - Checking only in the sink: the sink cannot name the file. Under retries, the reloader logs repeats of the same failure at Debug, so the first Error line would still not say which file to fix. - Checking inside `FileData.merge`: merge does not know the file paths, and the ordinary data sources would share it once they move onto the engine. A per-document hook keeps the rule on the override path. - Logging every sink rejection, including an entry without a key: a file with an empty key (`"": ...`) reaches the sink, and the reloader retries every second. An Error line from the sink on each retry would repeat what the reloader already logs once. So only the deleted-item rejection, which the file source never reaches now, is logged by the sink. **Additional context** Separate follow-up, not fixed here: the v1 file data source has its own bug. `LDClient.new` raises `NoMethodError` when a file has an entry marked as deleted, because `dependency_tracker.rb:60` checks `deleted` only on a hash. Line 70 then reads the prerequisites of a deleted model, which are nil. The same happens on `main`. The v1 pin in this PR writes to a data store without an update sink, so it does not touch that path. <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=ruby-server-sdk --> <sub>Opened from termag</sub> [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Adds a **`check_document`** hook to `FileData::Reloader` so each parsed file can be validated before merge; a `MergeError` from the hook fails the reload like a parse error and keeps the last good snapshot. > > The **file override source** wires this to **`reject_deleted_entries`**, which rejects files containing flag or segment entries with `deleted` set (other than false/null), logging errors that name the **file path** and **key**. **Override sink** still rejects full snapshots with deleted items but now uses user-facing "flag"/"segment" wording and **logs an ERROR** when rejecting, so custom sources get visibility too. > > **Public docs** for `FileData.override_source` and `OverrideSink#set_overrides` note that deleted entries fail loads / are rejected. **v1 and FDv2 file data sources** are unchanged and still accept deleted entries (pinned by new specs). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cbf2a1d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…ich deployments watching does not follow (#484) <!-- TERMAG_PR_BODY_BEGIN --> ## Summary Follow-up to #449 for two findings raised on the Python port of the same watcher (launchdarkly/python-server-sdk#525), both of which apply here. - A configured link that cannot be resolved keeps the once-per-second retry running. When a retry resolved such a link to a target in a directory that was already watched, for example because another configured file is in it, it added no watch and so ran no callback. Content that appeared while the link could not be resolved was then not read until some later change. `refresh_links` now reports a change whenever a link leads to a target it did not lead to before, so the retry runs the callback once. - The watcher comment said that a link that comes to point elsewhere is followed. That holds when the old target's directory is deleted, as a ConfigMap update does, but not for a link further along the chain that is pointed elsewhere while its old target stays, a configured directory that is itself a link and is pointed elsewhere, or a configured directory that is renamed and replaced. The comment now names those cases and points such deployments, like a `current` link to a release directory, to polling, which is the default. The override source's `change_detection` documentation says the same in #453. ## Tests A new watcher spec configures a link that leads through a directory that does not exist yet to a directory that another configured file keeps watched, then creates the missing directory as a link. It fails without the change and passes with it. The watcher specs pass (24 examples), the full suite passes apart from `spec/launchdarkly-server-sdk_spec.rb:5`, which fails the same way on `feat/overrides` on this box because Bundler cannot write its lock file, and RuboCop reports no offenses. [SDK-3249](https://launchdarkly.atlassian.net/browse/SDK-3249) <!-- TERMAG_PR_BODY_END --> <!-- termag: box=i-0575d1c54853058d7 project=ruby-server-sdk --> <sub>Opened from termag</sub> [SDK-3249]: https://launchdarkly.atlassian.net/browse/SDK-3249?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Fixes **file data watcher** behavior when a configured symlink finally resolves to a file in a directory that was **already** being watched (e.g. another configured path). `refresh_links` now treats newly discovered link targets in existing watches as a change, so the retry loop invokes **`on_change` once** instead of silently skipping until some unrelated filesystem event. > > Watcher docs are tightened to list symlink/deployment patterns **inotify does not track** (mid-chain repointing, directory-as-symlink swaps, rename-and-replace layouts) and to steer **`current` → release** style setups toward **polling**. > > Specs cover resolution into an already-watched directory and assert the callback is **not** fired on every failed watch retry. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d232195. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ 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 156611b. Configure here.
| reason: detail.reason, | ||
| prerequisites: eval_state.prerequisites, | ||
| # A failed evaluation has no evaluation state, so it reports no prerequisites. | ||
| prerequisites: eval_state&.prerequisites, |
There was a problem hiding this comment.
Failed flag eval reuses prior result
Medium Severity
eval_result and eval_state are assigned inside the all_flags_state loop and never cleared when evaluate raises. In Ruby those names are method-local, so a later failed flag keeps the previous flag’s result. eval_result.nil? is then false, so override_affected and prerequisites come from the earlier flag. Bootstrap JSON can list another flag’s prerequisites, and tracking fields can be wrongly enabled or suppressed.
Reviewed by Cursor Bugbot for commit 156611b. Configure here.
Member
Author
There was a problem hiding this comment.
This reply was written by an AI agent (Claude Code) working on behalf of @kinyoklion.
This one does not apply. eval_result and eval_state are assigned only inside the features.each do |k, f| block, and nowhere earlier in all_flags_state. A local variable first assigned inside a block belongs to that block, and each iteration starts with it unset. So when evaluate raises for a flag, eval_result is nil for that flag, and override_affected and prerequisites cannot carry over from the flag before it. For example, ruby -e '[1,2].each { |i| (r, st) = [i, :s] if i == 1; p [i, r, st] }' prints [1, 1, :s] and then [2, nil, nil].
| prereq_flag[:debugEventsUntilDate], | ||
| prereq_of_flag[:key], | ||
| prereq_flag[:samplingRatio], | ||
| !!prereq_flag[:excludeFromSummaries] |
There was a problem hiding this comment.
Pre-init eval reads leftover store data
Medium Severity
Before initialization, only the top-level flag is read from the override store. evaluate still resolves prerequisites and segments through the overlay, which falls through to the base store. A store that is not initialized can still hold LaunchDarkly data, so an override flag can be decided by leftover prerequisites or segments that a direct evaluation of those keys would refuse to serve.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 156611b. Configure here.
Member
Author
There was a problem hiding this comment.
This reply was written by an AI agent (Claude Code) working on behalf of @kinyoklion.
This is the intended behavior, and @kinyoklion has accepted it. Before initialization, the SDK serves a flag that the override layer holds instead of returning the not-ready default (OVERRIDE Requirement 1.5.3). Every definition that the evaluation then reads goes through the read boundary (Requirement 1.5.1), so a prerequisite or segment that has no override comes from LaunchDarkly data, including data in a store that is not initialized yet. The Go SDK behaves the same way.
An operator who wants an override flag to depend only on overridden definitions can override its prerequisites and segments as well.
keelerm84
approved these changes
Nov 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BEGIN_COMMIT_OVERRIDE
feat: Add experimental support for flag overrides, read from local JSON or YAML files or supplied by a custom override source
feat: Mark evaluations that a flag override affected with
override_affectedin the evaluation reason, and count them under separate summary countersEND_COMMIT_OVERRIDE
Every change in this PR was already reviewed and merged through its own PR against
feat/overrides: #449, #450, #451, #452, #453, #454, #484, #493. This PR merges the branch intomainand adds nothing of its own.Flag overrides let an operator force flags and segments to known definitions on a running client, whether or not the client can reach LaunchDarkly, and remove the override when the incident is over. They are an option of the FDv2 data system (
LaunchDarkly::DataSystem::ConfigBuilder#overrides), with a file-based source inLaunchDarkly::Integrations::FileData.override_source, and are experimental.mainhas one commit thatfeat/overridesdoes not (#490), and the two merge without conflicts. CI onfeat/overrides@ 156611b is green on every job, Windows included.Opened from termag
Note
Overview
Adds experimental flag overrides so operators can force known flag/segment definitions at evaluation time without replacing LaunchDarkly streaming or polling.
Configuration & data path. FDv2 gains an optional override source via
DataSystemConfig/ConfigBuilder#overrides. When set, the client builds an override layer, overlays it on the feature store for reads, and wires the layer’s dependency graph into the main store so prerequisite/segment changes still fan out to affected flags. Contract tests advertiseflag-overridesand can wire file paths throughLaunchDarkly::Integrations::FileData.override_source.File-based source. New
FileDataplumbing (parse/merge, poller, reloader with in-place rewrite handling, optionallisten-based watcher) backsFileData.override_sourcewith polling or watching, duplicate-key policy, and resilient reload/retry. Thelistengem is added for contract tests’ watching mode.Evaluation & analytics. Flag/segment models carry an internal override marker. The evaluator propagates override-affected marking through prerequisites and segment matches;
EvaluationReasonexposesoverrideAffectedin JSON when true. Override-affected evaluations are excluded from individual and debug events but get separate summary counters keyed by the marker.Client behavior. Before initialization (or when the store is unavailable), evaluations and
all_flags_statecan serve override entries only, with one-time warnings. Docs/examples shift todata_system_config: ... .buildinstead of passing raw builders.Reviewed by Cursor Bugbot for commit 156611b. Bugbot is set up for automated code reviews on this repo. Configure here.