|
| 1 | +--- |
| 2 | +# Valid statuses: draft | proposed | rejected | accepted | superseded |
| 3 | +status: draft |
| 4 | +author: Parth Suthar (@suthar26) |
| 5 | +created: 2026-04-01 |
| 6 | +--- |
| 7 | +# Treat Disabled Flag Evaluation as Successful with Reason DISABLED |
| 8 | + |
| 9 | +This ADR proposes changing flagd's handling of disabled flags from returning an error (`reason=ERROR`, `errorCode=FLAG_DISABLED`) to returning a successful evaluation with `reason=DISABLED` and the flag's `defaultVariant` value. A flag that does not exist in a flag set should remain a `FLAG_NOT_FOUND` error. |
| 10 | +This aligns flagd with the [OpenFeature specification's resolution reasons](https://openfeature.dev/specification/types/#resolution-reason), which defines `DISABLED` as a valid resolution reason for successful evaluations. |
| 11 | + |
| 12 | +## Background |
| 13 | + |
| 14 | +flagd currently treats the evaluation of a disabled flag as an error. When a flag exists in the store but has `state: DISABLED`, the evaluator returns `reason=ERROR` with `errorCode=FLAG_DISABLED`. This error propagates through every surface — gRPC, OFREP, and in-process providers — resulting in the caller receiving an error response rather than a resolved value. |
| 15 | + |
| 16 | +This is problematic for several reasons: |
| 17 | + |
| 18 | +1. **Spec misalignment**: The [OpenFeature specification](https://openfeature.dev/specification/types/#resolution-reason) explicitly defines `DISABLED` as a resolution reason with the description: *"The resolved value was the result of the flag being disabled in the management system."* This implies a successful evaluation that communicates the flag's disabled state, not an error. |
| 19 | +2. **OFREP masks the disabled state**: The OFREP response handler in `core/pkg/service/ofrep/models.go` rewrites `FLAG_DISABLED` to `FLAG_NOT_FOUND` in the structured error response (while only preserving the "is disabled" distinction in the free-text `errorDetails` string). This means OFREP clients cannot programmatically distinguish between a flag that doesn't exist and one that was intentionally disabled. |
| 20 | +3. **Conflation of "missing" and "disabled"**: gRPC v1 maps both `FLAG_NOT_FOUND` and `FLAG_DISABLED` to `connect.CodeNotFound`. These are semantically different situations: a missing flag is a configuration or deployment error, while a disabled flag is an intentional operational decision (incident remediation, environment-specific rollout, not-yet-ready feature). |
| 21 | +4. **Loss of observability**: When disabled flags are treated as errors, they pollute error metrics and alerting. Operators who disable a flag for legitimate reasons (ongoing incident remediation, feature not ready for an environment) see false error signals. Conversely, if they suppress these errors, they lose visibility into flag state entirely. A successful evaluation with `reason=DISABLED` would give operators a clean signal without noise. |
| 22 | +5. **Flag set use cases**: In multi-flag-set deployments, a flag may exist in a shared definition but be disabled in certain flag sets (e.g., disabled for `staging` but enabled for `production`). Treating this as an error forces the application into error-handling paths when the flag is simply not active — a normal operational state, not an exceptional one. |
| 23 | + |
| 24 | +Related context: |
| 25 | + |
| 26 | +- [OpenFeature Specification - Resolution Reasons](https://openfeature.dev/specification/types/#resolution-reason) |
| 27 | +- [flagd Flag Definitions Reference](https://flagd.dev/reference/flag-definitions/) |
| 28 | +- [ADR: Support Explicit Code Default Values](./support-code-default.md) — establishes the pattern of returning `defaultVariant` with appropriate reason codes |
| 29 | + |
| 30 | +## Requirements |
| 31 | + |
| 32 | +- Evaluating a disabled flag must return a successful response with `reason=DISABLED` across all surfaces (gRPC v1, gRPC v2, OFREP, in-process) |
| 33 | +- The resolved value for a disabled flag must be the flag's configured `defaultVariant` |
| 34 | +- If the flag's `defaultVariant` is `null` (per the code-default ADR), the disabled response must defer to code defaults using the same field-omission pattern, but with `reason=DISABLED` instead of `reason=DEFAULT` |
| 35 | +- Evaluating a flag key that does not exist in the store or flag set must remain a `FLAG_NOT_FOUND` error |
| 36 | +- `reason=DISABLED` must be distinct from all other reasons (`STATIC`, `DEFAULT`, `SPLIT`, `TARGETING_MATCH`, `ERROR`) and must not trigger error-handling paths in providers or SDKs |
| 37 | +- Bulk evaluation (`ResolveAll`) must include disabled flags in the response with `reason=DISABLED`, rather than silently omitting them |
| 38 | +- Telemetry and metrics must record disabled flag evaluations as successful (non-error) with the `DISABLED` reason |
| 39 | +- Existing flag configurations must continue to work without modification (backward compatible at the configuration level) |
| 40 | + |
| 41 | +## Considered Options |
| 42 | + |
| 43 | +- **Option 1: Successful evaluation with `reason=DISABLED` returning `defaultVariant`** — Disabled flags evaluate successfully, returning the `defaultVariant` value and `reason=DISABLED` |
| 44 | +- **Option 2: Return a successful evaluation with** `reason=DEFAULT` — Treat disabled flags as if they had no targeting, collapsing the disabled state into the default reason |
| 45 | +- **Option 3: Status quo** — Keep the current error behavior and document it as intentional divergence from the OpenFeature spec |
| 46 | + |
| 47 | +## Proposal |
| 48 | + |
| 49 | +We propose **Option 1: Successful evaluation with `reason=DISABLED` returning `defaultVariant`**. |
| 50 | + |
| 51 | +When a flag exists in the store but has `state: DISABLED`, the evaluator should return a successful evaluation with the following properties: |
| 52 | + |
| 53 | +- **value**: The flag's `defaultVariant` value (from the flag configuration) |
| 54 | +- **variant**: The flag's `defaultVariant` key |
| 55 | +- **reason**: `DISABLED` |
| 56 | +- **error**: `nil` (no error) |
| 57 | +- **metadata**: The merged flag set + flag metadata (consistent with current behavior for successful evaluations) |
| 58 | + |
| 59 | +This aligns with the OpenFeature specification's definition of `DISABLED` as a resolution reason and leverages the existing but unused `DisabledReason` constant already defined in flagd. |
| 60 | + |
| 61 | +### Interaction with other resolution reasons |
| 62 | + |
| 63 | +The [OpenFeature specification](https://openfeature.dev/specification/types/#resolution-reason) defines several resolution reasons. Here is how `DISABLED` interacts with each: |
| 64 | + |
| 65 | +| Reason | Current flagd usage | Interaction with DISABLED | |
| 66 | +| ----------------- | ---------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | |
| 67 | +| `STATIC` | Returned when a flag has no targeting rules and resolves to `defaultVariant` | When disabled, `DISABLED` takes precedence. The flag's `defaultVariant` is still returned as the value, but the reason is `DISABLED`, not `STATIC`. This distinction matters: `STATIC` tells caching providers the value is safe to cache indefinitely, while `DISABLED` signals the value may change when the flag is re-enabled. | |
| 68 | +| `DEFAULT` | Returned when targeting rules exist but evaluate to the `defaultVariant` | When disabled, `DISABLED` takes precedence. Targeting rules are not evaluated at all for disabled flags, so `DEFAULT` (which implies targeting was attempted) is not appropriate. | |
| 69 | +| `SPLIT` | Defined in flagd but used for pseudorandom assignment (fractional targeting) | When disabled, `DISABLED` takes precedence. Fractional targeting rules are not evaluated for disabled flags. | |
| 70 | +| `TARGETING_MATCH` | Returned when targeting rules match and select a specific variant | Not applicable. Targeting rules are never evaluated for disabled flags. | |
| 71 | +| `ERROR` | Currently returned for disabled flags (this is what we are changing) | `DISABLED` replaces `ERROR` for this case. `ERROR` remains the reason for genuine errors (parse errors, type mismatches, etc.). | |
| 72 | + |
| 73 | +**Key principle**: `DISABLED` is a terminal reason. When a flag is disabled, no targeting evaluation occurs, so reasons that describe targeting outcomes (`STATIC`, `DEFAULT`, `SPLIT`, `TARGETING_MATCH`) never apply. The evaluation short-circuits to `reason=DISABLED` with the `defaultVariant` value. |
| 74 | + |
| 75 | +### Interaction with code defaults (`defaultVariant: null`) |
| 76 | + |
| 77 | +Per the [code-default ADR](./support-code-default.md), when `defaultVariant` is `null`, the server omits the value and variant fields to signal code-default deferral. This pattern applies to disabled flags as well: |
| 78 | + |
| 79 | +- `defaultVariant` is a string → return the variant value with `reason=DISABLED` |
| 80 | +- `defaultVariant` is `null` → omit value/variant fields, return `reason=DISABLED` |
| 81 | + |
| 82 | +The only difference from a normal code-default response is the reason field: `DISABLED` instead of `DEFAULT`. |
| 83 | + |
| 84 | +### API changes |
| 85 | + |
| 86 | +**Evaluator core — `evaluateVariant`** (`core/pkg/evaluator/json.go`): |
| 87 | + |
| 88 | +The `evaluateVariant` function changes from returning an error to returning a successful result. Since `flag.DefaultVariant` is already `""` when no default variant is configured, no additional branch is needed: |
| 89 | + |
| 90 | +```go |
| 91 | +// Before |
| 92 | +if flag.State == Disabled { |
| 93 | + return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagDisabledErrorCode) |
| 94 | +} |
| 95 | + |
| 96 | +// After |
| 97 | +if flag.State == Disabled { |
| 98 | + return flag.DefaultVariant, flag.Variants, model.DisabledReason, metadata, nil |
| 99 | +} |
| 100 | +``` |
| 101 | + |
| 102 | +**Evaluator core — `resolve[T]`** (`core/pkg/evaluator/json.go`): |
| 103 | + |
| 104 | +The generic `resolve[T]` function currently only short-circuits for `FallbackReason` when the variant is empty. For any other reason it attempts a variant map lookup, which would produce a `TYPE_MISMATCH` error for disabled flags that defer to code defaults (`defaultVariant: null`). `resolve[T]` must treat `DisabledReason` the same way when the variant is empty: |
| 105 | + |
| 106 | +```go |
| 107 | +// Before |
| 108 | +if reason == model.FallbackReason { |
| 109 | + var zero T |
| 110 | + return zero, variant, model.FallbackReason, metadata, nil |
| 111 | +} |
| 112 | + |
| 113 | +// After |
| 114 | +if reason == model.FallbackReason || reason == model.DisabledReason { |
| 115 | + if variant == "" { |
| 116 | + var zero T |
| 117 | + return zero, variant, reason, metadata, nil |
| 118 | + } |
| 119 | +} |
| 120 | +``` |
| 121 | + |
| 122 | +**Bulk evaluation — `ResolveAllValues`** (`core/pkg/evaluator/json.go`): |
| 123 | + |
| 124 | +Disabled flags are no longer skipped. They are evaluated and included in the response. Additionally, when `defaultVariant` is `null`, the type switch on `flag.Variants[flag.DefaultVariant]` hits the `default` case. For gRPC v1 requests (where `ProtoVersionKey` is set), the current `default` branch skips unknown types via `continue`. This must be adjusted so disabled flags are always included regardless of proto version: |
| 125 | + |
| 126 | +```go |
| 127 | +// Before |
| 128 | +if flag.State == Disabled { |
| 129 | + continue |
| 130 | +} |
| 131 | + |
| 132 | +// After: remove the skip — disabled flags flow through normal evaluation |
| 133 | +// and will be returned with reason=DISABLED |
| 134 | +``` |
| 135 | + |
| 136 | +```go |
| 137 | +// Before (default branch of type switch) |
| 138 | +default: |
| 139 | + if ctx.Value(ProtoVersionKey) == nil { |
| 140 | + value, variant, reason, metadata, err = resolve[interface{}](...) |
| 141 | + } else { |
| 142 | + continue |
| 143 | + } |
| 144 | + |
| 145 | +// After: disabled flags must not be skipped even for old proto versions |
| 146 | +default: |
| 147 | + if ctx.Value(ProtoVersionKey) == nil { |
| 148 | + value, variant, reason, metadata, err = resolve[interface{}](...) |
| 149 | + } else if flag.State == Disabled { |
| 150 | + value, variant, reason, metadata, err = resolve[interface{}](...) |
| 151 | + } else { |
| 152 | + continue |
| 153 | + } |
| 154 | +``` |
| 155 | + |
| 156 | +**OFREP success mapping** (`core/pkg/service/ofrep/models.go`): |
| 157 | + |
| 158 | +The `SuccessResponseFrom` function currently rewrites `FallbackReason` to `DefaultReason` and omits value/variant fields to signal code-default deferral. When a disabled flag defers to code defaults, the same field-omission pattern must apply, but the reason must remain `DISABLED` (not be rewritten to `DEFAULT`): |
| 159 | + |
| 160 | +```go |
| 161 | +// Before |
| 162 | +if result.Reason == model.FallbackReason { |
| 163 | + return EvaluationSuccess{ |
| 164 | + Value: nil, |
| 165 | + Key: result.FlagKey, |
| 166 | + Reason: model.DefaultReason, |
| 167 | + Variant: "", |
| 168 | + Metadata: result.Metadata, |
| 169 | + } |
| 170 | +} |
| 171 | + |
| 172 | +// After: handle disabled flags deferring to code defaults |
| 173 | +if result.Reason == model.FallbackReason { |
| 174 | + return EvaluationSuccess{ |
| 175 | + Value: nil, |
| 176 | + Key: result.FlagKey, |
| 177 | + Reason: model.DefaultReason, |
| 178 | + Variant: "", |
| 179 | + Metadata: result.Metadata, |
| 180 | + } |
| 181 | +} |
| 182 | +if result.Reason == model.DisabledReason && result.Variant == "" { |
| 183 | + return EvaluationSuccess{ |
| 184 | + Value: nil, |
| 185 | + Key: result.FlagKey, |
| 186 | + Reason: model.DisabledReason, |
| 187 | + Variant: "", |
| 188 | + Metadata: result.Metadata, |
| 189 | + } |
| 190 | +} |
| 191 | +``` |
| 192 | + |
| 193 | +**gRPC response** (single flag evaluation): |
| 194 | + |
| 195 | +```json |
| 196 | +{ |
| 197 | + "value": false, |
| 198 | + "variant": "off", |
| 199 | + "reason": "DISABLED", |
| 200 | + "metadata": { |
| 201 | + "flagSetId": "my-app", |
| 202 | + "scope": "production" |
| 203 | + } |
| 204 | +} |
| 205 | +``` |
| 206 | + |
| 207 | +**OFREP response** (single flag evaluation): |
| 208 | + |
| 209 | +```json |
| 210 | +{ |
| 211 | + "key": "my-feature", |
| 212 | + "value": false, |
| 213 | + "variant": "off", |
| 214 | + "reason": "DISABLED", |
| 215 | + "metadata": { |
| 216 | + "flagSetId": "my-app" |
| 217 | + } |
| 218 | +} |
| 219 | +``` |
| 220 | + |
| 221 | +**OFREP bulk response** (disabled flag now included): |
| 222 | + |
| 223 | +```json |
| 224 | +{ |
| 225 | + "flags": [ |
| 226 | + { |
| 227 | + "key": "my-feature", |
| 228 | + "value": false, |
| 229 | + "variant": "off", |
| 230 | + "reason": "DISABLED", |
| 231 | + "metadata": {} |
| 232 | + }, |
| 233 | + { |
| 234 | + "key": "active-feature", |
| 235 | + "value": true, |
| 236 | + "variant": "on", |
| 237 | + "reason": "STATIC", |
| 238 | + "metadata": {} |
| 239 | + } |
| 240 | + ] |
| 241 | +} |
| 242 | +``` |
| 243 | + |
| 244 | +### Consequences |
| 245 | + |
| 246 | +- Good, because it aligns flagd with the OpenFeature specification's definition of `DISABLED` as a resolution reason |
| 247 | +- Good, because it eliminates the OFREP bug where `FLAG_DISABLED` is silently masked as `FLAG_NOT_FOUND` |
| 248 | +- Good, because operators get clean observability: disabled flags appear as successful evaluations with a distinct reason, not polluting error metrics |
| 249 | +- Good, because it enables flag-set-based workflows where disabling a flag in one environment is a normal operational state |
| 250 | +- Good, because the existing `DisabledReason` constant is finally used as designed |
| 251 | +- Good, because it provides visibility into disabled flags in bulk evaluation responses, rather than silently omitting them |
| 252 | +- Good, because applications can distinguish between "flag doesn't exist" (a real problem) and "flag is disabled" (an intentional state) |
| 253 | +- Bad, because it is a breaking change for clients that rely on `FLAG_DISABLED` error responses for control flow, alerting, or metrics |
| 254 | +- Bad, because including disabled flags in `ResolveAll` responses increases payload size for flag sets with many disabled flags |
| 255 | +- Bad, because it requires coordinated updates across flagd core, all gRPC/OFREP surfaces, providers, and the testbed |
| 256 | +- Neutral, because the `FlagDisabledErrorCode` constant and related error-handling code can be removed (code simplification) |
| 257 | + |
| 258 | +### Timeline |
| 259 | + |
| 260 | +1. Update `evaluateVariant` in `core/pkg/evaluator/json.go` to return `reason=DISABLED` with `defaultVariant` instead of an error |
| 261 | +2. Update `resolve[T]` in `core/pkg/evaluator/json.go` to handle `DisabledReason` with empty variants (avoids `TYPE_MISMATCH` when disabled flags defer to code defaults) |
| 262 | +3. Remove the disabled-flag skip in `ResolveAllValues` and update the `default` branch of the type switch to include disabled flags for gRPC v1 requests |
| 263 | +4. Update `SuccessResponseFrom` in `core/pkg/service/ofrep/models.go` to preserve `reason=DISABLED` (with field omission) when a disabled flag defers to code defaults |
| 264 | +5. Remove `FlagDisabledErrorCode` handling from `errFormat`, `errFormatV2`, and `EvaluationErrorResponseFrom` |
| 265 | +6. Update gRPC v1 service layer (`flag_evaluator_v1.go`) to handle nil values in `ResolveAll` responses for disabled flags deferring to code defaults |
| 266 | +7. Update flagd-testbed with test cases for disabled flag evaluation across all surfaces |
| 267 | +8. Update OpenFeature providers to recognize `DISABLED` as a non-error, non-cacheable reason |
| 268 | +9. Update provider documentation and migration guides |
| 269 | + |
| 270 | +### Open questions |
| 271 | + |
| 272 | +- Should bulk evaluation include an option to exclude disabled flags for clients that prefer the current behavior (smaller payloads)? |
| 273 | +- How should existing dashboards and alerts that key on `FLAG_DISABLED` errors be migrated? Should we provide a deprecation period where both behaviors are available? |
| 274 | +- Does this change require a new flagd major version, or can it be introduced in a minor version with appropriate documentation given the spec alignment argument? |
| 275 | +- Should the `FlagDisabledErrorCode` constant be retained (but unused) for a deprecation period, or removed immediately? |
| 276 | +- How should in-process providers handle the transition? They evaluate locally and would need to be updated to return `DISABLED` reason instead of throwing an error. |
| 277 | + |
| 278 | +## More Information |
| 279 | + |
| 280 | +- [OpenFeature Specification - Resolution Reasons](https://openfeature.dev/specification/types/#resolution-reason) |
| 281 | +- [OpenFeature Specification - Error Codes](https://openfeature.dev/specification/types/#error-code) — notably, `FLAG_DISABLED` is not in the spec's error code list |
| 282 | +- [flagd Flag Definitions Reference](https://flagd.dev/reference/flag-definitions/) |
| 283 | +- [ADR: Support Explicit Code Default Values](./support-code-default.md) |
| 284 | +- [flagd Testbed](https://github.com/open-feature/flagd-testbed) |
0 commit comments