chore(ADR): Create fractional-non-string-rand-units.md

[cupofcat]

This PR

• Proposes Support Non-String Inputs for Fractional Bucketing ADR

Related Issues

#1737

[netlify]

✅ Deploy Preview for polite-licorice-3db33c ready!

Name	Link
🔨 Latest commit	42b34dc
🔍 Latest deploy log	https://app.netlify.com/projects/polite-licorice-3db33c/deploys/692f133dc5b19a0008079949
😎 Deploy Preview	https://deploy-preview-1783--polite-licorice-3db33c.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[gemini-code-assist]

Summary of Changes

Hello @cupofcat, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new Architecture Decision Record (ADR) proposing an enhancement to the fractional operator in flagd. The core purpose is to enable the fractional operator to directly support non-string input values for hashing, such as integers or booleans, without requiring explicit string coercion. This change aims to improve consistency across different provider implementations and simplify flag definitions by allowing direct use of various data types for bucketing.

Highlights

• Fractional Operator Enhancement: Proposes an Architecture Decision Record (ADR) to allow the fractional operator to accept non-string inputs (e.g., integers, booleans) for hashing, eliminating the need for explicit string coercion.
• Improved Hashing Consistency: Addresses current inconsistencies in hashing across different provider implementations (Go, JS) by proposing direct byte representation hashing for non-string inputs, while maintaining backward compatibility for existing string-based inputs.
• No Schema Changes: The proposed change is purely semantic, affecting only the evaluation logic within flagd providers, and introduces no modifications to the existing flagd JSON schema.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces an Architecture Decision Record (ADR) to support non-string inputs for fractional bucketing, which is a valuable enhancement for flagd. The ADR is well-structured and clearly outlines the problem, proposed solution, and consequences. My review focuses on improving the clarity and correctness of the document. I've provided suggestions to fix an invalid JSON example, correct list formatting, and resolve a critical ambiguity in the byte representation of non-string types to ensure cross-platform consistency. A few minor typos have also been identified for correction.

[cupofcat]

I've been thinking about this more and I wonder WDYT about this approach:

We take this pre-1.0 opportunity to introduce a breaking change for how hashing / bucketing works:

• If fractional arguments are all arrays, then we use targetingKey for hashing, similar to today
• Otherwise, the first argument needs to be an object ({})

If the first argument is an object, it's evaluated and the evaluated value is passed to CBOR for marshalling into a consistent byte array. That byte array is passed to MurMur3 for hashing.

If targetingKey is used, a 2 element array is created with the first element being the flag key, and the second element being the targetingKey value. That array is passed to CBOR for marshalling and the resulting byte array is passed to MurMur3 for hashing.

CBOR libraries in each language ensure that the same values get the same byte encoding and murmur3 libraries will ensure that same byte arrays will get the same hash. This works across any type, even for strings.

That way we have langauge-agnostic, stable, and consistent bucketing.

CBOR is specified in an Internet Standard RFC by IETF, which should mean this stays stable for foreseeable future.

[beeme1mr]

We take this pre-1.0 opportunity to introduce a breaking change for how hashing / bucketing works:

• If fractional arguments are all arrays, then we use targetingKey for hashing, similar to today
• Otherwise, the first argument needs to be an object ({})

@cupofcat would you mind showing an example configuration of what you had in mind? When you say object, do you mean referencing a context attribute { "var": "useId" } or something else?

[cupofcat]

We take this pre-1.0 opportunity to introduce a breaking change for how hashing / bucketing works:

• If fractional arguments are all arrays, then we use targetingKey for hashing, similar to today
• Otherwise, the first argument needs to be an object ({})

@cupofcat would you mind showing an example configuration of what you had in mind? When you say object, do you mean referencing a context attribute { "var": "useId" } or something else?

I meant that essentially the first item in fractional is {...} and not a list. It can be { var: ...} or something else. I'm open to other suggestions, we just need some way to detect if we should use targetingKey or evaluate the first argument for hashing input.

I would want to keep it pretty generic though. In theory, a list of elements could also be a valid input to hashing.

Edit: The above did not make sense, sorry.

@beeme1mr I've been thinking about this more and I think we have the following options:

1. Similarly to today we have an optional 1st argument to fractional to provide value to hash for bucketing.
2. We make the first argument mandatory; if someone wants to use targetingKey they need to provide null as first argument
3. We introduce a new custom operator to be used inside fractional
   A. something like cbor-8949-bytes that is guaranteed to return a consistent byte array in every language
   B. something like object that preserves the keys and evaluates the values
   C. we can brainstorm other custom operator with other semantics too

For (1), since the first argument is optional, we need a way to distinguish it from the subsequent ones. Today, this is done based on type (if it casts to a string, it's treated specially). That means the first argument, if the user wants to use it for bucketing, cannot evaluate to an array. If it is an array we cannot distinguish it from the rest of fractional values. We could go further, and look into types inside the array and say it cannot start with [string, number] but overall I think this is a really hacky semantic. We could say, that if it evaluates to a map/dictionary/object then we encode it to bytes and hash that. However, I cannot seem to figure out how to return an object in JSONLogic using existing operators...

(2) is much clearer, but the semantic of the current schema changes (the schema itself can stay the same). We make the first argument mandatory as the bucketing input. If it's null we use the targetingKey. This is a pretty big breaking change, as all the flag configs that rely on targetingKey would break. But, perhaps, this is OK when moving to 1.0.

(3A) Similarly to today, if the first value in fractional array is bytes we hash that. If not, we use the current behavior as is.

"fractional": [
  {
    "cbor-8949-bytes": [{"var" : "$flagd.flagKey"}, {"var" : "some-var"}]
  },
  ["a", 50], ...
]

(3B) Similarly to today, if the first value in fractional array is a map/dict we hash that. If not, we use the current behavior as is.

"fractional": [
  {
    "object": {
      "comment" : "Any keys are valid, we will just translate all of this to bytes (JSONLogic would still evaluate the values)",
      "key1" : {"var" : "$flagd.flagKey"},
      "key2" : {"var" : "some-var"},
      "extra-salt" : "himalayan"
    }
  },
  ["a", 50], ...
]

If none of the above works, perhaps there is also:

4. We introduce a new custom operator replacing fractional. We can call it fractional1 or fractionalWithKey or fractionalBytes or something. We make it officially recommended for flagd 1.0 and we deprecate fractional. We could keep evaluating fractional using current logic with warnings. We could also remove it completely from 1.0, given that this will be a pretty big breaking change anyway for the users.

WDYT?

P.S. What I find complicates this exercise is that today we send the entire contents of targeting to JSONLogic for evaluation. So we cannot really do any parsing on the structure of the object. Our fractional evaluation implementation already receives evaluated values so we don't know what the arguments looked like.

So I guess there is also:

5. Parse either targeting or fractional looking for specific fields to control the behavior of what gets passed to JSONLogic and how.

[cupofcat]

Based on the discussions on Slack I updated this ADR to focus on hashing improvements in the existing fractional only.

[cupofcat]

I updated the ADR to explicitly require CBOR encodings + some other minor improvements based on feedback.

[toddbaert]

I am in favor of this proposal, but I can't help but consider this a bit of a blocker.

@beeme1mr WDYT about this one? If we cant append the flag key, should we remove this in the string case? I'd prefer not to have string as a "special case".

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cupofcat]

I am in favor of this proposal, but I can't help but consider this a bit of a blocker.

@beeme1mr WDYT about this one? If we cant append the flag key, should we remove this in the string case? I'd prefer not to have string as a "special case".

I would not necessarily consider this a special case for strings. It's a result of JSONLogic rather than flagd approach (JSON Logic has "cat" operator for strings but does not have anything like that for non-string types). We can fix that by implementing an additional custom operator.

That way we keep the new approach backwards compatible with old configs (and, tbh, there is no way around this - "cat" is a feature of JSONLogic, not flagd, so we cannot really block it unless we do some hacks).

[toddbaert]

I am in favor of this proposal, but I can't help but consider this a bit of a blocker.
@beeme1mr WDYT about this one? If we cant append the flag key, should we remove this in the string case? I'd prefer not to have string as a "special case".

I would not necessarily consider this a special case for strings. It's a result of JSONLogic rather than flagd approach (JSON Logic has "cat" operator for strings but does not have anything like that for non-string types). We can fix that by implementing an additional custom operator.

That way we keep the new approach backwards compatible with old configs (and, tbh, there is no way around this - "cat" is a feature of JSONLogic, not flagd, so we cannot really block it unless we do some hacks).

I can accept that the "salting" only occurs in the string case, as long as we document it clearly. The main advantage of the "salting" is that it means that users don't end up in the same bucket for every fractional flag... but the nonstring situation is a corner case of a corner case here, so I don't want to block this ADR just for this.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud