From 287aa99484fe5a043fc0f6fec8019ae00a6b1d78 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 17 Jul 2025 21:24:42 +0200 Subject: [PATCH 1/6] docs(ADR): standardize Flag configuration parsing and error handling Signed-off-by: Simon Schrottner --- .../flag_configuration_parsing.md | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100755 docs/architecture-decisions/flag_configuration_parsing.md diff --git a/docs/architecture-decisions/flag_configuration_parsing.md b/docs/architecture-decisions/flag_configuration_parsing.md new file mode 100755 index 000000000..b052f8460 --- /dev/null +++ b/docs/architecture-decisions/flag_configuration_parsing.md @@ -0,0 +1,77 @@ +--- +# Valid statuses: draft | proposed | rejected | accepted | superseded +status: draft +author: Simon Schrottner +created: 2025-07-17 +updated: YYYY-MM-DD +--- + +# Proposal for Unified Flag Parsing and Handling + +The goal of this proposal is to standardize how flags are parsed and handled across implementations. Currently, there is significant variation in how this is done, stemming from the organic growth of the project and contributions from multiple developers. This proposal aims to rethink and unify the parsing and handling of configurations, particularly for in-process scenarios. + +## Background + +Over time, the project has grown with contributions from various developers, leading to inconsistencies in how flags are parsed and handled. These inconsistencies have resulted in bugs, unexpected behaviors, and challenges in maintaining the codebase. For example, the file provider and JSON parsing behave differently across languages, and there is no clear definition of what constitutes a parsing error versus a flag-not-found error. + +Several issues have been raised that highlight the need for a unified approach: + +- [BUG] Inconsistency in file provider and JSON parsing for different languages (#1627) +- [BUG] [File Provider] Support File Formats (#1689) +- [FEATURE] Provider option to reject flag set with invalid rules (#1487) + +To address these issues, it is essential to establish a consistent and testable framework for flag parsing and handling. + +## Requirements + +- Utilize schema validation as much as possible, instead of custom logic, on a flag level rather than a configuration level. +- Define what consistutes a `PARSE_ERROR` versus a `FLAG_NOT_FOUND`. +- Define if an invalid flag entry within a configuration can invalidate the whole configuration. +- Define which file formats are supported (e.g., JSON, YAML, etc.). +- Ensure the framework is testable via the existing testbed/test-harness. + +## Considered Options + +- Standardize flag parsing and handling using schema validation (Option 1). +- Continue with the current approach but document the inconsistencies (Option 2). +- Develop separate parsing logic for each implementation to address specific needs (Option 3). + +## Proposal + +### Standardize Flag Parsing and Handling Using Schema Validation (Option 1) + +This proposal recommends adopting schema validation as the primary method for parsing and handling flags. The key elements of this approach include: + +1. **Schema Validation**: Use schema validation to ensure that flags meet predefined criteria. This reduces the reliance on custom logic and improves consistency across implementations. +2. **Supported File Formats**: Define and document the file formats that are supported (e.g., JSON, YAML). This aligns with the issue raised in [BUG] [File Provider] Support File Formats (#1689). +3. **Error Definitions**: Clearly define what constitutes a `PARSE_ERROR` versus a `FLAG_NOT_FOUND` error. Each inconsistent flag should evaluate as a `PARSE_ERROR`, only if the flag is not part of the configuration (or due to other definitions), it should evaluate to a `FLAG_NOT_FOUND` +4. **Invalid Flag Handling**: A invalid flag configuration should not affect the whole configuration. Other valid flags might still be important, and an error on one flag should not hinder updates or changes to the flag configuration. +5. **Testability**: Design the framework to be testable using the existing testbed/test-harness. This ensures that the implementation is robust and meets the defined requirements. + +### API Changes + +No immediate API changes are proposed. However, the internal logic for flag parsing and handling will be standardized, which may indirectly impact APIs that rely on these processes. + +### Consequences + +- **Positive Consequences**: + - Improved consistency across implementations. + - Easier maintenance and debugging. + - Enhanced testability and reliability. +- **Negative Consequences**: + - Initial development effort to implement the unified framework. + - Potential learning curve for contributors to adapt to the new framework. + +### Timeline + +### Open Questions + +- Are there any edge cases that need to be addressed in the schema validation rules? + +## More Information + +For additional context, refer to the following issues: + +- [BUG] Inconsistency in file provider and JSON parsing for different languages (#1627) +- [BUG] [File Provider] Support File Formats (#1689) +- [FEATURE] Provider option to reject flag set with invalid rules (#1487) From b92097583f9a1e0d9aac8b122e1eafdf8381a986 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Fri, 18 Jul 2025 08:42:52 +0200 Subject: [PATCH 2/6] fixup: add diagram Signed-off-by: Simon Schrottner --- .../flag_configuration_parsing.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/architecture-decisions/flag_configuration_parsing.md b/docs/architecture-decisions/flag_configuration_parsing.md index b052f8460..b26ae0b1d 100755 --- a/docs/architecture-decisions/flag_configuration_parsing.md +++ b/docs/architecture-decisions/flag_configuration_parsing.md @@ -48,6 +48,20 @@ This proposal recommends adopting schema validation as the primary method for pa 4. **Invalid Flag Handling**: A invalid flag configuration should not affect the whole configuration. Other valid flags might still be important, and an error on one flag should not hinder updates or changes to the flag configuration. 5. **Testability**: Design the framework to be testable using the existing testbed/test-harness. This ensures that the implementation is robust and meets the defined requirements. +#### Parsing Flow + +```mermaid +flowchart TD + A[Start Parsing] --> B{Is it valid JSON?} + B -->|No| C[Throw Error] + B -->|Yes| D[Check and Parse Configuration Metadata] + D --> E[Iterate Over All Flags] + E --> F{Does the Flag Pass Schema Validation?} + F -->|Yes| G[Add Flag with Key to Store] + F -->|No| H[Add Null Object with Key to Store] + E@{ shape: processes, label: "each Flag" } +``` +[Double click to switch code/render] ### API Changes No immediate API changes are proposed. However, the internal logic for flag parsing and handling will be standardized, which may indirectly impact APIs that rely on these processes. @@ -58,6 +72,7 @@ No immediate API changes are proposed. However, the internal logic for flag pars - Improved consistency across implementations. - Easier maintenance and debugging. - Enhanced testability and reliability. + - Robustness of Configuration issues (one invalid flag can not stop and update of others) - **Negative Consequences**: - Initial development effort to implement the unified framework. - Potential learning curve for contributors to adapt to the new framework. @@ -67,6 +82,7 @@ No immediate API changes are proposed. However, the internal logic for flag pars ### Open Questions - Are there any edge cases that need to be addressed in the schema validation rules? +- Should and error in the global metadata cause and issue with the parsing? ## More Information @@ -75,3 +91,4 @@ For additional context, refer to the following issues: - [BUG] Inconsistency in file provider and JSON parsing for different languages (#1627) - [BUG] [File Provider] Support File Formats (#1689) - [FEATURE] Provider option to reject flag set with invalid rules (#1487) + From 868122a89c8a9b05c701bc0ccf746ea6877abe0a Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Fri, 18 Jul 2025 08:45:52 +0200 Subject: [PATCH 3/6] fixup: suggestions Signed-off-by: Simon Schrottner --- .../architecture-decisions/flag_configuration_parsing.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/architecture-decisions/flag_configuration_parsing.md b/docs/architecture-decisions/flag_configuration_parsing.md index b26ae0b1d..0b3c6df49 100755 --- a/docs/architecture-decisions/flag_configuration_parsing.md +++ b/docs/architecture-decisions/flag_configuration_parsing.md @@ -12,7 +12,9 @@ The goal of this proposal is to standardize how flags are parsed and handled acr ## Background -Over time, the project has grown with contributions from various developers, leading to inconsistencies in how flags are parsed and handled. These inconsistencies have resulted in bugs, unexpected behaviors, and challenges in maintaining the codebase. For example, the file provider and JSON parsing behave differently across languages, and there is no clear definition of what constitutes a parsing error versus a flag-not-found error. +Initially we did not have a JSON schema, and validation was done manually after the configuration was parsed; in this case parsing was not consistent at all. +These inconsistencies have resulted in bugs, unexpected behaviors, and challenges in maintaining the codebase. +For example, the file provider and JSON parsing behave differently across languages, and there is no clear definition of what constitutes a parsing error versus a flag-not-found error. Several issues have been raised that highlight the need for a unified approach: @@ -25,7 +27,7 @@ To address these issues, it is essential to establish a consistent and testable ## Requirements - Utilize schema validation as much as possible, instead of custom logic, on a flag level rather than a configuration level. -- Define what consistutes a `PARSE_ERROR` versus a `FLAG_NOT_FOUND`. +- Define what constitutes a `PARSE_ERROR` versus what prevents the flag set from being updated. - Define if an invalid flag entry within a configuration can invalidate the whole configuration. - Define which file formats are supported (e.g., JSON, YAML, etc.). - Ensure the framework is testable via the existing testbed/test-harness. @@ -61,7 +63,9 @@ flowchart TD F -->|No| H[Add Null Object with Key to Store] E@{ shape: processes, label: "each Flag" } ``` + [Double click to switch code/render] + ### API Changes No immediate API changes are proposed. However, the internal logic for flag parsing and handling will be standardized, which may indirectly impact APIs that rely on these processes. @@ -91,4 +95,3 @@ For additional context, refer to the following issues: - [BUG] Inconsistency in file provider and JSON parsing for different languages (#1627) - [BUG] [File Provider] Support File Formats (#1689) - [FEATURE] Provider option to reject flag set with invalid rules (#1487) - From 1b73a4c316031ad2b790f8549c8795aa6a5a0ac2 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Fri, 18 Jul 2025 08:58:16 +0200 Subject: [PATCH 4/6] fixup: adding flow diagram Signed-off-by: Simon Schrottner --- .../flag_configuration_parsing.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/docs/architecture-decisions/flag_configuration_parsing.md b/docs/architecture-decisions/flag_configuration_parsing.md index 0b3c6df49..fbfc48400 100755 --- a/docs/architecture-decisions/flag_configuration_parsing.md +++ b/docs/architecture-decisions/flag_configuration_parsing.md @@ -64,6 +64,39 @@ flowchart TD E@{ shape: processes, label: "each Flag" } ``` +#### Resolution flow + +> [!NOTE] +> This was generated with AI and enhanced based on the java flagd provider + +```mermaid +flowchart TD + A[Start Evaluation] --> B[Retrieve flag from store] + B --> B1{Is an entry in the store} + B1 -->|No| D[Return FLAG_NOT_FOUND with error message] + B1 -->|Yes| B2{Is the flag null} + B2 -->|Yes| B3[Return PARSE_ERROR] + B2 -->|No| E{Is flag state DISABLED?} + E -->|Yes| F[Return FLAG_NOT_FOUND with disabled error] + E -->|No| G{Is targeting string empty?} + G -->|Yes| H[Set resolvedVariant to defaultVariant and reason to STATIC] + G -->|No| I[Evaluate targeting rule] + I --> J{Targeting evaluation successful?} + J -->|No| K[Throw ParseError for targeting rule] + J -->|Yes| L{Is targeting result null?} + L -->|Yes| M[Set resolvedVariant to defaultVariant and reason to DEFAULT] + L -->|No| N[Set resolvedVariant to targeting result and reason to TARGETING_MATCH] + M --> O[Check variant existence] + N --> O[Check variant existence] + O --> P{Is variant found in flag?} + P -->|No| Q[Throw General for missing variant] + P -->|Yes| R[Check variant type compatibility] + R --> S{Is type compatible?} + S -->|No| T[Throw TypeMismatchError for invalid type] + S -->|Yes| U[Return ProviderEvaluation with resolved value, variant, and reason] + +``` + [Double click to switch code/render] ### API Changes From ae0d294be04cef0a4bda977d26254bb832c2608b Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 21 Jul 2025 17:11:32 +0200 Subject: [PATCH 5/6] fixup: add option 4 Authored-by: Simon Schrottner Signed-off-by: Simon Schrottner --- docs/architecture-decisions/flag_configuration_parsing.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/architecture-decisions/flag_configuration_parsing.md b/docs/architecture-decisions/flag_configuration_parsing.md index fbfc48400..4c0d72961 100755 --- a/docs/architecture-decisions/flag_configuration_parsing.md +++ b/docs/architecture-decisions/flag_configuration_parsing.md @@ -37,6 +37,7 @@ To address these issues, it is essential to establish a consistent and testable - Standardize flag parsing and handling using schema validation (Option 1). - Continue with the current approach but document the inconsistencies (Option 2). - Develop separate parsing logic for each implementation to address specific needs (Option 3). +- Be rigid and enforce the JSON Schema Validation for all updates, even if that means ignoring updates if just one flag is violating the schema (Option 3). ## Proposal From a9b3444ffb7fef7ca900ce6973dca254262dda18 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Tue, 23 Sep 2025 12:46:18 +0200 Subject: [PATCH 6/6] fixup: update Signed-off-by: Simon Schrottner --- .../flag_configuration_parsing.md | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/docs/architecture-decisions/flag_configuration_parsing.md b/docs/architecture-decisions/flag_configuration_parsing.md index 4c0d72961..afea2e566 100755 --- a/docs/architecture-decisions/flag_configuration_parsing.md +++ b/docs/architecture-decisions/flag_configuration_parsing.md @@ -34,14 +34,14 @@ To address these issues, it is essential to establish a consistent and testable ## Considered Options -- Standardize flag parsing and handling using schema validation (Option 1). +- Standardize flag parsing and handling using schema validation ([Option 1](#option-1-standardize-flag-parsing-and-handling-using-schema-validation). - Continue with the current approach but document the inconsistencies (Option 2). - Develop separate parsing logic for each implementation to address specific needs (Option 3). -- Be rigid and enforce the JSON Schema Validation for all updates, even if that means ignoring updates if just one flag is violating the schema (Option 3). +- Be rigid and enforce the JSON Schema Validation for all updates, even if that means ignoring updates if just one flag is violating the schema ([Option 4](#option-4-full-document-validation-on-sync)). ## Proposal -### Standardize Flag Parsing and Handling Using Schema Validation (Option 1) +### Option 1: Standardize Flag Parsing and Handling Using Schema Validation This proposal recommends adopting schema validation as the primary method for parsing and handling flags. The key elements of this approach include: @@ -98,13 +98,11 @@ flowchart TD ``` -[Double click to switch code/render] - -### API Changes +#### API Changes No immediate API changes are proposed. However, the internal logic for flag parsing and handling will be standardized, which may indirectly impact APIs that rely on these processes. -### Consequences +#### Consequences Option 1 - **Positive Consequences**: - Improved consistency across implementations. @@ -115,12 +113,43 @@ No immediate API changes are proposed. However, the internal logic for flag pars - Initial development effort to implement the unified framework. - Potential learning curve for contributors to adapt to the new framework. +### Option 4: Full Document Validation on Sync + +#### Summary + +This alternative proposal suggests validating the entire flags configuration document using the schema provided at [https://flagd.dev/schema/v0/flags.json](https://flagd.dev/schema/v0/flags.json). The validation would occur both at startup and during each sync update. + +#### Key Elements + +1. **Validation at Startup**: + +- If any part of the configuration is invalid, the application fails to start. +- This aligns with the current behavior, where failure to parse the flags configuration prevents startup. + +1. **Validation on Sync Updates**: + +- If any part of the updated configuration is invalid, the update is ignored. +- An error or warning message is logged prominently to alert the user. +- This builds on the existing behavior, where unparseable configurations are rejected. + +#### Consequences Option 4 + +- **Positive Consequences**: + - Ease of Implementation: The validation step can be added to the existing parsing logic with minimal changes. + - Simplified Testing: Many error-edge cases in the test suite can be removed, as invalid configurations will never be allowed. + - Schema-Driven Validation: Relying on the schema eliminates the need for detailed error-handling logic. + +- **Negative Consequences**: + - Blocking Updates Due to Errors: A single invalid flag will prevent the entire update from being applied. + - However, this is consistent with the current behavior, where unparseable flags already block updates. + ### Timeline ### Open Questions - Are there any edge cases that need to be addressed in the schema validation rules? - Should and error in the global metadata cause and issue with the parsing? +- What solution do we prefer? ## More Information