From 52ceee6d8d0d74e7d03cb9707c7dc9d274a9dc0e Mon Sep 17 00:00:00 2001 From: Parth Suthar Date: Wed, 5 Aug 2026 14:27:28 -0400 Subject: [PATCH] docs: add adr for targeting metadata Signed-off-by: Parth Suthar --- .../targeting-variant-metadata.md | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 docs/architecture-decisions/targeting-variant-metadata.md diff --git a/docs/architecture-decisions/targeting-variant-metadata.md b/docs/architecture-decisions/targeting-variant-metadata.md new file mode 100644 index 000000000..8705b5b05 --- /dev/null +++ b/docs/architecture-decisions/targeting-variant-metadata.md @@ -0,0 +1,188 @@ +--- +# Valid statuses: draft | proposed | rejected | accepted | superseded +status: draft +author: Parth Suthar +created: 2026-07-30 +updated: 2026-07-30 +--- + +# Per-evaluation metadata from targeting rules + +Let a targeting rule return `{ "variant": "", "reason": "", "details": "", "metadata": { ... } }` in place of a plain variant string, so the _branch that fired_ can annotate the evaluation with extra information. Existing string returns are unchanged. + +## Background + +An evaluation today returns `value`, `variant`, `reason`, and `metadata`. The `reason` is a coarse enum (`TARGETING_MATCH`, `DEFAULT`, `STATIC`, …); `metadata` carries only the static blocks defined at flag-set and flag level. Neither answers the debugging question we hit most often: **which branch of the targeting expression fired?** + +For a nested `if` / `and` / `or` tree the resolver just returns the winning variant key. Two rules landing on the same variant are indistinguishable in the response — the only way to know _why_ today is to fetch the flag config and re-run the logic by hand. Encoding the branch identity into the variant key (`"clubs-eu-rollout-a"`) is the workaround, and it pollutes the variant space with debug info consumers then have to parse back out. + +The plumbing to carry metadata already exists on most paths: `AnyValue.Metadata` (`core/pkg/evaluator/ievaluator.go`) is threaded through the evaluator and surfaced via OFREP (single and bulk) and single-resolution gRPC responses as `flagMetadata`. Two paths don't carry it yet — bulk gRPC `ResolveAll` and `RecordEvaluation` telemetry — but wiring those up is separate follow-up work. + +## Proposal + +In `definitions.primitive` in `schemas/json/targeting.json`, the `string` entry (the variant-key return) becomes a choice of the plain string or the tagged object — the other `primitive` entries (`null`, `boolean`, `number`, `array`) are untouched: + +```json +{ + "description": "When returned from rules, strings are used as keys to retrieve the associated value from the \"variants\" object. Be sure that the returned string is present as a key in the variants! As of , an object of the form { \"variant\": ..., \"reason\": ..., \"details\": ..., \"metadata\": ... } may be used instead, to attach a rule id, a free-text explanation, and arbitrary metadata to the branch that fired.", + "oneOf": [ + { "type": "string" }, + { + "type": "object", + "required": ["variant"], + "additionalProperties": false, + "properties": { + "variant": { "type": "string" }, + "reason": { "type": "string" }, + "details": { "type": "string" }, + "metadata": { + "$ref": "https://flagd.dev/schema/v0/flags.json#/definitions/metadata" + } + } + } + ] +} +``` + +`reason` and `details` are optional plain strings, a short rule identifier and a longer free-text explanation. They're first-class schema-validated properties rather than an ad-hoc convention in freeform `metadata`, so consumers get stable key names instead of every config author inventing their own (`rule`, `ruleId`, `why`, …). The `metadata` property reuses the existing definition, so its values stay restricted to `string | number | boolean`, matching flag and flag-set metadata. + +In `evaluateVariant` (`core/pkg/evaluator/json.go`), replace the current string-strip of the JsonLogic result with a typed decode: + +- string → variant key, as today +- object with `variant` field → use it as the variant key; write `reason`/`details` (if present) into the returned metadata, then shallow-merge the rule's `metadata` object on top. Top-level `reason`/`details` win if the rule's `metadata` also sets keys of those names. +- anything else → `PARSE_ERROR`, same as an unrecognized return today + +Merge precedence, lowest → highest, so more specific wins: flag-set metadata → flag metadata → **rule-returned metadata**. + +One wrinkle: the Go JsonLogic engine (`github.com/diegoholiveira/jsonlogic`) treats a returned map as a data literal only when it has **more than one key** (`apply()` in `jsonlogic.go`); a single-key map is looked up as an operator and panics if unregistered. Register `variant` as a passthrough JsonLogic operator — same way `fractional`, `starts_with`, `ends_with`, and `semver` are already registered — so all shapes reach the typed decode instead of panicking. + +### Example — chained `if` + +Each branch tags the evaluation with the rule that fired: + +```json +{ + "acceptable-feature-stability": { + "state": "ENABLED", + "defaultVariant": "ga", + "variants": { "alpha": "alpha", "beta": "beta", "ga": "ga" }, + "targeting": { + "if": [ + { "===": [{ "var": "customerId" }, "customer-A"] }, + { + "variant": "alpha", + "reason": "customer-A-allowlist", + "details": "explicit allowlist for enterprise pilot" + }, + { "in": [{ "var": "customerId" }, ["customer-B1", "customer-B2"]] }, + { "variant": "beta", "reason": "beta-cohort" }, + { "variant": "ga", "reason": "ga-default" } + ] + } + } +} +``` + +### Example — nested `if` with a fractional split + +`fractional` returns a plain string (`"on"` or `"off"`), and both are truthy under JsonLogic — so it can't be used directly as an `if` condition; wrapping it in `==` against a target bucket label makes it a real boolean: + +```json +{ + "targeting": { + "if": [ + { "==": [{ "var": "locale" }, "en-US"] }, + { + "if": [ + { + "==": [ + { + "fractional": [ + { "var": "targetingKey" }, + ["on", 10], + ["off", 90] + ] + }, + "on" + ] + }, + { "variant": "on", "reason": "us-10pct-rollout" }, + { "variant": "off", "reason": "us-holdback" } + ] + }, + { "variant": "off", "reason": "non-us-off" } + ] + } +} +``` + +The 90% US holdback and the non-US off case both serve `off` but carry different `reason` values, so they stay distinguishable in telemetry. + +### Narrowing the coarse `reason` enum + +flagd's top-level `reason` enum today (`TARGETING_MATCH`, `DEFAULT`, `STATIC`, `DISABLED`, `ERROR`, `FALLBACK`) says _what class_ of evaluation happened but not _which specific path_ inside it — every rule-driven result collapses to `TARGETING_MATCH`. A rule-scoped `reason` + `details` sub-classifies that. The schema doesn't fix an enum, so operators pick whatever taxonomy fits their fleet. + +Note that the OpenFeature spec ([evaluation details, requirement 6.1](https://openfeature.dev/specification/sections/evaluation-context)) types `reason` as a free-form string — the values in the spec (`TARGETING_MATCH`, `SPLIT`, `DEFAULT`, …) are _recommended_, not exhaustive, and providers are explicitly allowed to emit their own. That means option 2 in the open questions below — letting a branch-scoped `reason` override the top-level `reason` — is spec-legal without any SDK contract change; SDK hooks and telemetry sinks already treat the field as opaque. + +The user facing reason and mechanism does not match what the falgd engine is evaluating for differnet variants of `SPLIT`. Today split covers both fractional and gradual rollout. Adding a reason override for from the engine result allows for an easy win in this case. Also there are differnet type of `OVERRIDES` which can benefit from this. + +```json +{ "variant": "on", "reason": "SPLIT_GRADUAL", "details": "5%→50% ramp, week 3" } +{ "variant": "on", "reason": "SPLIT_STEPPED", "details": "stage 2 of 4, cohort=eu-west" } +{ "variant": "on", "reason": "SPLIT_RANDOM", "details": "50/50 A/B, salt=exp-4231" } +``` + +Seeing these reason vs `SPLIT` allows for a much richer telemetry signals + +Other patterns the free-form field unlocks inside a single `TARGETING_MATCH`: + +- **Split vs. allowlist under the same top-level reason** — two paths reach the same variant, telemetry can tell them apart: + + ```json + { "variant": "on", "reason": "SPLIT", "details": "10% cohort bucketed by email" } + { "variant": "on", "reason": "ALLOWLIST", "details": "customerId in enterprise-pilot" } + ``` + +- **Kill switch / override tagged distinctly** — a branch short-circuiting an incident stays visible in traces: + + ```json + { + "variant": "off", + "reason": "OVERRIDE", + "details": "incident-4231 kill switch" + } + ``` + +- **Audience-segment attribution** — which specific condition inside a large predicate fired: + + ```json + { + "variant": "beta", + "reason": "AUDIENCE_MATCH", + "details": "country=US AND app_version>=2.0" + } + ``` + +- **Opt-in path** — variant served because the user opted in explicitly, distinguished from a targeting match: + + ```json + { "variant": "on", "reason": "OVERRIDE" } + ``` + +## Consequences + +- **Good** — direct answer to "which branch fired" without overloading the variant key; no wire-format changes. `flagMetadata` is already emitted on OpenFeature evaluation events and picked up by SDK hooks (OpenTelemetry, logging, custom exporters), so anything reading it — OTel spans, A/B dashboards, debug logs — picks up rule-level attribution automatically. +- **Good** — backwards compatible for existing configs: string returns are byte-for-byte unchanged on an upgraded flagd binary. +- **Bad** — not forward compatible: a config using the new object-return shape, evaluated by an older flagd binary, fails (the returned object doesn't quote-strip into a valid variant key). No schema/feature version negotiation exists today, so rollouts must upgrade flagd before configs start using the new shape. Needs an explicit migration note. +- **Bad** — in-process implementations (Java, JS, Kotlin, Python, .NET) each need the same object-return handling to stay conformant. Covered the usual way — a new suite in [flagd-testbed](https://github.com/open-feature/flagd-testbed). +- **Bad** — automatic telemetry attribution requires the separate `RecordEvaluation` work called out in Background; without it, rule ids ride `flagMetadata` for SDK-side hooks but don't reach flagd's own OTel metrics. + +## Open questions + +- **Fractional buckets.** `fractional` returns a variant string directly, so it can't tag the picked bucket with metadata without a separate operator extension (e.g. an optional third element per weight tuple). Proposal: defer — ship the base object-return shape first. +- **JsonLogic operator registration for `variant`.** Registering `variant` as a passthrough operator is a workaround for a dependency quirk, not a documented contract of `github.com/diegoholiveira/jsonlogic`. Pin the behavior with an integration test so a future engine swap or upgrade doesn't silently regress single-key tagged-object returns. +- **Metadata size.** No metadata field is capped today; consistency says leave uncapped, but a soft warning at config load is cheap if reviewers want it. +- **`reason` naming overlap and semantics.** The tagged object's `reason` property and the evaluation response's top-level `reason` enum share a name. Three options: + 1. **Ride in `flagMetadata`** — rule-emitted `reason`/`details` merged into the response `metadata` map; top-level `reason` stays `TARGETING_MATCH`. Simple, no wire-format change, but two fields share a name. + 2. **Narrow the top-level `reason`** — rule-emitted `reason` overrides the response's top-level `reason` when set (e.g. `SPLIT`, `OVERRIDE`, `OPT_IN` as first-class values), with `details` alongside. Cleaner semantically, but expands the SDK contract and needs an OpenFeature-side conversation. + 3. **Rename the rule-emitted field** (e.g. `ruleId`) — sidesteps the collision entirely if option 2 is too invasive.