Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLI-922] Support combined segment and threshold constraint #2892

Open
1 task done
GeorgeMac opened this issue Mar 23, 2024 · 0 comments
Open
1 task done

[FLI-922] Support combined segment and threshold constraint #2892

GeorgeMac opened this issue Mar 23, 2024 · 0 comments
Labels
enhancement Created by Linear-GitHub Sync lg Little bit big linear

Comments

@GeorgeMac
Copy link
Member

GeorgeMac commented Mar 23, 2024

Problem

The variant type flag supported distirbutions directly on segment matches, as opposed to separated segment vs threshold matches as we have in the boolean flag. While this was restrictive in the sense that you had to have a segment to defined a threshold on a variant flag, it did allow you to express that a context was both in a segment AND within some threshold.

We should add some mechanism to support this usecase in Boolean flag type.

i.e. to express that a context both matches a segment and is within some threshold (e.g. 30% of internal users).

Ideal Solution

There are a few ways we could express this. Each vary in terms of implementation effort.

Whichever we change, it is going to touch quite a few places. At-least:

  • The protocol definitions (either additional fields or potentially new messages too)
  • The Go server-side evaluator
  • The Rust client-side evaluator
  • The relational (common SQL) and declarative (snapshot) storage layers
  • Additional integration test cases

Here are two that come immediately to mind:

  1. Add a new rollout type

Segment and Threshold are both "rollout types" for boolean type flags.
We could add a third, something like "SegmentWithThreshold".
This would be to distinguish it from the existing two:

flipt/rpc/flipt/flipt.proto

Lines 300 to 304 in 3f8e7bd

enum RolloutType {
UNKNOWN_ROLLOUT_TYPE = 0;
SEGMENT_ROLLOUT_TYPE = 1;
THRESHOLD_ROLLOUT_TYPE = 2;
}

This would involve adding new enum case, new rollout type messages and updating all the storage mechanisms to persist them. For example, currently the relational backend actually breaks these types out across different tables:
https://github.com/flipt-io/flipt/blob/main/internal/storage/sql/common/rollout.go#L19-L24

  1. Extend existing RolloutSegment with optional percentage threshold

flipt/rpc/flipt/flipt.proto

Lines 326 to 331 in 3f8e7bd

message RolloutSegment {
string segment_key = 1 [deprecated = true];
bool value = 2;
repeated string segment_keys = 3;
SegmentOperator segment_operator = 4;
}

We could simply add an optional percentage to this message.
The default for not suplying it would be to ignore the threshold.
Otherwise, supplying it would imply the normal entityId based bucketing strategy we do for other thresholds.

This approach would minimize the blast radius. The relational changes involved would be limited to adding an optional column to the existing segments rollout table.

Search

  • I searched for other open and closed issues before opening this

Additional Context

No response

FLI-922

@GeorgeMac GeorgeMac added the enhancement Created by Linear-GitHub Sync label Mar 23, 2024
@markphelps markphelps changed the title Support combined segment and threshold constraint [FLI-922] Support combined segment and threshold constraint Mar 25, 2024
@GeorgeMac GeorgeMac added the lg Little bit big label Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Created by Linear-GitHub Sync lg Little bit big linear
Projects
Status: No status
Development

No branches or pull requests

2 participants