Skip to content

docs: improve fractional non-string adr #1963

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.

Sign up for GitHub

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

Merged
toddbaert merged 1 commit into from
May 29, 2026
Merged

docs: improve fractional non-string adr #1963

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 27 additions & 7 deletions docs/architecture-decisions/fractional-non-string-rand-units.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ As such, the encodings for the following types as first argument (either as lite

**array / sequence** will be explicitly not supported as the first argument in fractional so it's possible to distinguish between hashing input and variant bucket. Nevertheless, it can be a part of object type and its encoding needs to be standardized as well.

**null** will be explicitly not supported as the first argument, as multiple provider implementations return `null` when an error occurs during evaluation, and JSON Logic returns `null` for a missing key in the evaluation context. Rejecting `null` prevents silent errors in common use cases.

## Non-requirements

* This change does not need to be backward-compatible.
Expand All @@ -86,10 +88,11 @@ We will modify the evaluation logic for the `fractional` operator.

When inspecting the first element of the `fractional` array:

1. If the first element in `fractional` evaluates to a non-array type then deterministically encode it to a well defined byte array and hash the bytes.
2. Otherwise, if `targetingKey` is a string, build a 2-elements array of `flagKey` and `targetingKey`, deterministically encode that and hash (**NOTE:** This is different than string concatenation used today).
3. Otherwise, if `targetingKey` is non-string, report an error and return nil (as this breaks the [OpenFeature spec](https://openfeature.dev/specification/glossary/#targeting-key)).
4. Otherwise, if `targetingKey` is missing, report an error and return nil
1. If the first element in `fractional` evaluates to `null`, we report an error and return `nil`.
2. If the first element in `fractional` evaluates to a non-array type then deterministically encode it to a well defined byte array and hash the bytes.
3. Otherwise, if `targetingKey` is a string, build a 2-elements array of `flagKey` and `targetingKey`, deterministically encode that and hash (**NOTE:** This is different than string concatenation used today).

@toddbaert toddbaert May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to maintain the current cat behavior?

If not, that's okay but we did get some complains about the re-random assignment this caused in the past. Not a huge deal <1.0.

@m-olko m-olko May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior is copied from original ADR, I just modified the numbering, due to adding additional check at the beginning. I think we can leave the current cat behavior, but it still will introduce rebucketing due to the fact, that cbor encoding that we use for fractional before passing to murmur hash, stores the string type info and its length. So even if we leave current behavior as is, the rebucket will occur.

The only difference would be between evaluation of explicit cat in the code, and leveraging this syntactic sugar:

"fractional-flag-shorthand": {
      "state": "ENABLED",
      "variants": {
        "heads": "heads",
        "tails": "tails",
        "draw": "draw"
      },
      "defaultVariant": "draw",
      "targeting": {
        "fractional": [
          ["heads", 1],
          ["tails", 1]
        ]
      }
    },
"fractional-flag-explicit-cat": {
      "state": "ENABLED",
      "variants": {
        "heads": "heads",
        "tails": "tails",
        "draw": "draw"
      },
      "defaultVariant": "draw",
      "targeting": {
        "fractional": [
          {"cat": [{"var": "$flag.flagKey"}, {"var": "targetingKey"}]},
          ["heads", 1],
          ["tails", 1]
        ]
      }
    },

In current proposal, they would yield different bucketings, while staying with cat, would cause them to be identical.

4. Otherwise, if `targetingKey` is non-string, report an error and return `nil` (as this breaks the [OpenFeature spec](https://openfeature.dev/specification/glossary/#targeting-key)).
5. Otherwise, if `targetingKey` is missing, report an error and return `nil`

```json
// Will use the new logic
Expand Down Expand Up @@ -127,23 +130,40 @@ When inspecting the first element of the `fractional` array:
To meet requirement (2) [RFC 8949 Concise Binary Object Representation (CBOR)](https://www.rfc-editor.org/rfc/rfc8949.html) will be used to decide on byte encodings.

* `boolean` is major type 7
* `null` is major type 7
* `string` is major type 3
Comment thread
m-olko marked this conversation as resolved.
* `integer`:
* `unsigned integer` is major type 0
* `negative integer` is major type 1
* `float` is major type 7
* `map` (object, structure, dict) is major type 5
* `array` (list, sequence) is major type 4
* `datetime` is converted to POSIX epoch time (including fractional seconds for sub-second precision) and CBOR Tag 1 is used

**NOTE: As JSONLogic doesn’t have any datetime type, currently we don’t leverage CBOR Tag 1. Any datetime type used within provider implementation and passed to the fractional operator causes undefined behavior. If a user wants to manage datetime, they can do it by leveraging POSIX epoch encoded as integer value, or as ISO 8601 standard encoded as string.**
Comment thread
m-olko marked this conversation as resolved.

**ATTENTION: When encoding strings, CBOR appends the size of the encoding in first bytes. As such, even though the actual encoding of the string is still UTF-8, the resulting byte array will differ from raw UTF-8 encoding. As such, after this change, all hashes will change, which will result in rebucketing.**

Additionally, it is required to use [4.2.1. Core Deterministic Encoding Requirements](https://www.rfc-editor.org/rfc/rfc8949.html#section-4.2.1) (which includes Preferred Serizalization), to ensure:
However, to reach full cross-language consistency we need to fulfill those additional requirements:

* **Number Normalization (Integer vs. Float):**
JSON parsers natively lack strict differentiation between integers and floats (e.g., `1` vs `1.0`). To align with CBOR's distinct major types (Type 0/1 for integers, Type 7 for floats) and Section 6.2 of RFC 8949, all providers must implement a normalization step prior to encoding.
To prevent overflow errors in strongly-typed languages (e.g. Go) and inconsistent BigInt tagging in languages with arbitrary-precision integers (e.g. Python), the number normalization is restricted to the range $[-2^{63}, 2^{64}-1]$ (covering both signed and unsigned 64-bit integers):

1. If a numeric value has no fractional part (e.g., `val == math.Trunc(val)` in Go, or `val.is_integer()` in Python), the provider must attempt to cast it to a signed (if <0) or unsigned (if >=0) integer before encoding.
2. If a numeric value has fractional part, or if it falls outside the range $[-2^{63}, 2^{64}-1]$ (e.g., `1.0e+176`), it **must not** be normalized to an integer. It must be encoded as a float (Major Type 7).

@toddbaert toddbaert May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to apply this to anything larger than the Javascript MAX_SAFE_INTEGER (2^53 - 1). Things could get messy, especially in JS, otherwise.

Even Gson I think out of the box, would yield double here, IIUC.

If the goal here is just consistency across all languages including JS, I think encoding as float outside the "SAFE" JS range will be easiest to accomplish. We could also perhaps support parsing to bigint in JS, but that seems somewhat heavy for just this purpose.

@m-olko m-olko May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea behind this proposal was to make this adr future proof. As for now, we have problems with int64 in the whole flagd (JavaScript is only one of the problem, I think Go also uses double for all numbers when parsing flag config, and Java is not supporting even >Int32 values), and from my understanding we aren't officially supporting it anywhere. I have seen some discussions on creating and formalizing that support and thought that we could handle it from beginning here (In JavaScript, we can use BigInt for comparison regardless of number type passed to the fractional evaluator). Thanks to that, when we finally introduce support for full int64 range, we won't have to create breaking changes when trying to support int64 in fractional operator (since it would require again adr update and it would break encodings for numbers from 2^53 to 2^64).

I agree that supporting int64 is definitely out of the scope for this task, but as this range changes only the check in number normalization, and won't modify behavior of parsing, evaluation etc, and we can treat those two tasks as completely separate.

But it's definitely not a dealbreaker for me, so I can modify that range to handle only float-safe values.

@toddbaert toddbaert May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea behind this proposal was to make this adr future proof.

I agree that supporting int64 is definitely out of the scope for this task, but as this range changes only the check in number normalization, and won't modify behavior of parsing, evaluation etc, and we can treat those two tasks as completely separate.

That makes sense to me - to be clear - you are proposing interpreting any number passed here in the Javascript implementation as a BigInt in terms of generating the bucketing value? If so, I have no other comments, and I will approve.

@m-olko m-olko May 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to interpret that number. We can just compare it with BigInt which from my research have all necessary implementation to handle that comparison safely and reliably with any number type. As we only need to check if the value fits into the range, the comparison operation is the only thing necessary for implementation of this adr.


**NOTE: Both -0.0 and 0.0 float values should be mapped to unsigned integer value 0.**

**NOTE: As NaN and +/- infinity are not supported by JSON, operations on them are undefined behavior, even in languages that may support them. Using those values in live applications is discouraged.**

* **CBOR Deterministic Encoding:**

It is required to use [4.2.1. Core Deterministic Encoding Requirements](https://www.rfc-editor.org/rfc/rfc8949.html#section-4.2.1) (which includes Preferred Serialization), to ensure:

1. **Map Key Ordering**: Implementations must strictly adhere to the requirement that keys in maps (objects/structures) must be sorted using bytewise lexicographic order of their deterministic encodings.
2. **Preferred Serialization (Numbers)**: CBOR mandates using the shortest possible encoding. Providers must ensure consistency, especially between integer and float representations, and across different precisions. For example, if a value fits within a 32-bit float, it must be used instead of a 64-bit float, regardless of the native type in the provider's language.

**NOTE: Since flag configurations are parsed from JSON, the maps (objects, structures, dicts) always have strings as keys. That’s why often there is no difference between Core Deterministic Encoding defined in [rfc8949 Section 4.2](https://www.rfc-editor.org/rfc/rfc8949.html#name-deterministically-encoded-c) and Canonical Encoding defined in [rfc7049 Section 3.9](https://www.rfc-editor.org/rfc/rfc7049#section-3.9). In some languages, due to the absence of libraries that can handle the updated standard, it is possible to use the older one. In such cases please add code comments explaining the implementation choice.**

### API changes

There are **no** changes to the flagd JSON schema. The change is purely semantic, affecting the evaluation logic within providers.
Expand Down
Loading