Skip to content

Commit 2c45b2b

Browse files
committed
add implementation steps and address comments
1 parent 7b4902b commit 2c45b2b

1 file changed

Lines changed: 31 additions & 17 deletions

File tree

docs/architecture-decisions/disabled-flag-evaluation.md

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ When a flag exists in the store but has `state: DISABLED`, the evaluator should
5555

5656
The response has the following properties:
5757

58-
- **value**: Omitted (signals code-default deferral). The SDK/provider uses the application's code-defined default value.
59-
- **variant**: Omitted (empty string).
58+
- **value**: Omitted from the response. The SDK/provider uses the application's code-defined default value. In Go this surfaces as the type's zero value; on the wire (protobuf/JSON) the field is left unset; SDKs in languages with optional types should expose it as absent (`None`/`null`/`undefined`).
59+
- **variant**: Omitted from the response, using the same omission semantics as `value` above.
6060
- **reason**: `DISABLED`
6161
- **error**: `nil` (no error)
6262
- **metadata**: The merged flag set + flag metadata (consistent with current behavior for successful evaluations)
@@ -89,7 +89,8 @@ if flag.State == Disabled {
8989
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagDisabledErrorCode)
9090
}
9191

92-
// After
92+
// After — the empty string return is Go's zero value and is interpreted
93+
// downstream as "variant omitted"; it is not a literal empty-string variant.
9394
if flag.State == Disabled {
9495
return "", flag.Variants, model.DisabledReason, metadata, nil
9596
}
@@ -241,25 +242,38 @@ This is functionally the same change as the flagd core evaluator, replicated in
241242
- Bad, because it requires coordinated updates across flagd core, all gRPC/OFREP surfaces, in-process providers in each language SDK, and the testbed
242243
- Neutral, because the `FlagDisabledErrorCode` constant and related error-handling code can be removed (code simplification)
243244

244-
### Timeline
245+
### Versioning and migration
246+
247+
- This is a behavior-breaking change. Because flagd is pre-1.0, it is shipped as a minor-version bump and called out as breaking in the release notes — there is no dual-mode or deprecation period.
248+
- Operators relying on `FLAG_DISABLED` error signals (in dashboards, alerts, log filters, or HTTP 404 branches) must migrate to keying on successful evaluations with `reason=DISABLED`. Migration guidance is communicated through the release notes rather than a runtime compatibility flag.
249+
- The `FlagDisabledErrorCode` constant and its handling in `errFormat`, `errFormatV2`, and `EvaluationErrorResponseFrom` are removed outright in the same release. Retaining them serves no purpose once the evaluator no longer produces the error path, and there is no straightforward way to keep the old behavior reachable without re-introducing the spec-violating code path.
250+
251+
### Implementation steps
252+
253+
The work breaks down into three groups that must land together for a coherent release, but can be developed in parallel.
254+
255+
**flagd core (single release, behavior-breaking minor bump):**
256+
257+
1. Update `evaluateVariant` in `core/pkg/evaluator/json.go` to return `reason=DISABLED` with an omitted variant (code-default deferral) instead of an error.
258+
2. Update `resolve[T]` in `core/pkg/evaluator/json.go` to handle `DisabledReason` with omitted variants (avoids `TYPE_MISMATCH`).
259+
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.
260+
4. Update `SuccessResponseFrom` in `core/pkg/service/ofrep/models.go` to preserve `reason=DISABLED` (with field omission) for disabled flags deferring to code defaults.
261+
5. Update the gRPC v1 service layer (`flag_evaluator_v1.go`) to handle nil values in `ResolveAll` responses for disabled flags deferring to code defaults.
262+
6. Remove `FlagDisabledErrorCode` handling from `errFormat`, `errFormatV2`, and `EvaluationErrorResponseFrom`.
263+
264+
**Ecosystem (rolled out alongside or shortly after the flagd core release):**
265+
266+
7. Update each language SDK's in-process provider evaluator to return `reason=DISABLED` with code-default deferral instead of raising an error when encountering a disabled flag.
267+
8. Update OpenFeature providers (RPC and in-process) to recognize `DISABLED` as a non-error, non-cacheable reason.
268+
269+
**Validation and documentation:**
245270

246-
1. Update `evaluateVariant` in `core/pkg/evaluator/json.go` to return `reason=DISABLED` with an empty variant (code-default deferral) instead of an error
247-
2. Update `resolve[T]` in `core/pkg/evaluator/json.go` to handle `DisabledReason` with empty variants (avoids `TYPE_MISMATCH`)
248-
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
249-
4. Update `SuccessResponseFrom` in `core/pkg/service/ofrep/models.go` to preserve `reason=DISABLED` (with field omission) for disabled flags deferring to code defaults
250-
5. Remove `FlagDisabledErrorCode` handling from `errFormat`, `errFormatV2`, and `EvaluationErrorResponseFrom`
251-
6. Update gRPC v1 service layer (`flag_evaluator_v1.go`) to handle nil values in `ResolveAll` responses for disabled flags deferring to code defaults
252-
7. Update each language SDK's in-process provider evaluator to return `reason=DISABLED` with code-default deferral instead of raising an error when encountering a disabled flag
253-
8. Update flagd-testbed with test cases for disabled flag evaluation across all surfaces
254-
9. Update OpenFeature providers to recognize `DISABLED` as a non-error, non-cacheable reason
255-
10. Update provider documentation and migration guides
271+
9. Update flagd-testbed with test cases for disabled flag evaluation across all surfaces (gRPC v1, gRPC v2, OFREP single, OFREP bulk, in-process).
272+
10. Update provider documentation and call out the behavior change prominently in the flagd release notes.
256273

257274
### Open questions
258275

259276
- Should bulk evaluation include an option to exclude disabled flags for clients that prefer the current behavior (smaller payloads)?
260-
- 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?
261-
- 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?
262-
- Should the `FlagDisabledErrorCode` constant be retained (but unused) for a deprecation period, or removed immediately?
263277

264278
## More Information
265279

0 commit comments

Comments
 (0)