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

[FEATURE] Change fractional custom op from percentage-based to relative weighting. #1282

Closed
toddbaert opened this issue Apr 15, 2024 · 6 comments · Fixed by #1313
Closed
Labels
enhancement New feature or request

Comments

@toddbaert
Copy link
Member

toddbaert commented Apr 15, 2024

Requirements

Currently, the fractional operator is percentage-based; meaning you must specify the relative distribution of the variants in terms of a percentage. I believe we should change this instead to be a relative weight, meaning that the integers do not need to sum to 100. For example, this fractional:

"fractional": [
          [
            "red",
            25
          ],
          [
            "blue",
            25
          ],
          [
            "green",
            25
          ],
          [
            "grey",
            25
          ]
        ]

Would remain valid, but could also be expressed as:

"fractional": [
          [
            "red",
            1
          ],
          [
            "blue",
            1
          ],
          [
            "green",
            1
          ],
          [
            "grey",
            1
          ]
        ]

We could even make the number in the bucket optional, so that an even split would be assumed if none was provided (we could simply default to 1).

Benefits:

  • adding new buckets becomes easy, there's no need to alter previous buckets
  • supports much finer targeting
    • clear and consistent support across implementations for targeting finer than a single percentage (I don't believe all impls currently support non-integer distributions)
    • easy a clear support for fractions that aren't factors of 100 (1/1/1 vs 33/33/34)
  • non breaking (current percentage based rules will still work)

Challenges:

  • we should specify max values for the integers, and perhaps a max array length for the bucket array

If there's a general consensus to this, I will create some child issues in the relevant repositories and use this issue as the parent "tracking" issue.

@toddbaert toddbaert added enhancement New feature or request Needs Triage This issue needs to be investigated by a maintainer and removed Needs Triage This issue needs to be investigated by a maintainer labels Apr 15, 2024
@toddbaert toddbaert changed the title [FEATURE] Change fractional custom op from _percentage-based_ to _relative weighting_. [FEATURE] Change fractional custom op from percentage-based to relative weighting. Apr 15, 2024
@Kavindu-Dodan
Copy link
Contributor

+1 I think this is a good new (non-breaking) feature

@colebaileygit
Copy link
Contributor

Regarding default weights, I think it'd be cleanest to allow specifying strings directly in place of arrays. Though this does become incompatible with the shorthand syntax which relies on string/arr type checking.

Overall I like having the relative weighting syntax available even without the optionality of traffic weights

@agardnerIT
Copy link
Contributor

We did this with Keptn and it was never understood by users. So I'm not in favour as it seems to add complexity where there doesn't need to be any.

What would be nice (and perhaps this achieves this) is the ability to quickly say: 25% to red and the rest (implicitly) 75% to the default:

"defaultValue": "blue",
"fractional": [
          [
            "red",
            25
          ],
]

@colebaileygit
Copy link
Contributor

@agardnerIT How would you support 3-way experiments with 33.3% traffic share, or any non-integer traffic share then?

Should we allow floats or strings in addition to integer?

@beeme1mr
Copy link
Member

I think we should likely stick to integers only. What would be the use case for floats or strings?

I also don't think this complicates the configuration. In my opinion, it would make the configuration simpler and more flexible. It is also non-breaking, so the old way would continue to work.

@bacherfl
Copy link
Contributor

@toddbaert If still available, I would like to work on this one :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants