PRSD-1520: First draft of feature flag adr #777
Conversation
| Draft | ||
|
|
||
| Date of decision: Pending |
There was a problem hiding this comment.
Just a reminder to update these to accepted and with the date before you merge.
| * Bad, because it leads to messy code and makes feature flag logic hard to maintain and test. | ||
| * Bad, because it does not leverage our dependency injection system, reducing code clarity and maintainability. | ||
|
|
||
| ### Beans selected by static configuration |
There was a problem hiding this comment.
Is there maybe another 'bad' here, that configuration and the bean level is relatively coarse and/or will force us into more complex designs just to allow some portion of code to be a configurable bean?
| Directly branch code using if statements for feature flags. | ||
|
|
||
| * Good, because it is simple to implement and requires no additional dependencies. | ||
| * Bad, because it leads to messy code and makes feature flag logic hard to maintain and test. |
There was a problem hiding this comment.
Could this be mitigated a little by having some basic interface for checking FFs, which might initially be implemented as something as simple as checking a relevant constant, but could end up being replaced by a more fully featured library?
Also, wouldn't a more fully-feature library approach also mean you need to sprinkle a load of if statements through the code?
There was a problem hiding this comment.
Agree that we'll probably need to have some if statements regardless - in my mind this option was more "ONLY use if statements, and not beans", whereas the next was more 'Use beans where possible", but I maybe missing some nuance.
There was a problem hiding this comment.
Sort of - in fact the suggestion here is that we will ALWAYS use two beans even if they are two potential instances of one class with an if statement in it. In that scenario we would have some kind of configuration interface passed into the class constructor describing the desired bahviour at the toggle point (e.g. should send additional warning email when details updated) which will be set based on the feature flag value (e.g. new account control notifications).
The thought here is to make switching beans or bean configuration the way to apply feature flag configurations so that the application point is clean.
There was a problem hiding this comment.
Ah, so we are saying you would have two copies of (in extremis) the same journey class, one with an extra step in, and the other without, so that we could select between them at the class/ bean level?
| * Good, because it integrates well with our DI system and keeps code clean. | ||
| * Good, because it supports some advanced features without custom code. | ||
| * Bad, because any advanced features not supported by the library would require integrated custom implementation. | ||
| * Neutral, because it allows runtime flag changes, to alter the service behaviour without redeployment (but this is not needed). |
There was a problem hiding this comment.
(but this is not needed)
Well, ish. I think we could make use of this feature as an emergency turn-off-the-broken-thing tool, for example?
There was a problem hiding this comment.
Yes, think this is a good + a bad rather than a neutral (if that makes sense) - good because if the client desires they can turn things on/ off without a deployment/ dev support, but bad because they makes everything less predictable/ repeatable
There was a problem hiding this comment.
Yeah, that makes sense.
Ticket number
PRSD-1520
Goal of change
Add an ADR for feature flags.
Checklist
N/A