Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/internal/feature-flags/api-writes.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ A stored version 2 row that is not remote-configured and carries no encrypted pa
- Writer-only rules (`check_writer_rules` in `facade/config_writes.py`, currently the `$false`, `$true` and `"null"` reservation and the regex check below) apply to a created or replacing document and to the stored document being enabled. The shared validator, and with it the flags caches and the stored side of an update, does not apply them: tightening a writer rule never hides a stored row from evaluation or blocks replacing it. A `"null"` value the stored document already serves, as the default, a rule value or a variant value, passes the reservation, as in v1.
- Experiment-rule variant keys follow the v1 variant-key rule for every return type: a new `"null"` key is rejected, writer-only, unless the stored rule with the same ID already uses that key. The key belongs to that rule's experiment, so another rule cannot take it over.
- `regex` and `not_regex` patterns must compile with Python's `re`, as on the v1 write path. This is a writer-only rule: readers store an uncompilable pattern and report an evaluation error when evaluation reaches it. The flags service compiles with `fancy_regex`, whose dialect differs at the edges, so, as in v1, a pattern the stored document already holds is not checked again, and enabling checks no pattern.
- Rule types are `targeted_release`, `percentage_rollout` and `experiment`. An experiment rule names no Experiment: an Experiment links to its rule through the rule ID (`Experiment.feature_flag_rule_id`), and writing the rule creates or links no Experiment row. The rule splits the subjects its `rollout_percentage` enrolls across two to twenty `variants` (unique keys of letters, digits, `-` and `_`; weights with at most two decimal places that total exactly 100; values of the flag's return type, equal values allowed), returns the flag default while `paused` is true, and may carry a `holdout` that belongs to the rule: `{seed, exclusion_percentage}`, where the client chooses `exclusion_percentage` and the server owns the seed. At `/flags?v=3` a variant assignment reports `experiment_split` with the rule ID and the variant key; no response carries an experiment or holdout ID. An `experiment_id` on the rule and an `id` in its holdout are unknown fields, whatever their value, `null` included.
- Rule types are `targeted_release`, `percentage_rollout` and `experiment`. An experiment rule names no Experiment: an Experiment links to its rule through the rule ID (`Experiment.feature_flag_rule_id`), and writing the rule creates or links no Experiment row. The rule splits the subjects its `rollout_percentage` enrolls across two to twenty `variants` (unique keys of letters, digits, `-` and `_`; weights with at most two decimal places that total exactly 100; values of the flag's return type, equal values allowed), returns the flag default while `paused` is true, and may carry a `holdout` that belongs to the rule: `{seed, exclusion_percentage}`, where the client chooses `exclusion_percentage` and the server owns the seed. At `/flags?v=4` a variant assignment reports `experiment_split` with the rule ID and the variant key; no response carries an experiment or holdout ID. An `experiment_id` on the rule and an `id` in its holdout are unknown fields, whatever their value, `null` included.
- Rule IDs, the seeds of percentage rollouts and experiment rules, and holdout seeds are server-owned and identify rules rather than list positions. An echoed ID keeps its seeds through reordering and unrelated edits, and a rule keeps its seed while it stays a percentage rollout or an experiment rule; a rule that omits its ID is new and receives fresh identity. An unknown ID, an ID used twice, a client-chosen seed, and a changed seed are all rejected.
- `FeatureFlag.version` is required on every update and must equal the locked row's value. Unlike v1 there is no `original_flag` merge: a stale token is a conflict even when no individual field clashes. The token applies to disabling and deleting too.
- Identity resolution, validation and the enable check run against the locked row inside the existing update transaction. A stored document with an unsupported shape or semantics is rejected rather than replaced or enabled. Configuration and metadata byte limits apply to a replacement, so an oversized stored document can be edited back under a lowered limit; enabling requires the stored document to fit the current limits.
Expand Down
78 changes: 56 additions & 22 deletions rust/feature-flags/src/api/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
errors::{ClientFacingError, FlagError},
flags_rate_limiter::RateLimitResult,
types::{
ConfigResponse, FlagsQueryParams, FlagsResponse, FlagsResponseV3, LegacyFlagsResponse,
ConfigResponse, FlagsQueryParams, FlagsResponse, FlagsResponseV4, LegacyFlagsResponse,
ServiceResponse,
},
},
Expand Down Expand Up @@ -167,13 +167,13 @@ fn get_minimal_flags_response(
headers: &HeaderMap,
version: Option<&str>,
is_from_legacy_decide: bool,
v3_enabled: bool,
v4_enabled: bool,
) -> Json<ServiceResponse> {
let request_id = extract_request_id(headers);

// `None` is preserved so `get_versioned_response` can apply its
// decide/non-decide defaults.
let version_num = version.map(|v| v.parse::<i32>().unwrap_or(1));
let version_num = version.map(parse_version);

let mut config = ConfigResponse::new();
config.set(
Expand All @@ -192,12 +192,16 @@ fn get_minimal_flags_response(
response.config = config;

let (service_response, _) =
get_versioned_response(is_from_legacy_decide, version_num, v3_enabled, response).expect(
get_versioned_response(is_from_legacy_decide, version_num, v4_enabled, response).expect(
"get_versioned_response is total for any (bool, Option<i32>, bool, FlagsResponse)",
);
Json(service_response)
}

fn parse_version(version: &str) -> i32 {
version.parse::<i32>().unwrap_or(1)
}

/// Determines the response format based on whether the request came from decide and the version parameter.
///
/// When the request is from decide (X-Original-Endpoint: decide):
Expand All @@ -207,15 +211,15 @@ fn get_minimal_flags_response(
/// - v>=4 -> FlagsV2 response format
///
/// When the request is not from decide:
/// - v>=3 -> FlagsV3 response format when `v3_enabled`, otherwise FlagsV2
/// - v=2 -> FlagsV2 response format
/// - v=4 -> FlagsV4 response format when `v4_enabled`, otherwise FlagsV2
/// - any other v>=2 -> FlagsV2 response format
/// - v=1 or missing -> FlagsV1 response format
///
/// Returns a tuple of (response, format_name) for logging purposes
fn get_versioned_response(
is_from_legacy_decide: bool,
version: Option<i32>,
v3_enabled: bool,
v4_enabled: bool,
response: FlagsResponse,
) -> Result<(ServiceResponse, &'static str), FlagError> {
if is_from_legacy_decide {
Expand Down Expand Up @@ -247,10 +251,10 @@ fn get_versioned_response(
))
}
}
} else if serves_flags_v3(is_from_legacy_decide, version, v3_enabled) {
} else if serves_flags_v4(is_from_legacy_decide, version, v4_enabled) {
Ok((
ServiceResponse::V3(FlagsResponseV3::from_response(response)),
"FlagsV3",
ServiceResponse::V4(FlagsResponseV4::from_response(response)),
"FlagsV4",
))
} else if version.is_some_and(|v| v >= 2) {
Ok((ServiceResponse::V2(response), "FlagsV2"))
Expand All @@ -262,10 +266,13 @@ fn get_versioned_response(
}
}

/// True when [`get_versioned_response`] returns the v3 record. The body log
/// True when [`get_versioned_response`] returns the v4 record. The body log
/// uses it too, so the logged record matches the record the client receives.
fn serves_flags_v3(is_from_legacy_decide: bool, version: Option<i32>, v3_enabled: bool) -> bool {
v3_enabled && !is_from_legacy_decide && version.is_some_and(|v| v >= 3)
///
/// Only an exact `v=4` selects it: clients already send `v=3` and arbitrary
/// `v` values (cache busters, typos), and those keep the v2 record.
fn serves_flags_v4(is_from_legacy_decide: bool, version: Option<i32>, v4_enabled: bool) -> bool {
v4_enabled && !is_from_legacy_decide && version == Some(4)
}

/// Feature flag evaluation endpoint.
Expand Down Expand Up @@ -313,7 +320,7 @@ where
&headers,
query_params.version.as_deref(),
false,
state.config.flags_v3_response_enabled,
state.config.flags_v4_response_enabled,
)
.into_response());
}
Expand Down Expand Up @@ -453,7 +460,7 @@ where
&headers,
query_params.version.as_deref(),
is_from_legacy_decide,
state.config.flags_v3_response_enabled,
state.config.flags_v4_response_enabled,
)
.into_response());
}
Expand All @@ -472,11 +479,8 @@ where
}

// Parse version from query params (needed for response formatting)
let query_version = modified_query_params
.version
.as_deref()
.map(|v| v.parse::<i32>().unwrap_or(1));
let v3_enabled = state.config.flags_v3_response_enabled;
let query_version = modified_query_params.version.as_deref().map(parse_version);
let v4_enabled = state.config.flags_v4_response_enabled;

// Install a body-logging side channel only when at least one team is
// opted in. The OnceLock is filled inside the request decode flow with
Expand Down Expand Up @@ -581,11 +585,11 @@ where
log.team_id,
decoded_body,
&response,
serves_flags_v3(is_from_legacy_decide, query_version, v3_enabled),
serves_flags_v4(is_from_legacy_decide, query_version, v4_enabled),
);

// Determine the response format based on whether request is from decide and version
match get_versioned_response(is_from_legacy_decide, query_version, v3_enabled, response)
match get_versioned_response(is_from_legacy_decide, query_version, v4_enabled, response)
{
Ok((versioned_response, response_format)) => {
log.http_status = 200;
Expand Down Expand Up @@ -721,6 +725,36 @@ mod tests {
http::Uri,
};

#[rstest]
#[case::exact_v4("4", false, true, "FlagsV4")]
#[case::v4_setting_off("4", false, false, "FlagsV2")]
#[case::v3("3", false, true, "FlagsV2")]
#[case::v3_setting_off("3", false, false, "FlagsV2")]
#[case::v5("5", false, true, "FlagsV2")]
#[case::cache_buster("17380067", false, true, "FlagsV2")]
#[case::v2("2", false, true, "FlagsV2")]
#[case::fractional_v4("4.0", false, true, "FlagsV1")]
#[case::huge("12345678901234567890123456789012345678901234", false, true, "FlagsV1")]
#[case::junk("abc", false, true, "FlagsV1")]
#[case::decide_v4("4", true, true, "FlagsV2")]
#[case::decide_v3("3", true, true, "FlagsV1")]
fn test_only_an_exact_v4_selects_the_v4_record(
#[case] v: &str,
#[case] is_from_legacy_decide: bool,
#[case] v4_enabled: bool,
#[case] expected: &str,
) {
let version = Some(parse_version(v));
let response = FlagsResponse::new(false, HashMap::new(), None, Uuid::new_v4());
let (_, format) =
get_versioned_response(is_from_legacy_decide, version, v4_enabled, response).unwrap();
assert_eq!(format, expected);
assert_eq!(
serves_flags_v4(is_from_legacy_decide, version, v4_enabled),
expected == "FlagsV4"
);
}

#[tokio::test]
async fn test_query_param_extraction() {
// Test case 1: Full query string
Expand Down
30 changes: 15 additions & 15 deletions rust/feature-flags/src/api/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub struct FlagsQueryParams {
pub enum ServiceResponse {
Default(LegacyFlagsResponse),
V2(FlagsResponse),
V3(FlagsResponseV3),
V4(FlagsResponseV4),
DecideV1(DecideV1Response),
DecideV2(DecideV2Response),
}
Expand Down Expand Up @@ -224,7 +224,7 @@ pub struct FlagsResponse {
/// Set to `true` when the team is gated into slim `$feature_flag_called` events
/// (TeamFeatureFlagsConfig.minimal_flag_called_events). Omitted otherwise, so SDKs
/// that see no field at all fall back to full events, same as legacy teams.
/// Only reaches the wire on this v2 shape and on `FlagsResponseV3`: `LegacyFlagsResponse`,
/// Only reaches the wire on this v2 shape and on `FlagsResponseV4`: `LegacyFlagsResponse`,
/// `DecideV1Response`, and `DecideV2Response` intentionally never carry it over. SDKs old
/// enough to hit those response shapes predate this field and have no code path that reads
/// it, so there's nothing gained by sending it to them.
Expand Down Expand Up @@ -451,7 +451,7 @@ pub struct FlagDetails {
pub config_outcome: ConfigOutcome,
}

/// The config format and, for a v2 flag that evaluated, the typed outcome the v3 record needs.
/// The config format and, for a v2 flag that evaluated, the typed outcome the v4 record needs.
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub enum ConfigOutcome {
#[default]
Expand All @@ -471,9 +471,9 @@ impl ConfigOutcome {

#[derive(Debug, PartialEq, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct FlagsResponseV3 {
pub struct FlagsResponseV4 {
pub errors_while_computing_flags: bool,
pub flags: HashMap<String, FlagDetailsV3>,
pub flags: HashMap<String, FlagDetailsV4>,
#[serde(skip_serializing_if = "Option::is_none")]
pub quota_limited: Option<Vec<String>>,
pub request_id: Uuid,
Expand All @@ -484,14 +484,14 @@ pub struct FlagsResponseV3 {
pub config: ConfigResponse,
}

impl FlagsResponseV3 {
impl FlagsResponseV4 {
pub fn from_response(response: FlagsResponse) -> Self {
Self {
errors_while_computing_flags: response.errors_while_computing_flags,
flags: response
.flags
.into_iter()
.map(|(key, flag)| (key, FlagDetailsV3::from(flag)))
.map(|(key, flag)| (key, FlagDetailsV4::from(flag)))
.collect(),
quota_limited: response.quota_limited,
request_id: response.request_id,
Expand All @@ -502,22 +502,22 @@ impl FlagsResponseV3 {
}
}

/// `/flags?v=3` record: one typed `value` instead of `enabled` and `variant`.
/// `/flags?v=4` record: one typed `value` instead of `enabled` and `variant`.
#[derive(Debug, PartialEq, Deserialize, Serialize)]
pub struct FlagDetailsV3 {
pub struct FlagDetailsV4 {
pub key: String,
/// `null` is a null flag default or a failed record.
pub value: Value,
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
pub failed: bool,
pub reason: FlagEvaluationReason,
pub metadata: FlagDetailsMetadataV3,
pub metadata: FlagDetailsMetadataV4,
#[serde(skip_serializing_if = "Option::is_none")]
pub conditions: Option<Vec<ConditionAnalysis>>,
}

#[derive(Debug, PartialEq, Eq, Deserialize, Serialize)]
pub struct FlagDetailsMetadataV3 {
pub struct FlagDetailsMetadataV4 {
pub id: i32,
pub version: i32,
pub config_version: u8,
Expand All @@ -532,7 +532,7 @@ pub struct FlagDetailsMetadataV3 {
pub variant_key: Option<String>,
}

impl From<FlagDetails> for FlagDetailsV3 {
impl From<FlagDetails> for FlagDetailsV4 {
fn from(flag: FlagDetails) -> Self {
let legacy_value = match flag.to_value() {
FlagValue::Boolean(enabled) => Value::Bool(enabled),
Expand Down Expand Up @@ -586,7 +586,7 @@ impl From<FlagDetails> for FlagDetailsV3 {
value,
failed: flag.failed,
reason,
metadata: FlagDetailsMetadataV3 {
metadata: FlagDetailsMetadataV4 {
id: flag.metadata.id,
version: flag.metadata.version,
config_version,
Expand Down Expand Up @@ -2528,7 +2528,7 @@ mod tests {
}

#[test]
fn test_v3_record_of_a_failed_v1_flag_has_a_null_value() {
fn test_v4_record_of_a_failed_v1_flag_has_a_null_value() {
let flag = FlagDetails {
key: "v1-failed".to_string(),
enabled: false,
Expand All @@ -2550,7 +2550,7 @@ mod tests {
config_outcome: ConfigOutcome::V1,
};

let record = FlagDetailsV3::from(flag);
let record = FlagDetailsV4::from(flag);

assert_eq!(record.value, Value::Null);
assert!(record.failed);
Expand Down
10 changes: 5 additions & 5 deletions rust/feature-flags/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,9 +719,9 @@ pub struct Config {
#[envconfig(from = "FLAGS_SESSION_REPLAY_QUOTA_CHECK", default = "false")]
pub flags_session_replay_quota_check: bool,

/// Serve the v3 record on `/flags?v=3` and above. Off, those requests get the v2 record.
#[envconfig(from = "FLAGS_V3_RESPONSE_ENABLED", default = "false")]
pub flags_v3_response_enabled: bool,
/// Serve the v4 record on `/flags?v=4`. Off, those requests get the v2 record.
#[envconfig(from = "FLAGS_V4_RESPONSE_ENABLED", default = "false")]
pub flags_v4_response_enabled: bool,

// Flag definitions rate limiting
// Default rate limit for all teams (requests per minute)
Expand Down Expand Up @@ -1304,7 +1304,7 @@ impl Config {
element_chain_as_string_excluded_teams: TeamIdCollection::None,
debug: FlexBool(false),
flags_session_replay_quota_check: false,
flags_v3_response_enabled: false,
flags_v4_response_enabled: false,
flag_definitions_default_rate_per_minute: 600,
flag_definitions_rate_limits: FlagDefinitionsRateLimits::default(),
flag_definitions_conditional_rate_per_minute: 6000,
Expand Down Expand Up @@ -1544,7 +1544,7 @@ mod tests {
assert_eq!(config.new_analytics_capture_endpoint, "/i/v0/e/");
assert_eq!(config.debug, FlexBool(false));
assert!(!config.flags_session_replay_quota_check);
assert!(!config.flags_v3_response_enabled);
assert!(!config.flags_v4_response_enabled);
assert_eq!(config.skip_writes, FlexBool(false));
// Bot filter ships in LogOnly mode by default — pin the safe
// posture so a future env-var rename / refactor can't silently
Expand Down
2 changes: 1 addition & 1 deletion rust/feature-flags/src/flags/evaluate_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<'a> Evaluation<'a> {
}
}

/// The parts of an [`Evaluation`] the v3 record reads, owned so they outlive the configuration.
/// The parts of an [`Evaluation`] the v4 record reads, owned so they outlive the configuration.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct EvaluationDetail {
pub code: &'static str,
Expand Down
4 changes: 2 additions & 2 deletions rust/feature-flags/src/flags/flag_matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub struct FeatureFlagMatch {
pub reason: FeatureFlagMatchReason,
pub condition_index: Option<usize>,
pub payload: Option<Value>,
/// Set only by `get_match_v2`; the v3 record is built from it.
/// Set only by `get_match_v2`; the v4 record is built from it.
pub evaluation_v2: Option<EvaluationDetail>,
}

Expand Down Expand Up @@ -2103,7 +2103,7 @@ impl FeatureFlagMatcher {
use_explicit_exact_matching: self.use_explicit_exact_matching,
now: self.now,
})?;
// `/flags?v=2` reports these v1 reasons; `/flags?v=3` reports the evaluator's own code.
// `/flags?v=2` reports these v1 reasons; `/flags?v=4` reports the evaluator's own code.
let reason = match evaluation {
Evaluation::TargetingMatch { .. } | Evaluation::ExperimentSplit { .. } => {
FeatureFlagMatchReason::ConditionMatch
Expand Down
Loading
Loading