Repository navigation
feat: Enable Flag Dependency in the Rust flag engine - #42
Open
linear-code[bot] wants to merge 7 commits into
Open
linear-code[bot] wants to merge 7 commits into
linear-code[bot] wants to merge 7 commits into
Conversation
Bumps engine-test-data to v3.10.0. Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Segment conditions can read $.flags.<feature>.enabled|value|variant. Flags in a dependency cycle get reason ERROR; code=CIRCULAR_DEPENDENCY. Bumps engine-test-data to v3.11.0. Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
STATIC if no segment overrides the feature, DEFAULT if overrides exist but none matched. Bumps engine-test-data to v3.12.1. Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
RFC 9535 allows blank space before segments and inside bracketed selections. Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
2 of 4 tasks
Covers the engine-test-data contexts flagsmith-engine benchmarks, the rest of the corpus, and a large environment with and without flag dependencies. Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
JSONPath conditions read one serialisation of the context instead of each making their own, and evaluations reading no flag skip building the dependency index. Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Generated with [Linear](https://linear.app/flagsmith/agent-session/32b03921-b061-45c1-8951-e6bc08ade1a8) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
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.
Flags can't depend on other flags in the Rust engine yet, so the Rust SDK can't evaluate Flag Dependency locally. This ports flagsmith-engine #343 and brings in two smaller engine changes that the current engine-test-data (v3.12.1) also expects. Each change is a separate commit.
Changes
enabled,valueorvariantthrough$.flags.<feature>…, including chains of dependencies.ERROR; code=CIRCULAR_DEPENDENCY, and segments that run into a cycle don't match.STATICwhen no segment targets them andDEFAULTwhen segments target them but none matched, as in flagsmith-engine #347.For reviewers:
$.flags.a.enabledor$.flags['my feature'].enabled. Wildcards and filters read no flag. engine-test-data doesn't pin this case.serde_json_path(0.7.2, the latest) keeps its parsed AST private, so the flag name is pulled from the path text afterJsonPath::parsehas validated it.HashMap, and the result schema defines no order.FlagResultgains a publicvariantfield andFeatureValuegains a publickeyfield. Code that builds these structs field by field needs updating, which is why this ships as a minor release. flagsmith-rust-client builds and passes its tests against this branch with no changes.Performance
benches/engine.rs(criterion 0.8) measures:representative: the five engine-test-data contexts that flagsmith-engine benchmarks.corpus_without_flag_dependencies: the other v3.12.1 contexts (202 of them). The 18 contexts that read$.flagsare left out because the baseline evaluates them differently.large_environment/*: a synthetic environment with 200 features (20 multivariate) and 50 segments, plus an identity with 20 traits. Theflag_chainvariant adds a 49-step dependency chain andflag_fan_outhas 50 segments reading five flags. The baseline can't resolve these dependencies, so for them it measures the same contexts without the dependency work.Baseline is
mainat ba209c6, with the same bench file and the same v3.12.1 test data. Median times, from interleaved criterion runs on a 1 vCPU AMD EPYC 9655P VM with Rust 1.99.1:Before the
perf:commit, flag dependency made every workload slower except fan-out (representative +23%, identity +20%, environment +51%). That commit serializes the context to JSON once per evaluation instead of once per JSONPath condition, which was the dominant cost onmaintoo. It also builds the dependency index only once a condition reads a flag.Limitations:
large_environment/environment(+28%) reads no JSONPath property. Its extra cost is recording overrides of unmatched segments, which theSTATIC/DEFAULTreasons from #347 need. Removing that recording puts it 11% faster thanmain, but the reasons are then wrong. Withfoldhashhashing for that one map it measured +15%. I left that out because it would add a runtime dependency.corpus_without_flag_dependenciesmeasured between 2.05 and 2.35 ms across runs, whilemainstayed between 2.11 and 2.14 ms. I haven't found the cause.perf:commit, about 0.7 ms of theidentityregression couldn't be explained: both trees serialized the context the same number of times, at the same cost per call. No profiler was available.To reproduce:
Contributes to SDK-11
Review effort: 4/5