Add unique label on flags

[vayan]

Description

For now we only have one way to find a Flag: the description.

But description are not unique and very volatile and should not be used to find a precise Flag imo.

ID aren't good enough too since they're linked to the database and they change accross Flagr instances (e.g staging/production). An unique label fixes all of these issues.

I'm not good with the Vue frontend don't hesitate to push a commit on this PR to improve it.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov-io]

Codecov Report

Merging #148 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
+ Coverage   93.46%   93.49%   +0.03%     
==========================================
  Files          23       23              
  Lines        1178     1184       +6     
==========================================
+ Hits         1101     1107       +6     
  Misses         54       54              
  Partials       23       23

Impacted Files	Coverage Δ
pkg/entity/flag.go	100% <ø> (ø)	⬆️
pkg/mapper/entity_restapi/e2r/e2r.go	100% <100%> (ø)	⬆️
pkg/handler/crud.go	94.38% <100%> (+0.09%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f060319...b459201. Read the comment docs.

[zhouzhuojie]

Unique lable or more precisely - unique flag_key has its own pros/cons too.

• flag_key should be immutable, which makes it hard to name it right at the beginning. The combination of ID + description fixes it, so we can benefit from ID's immutablibility and description's flexibility.
• We have some considerations on the request/response payload size, for example, sending ~1000 flags to the batch evaluation endpoint, the string key size is significant larger than integers. This batch pattern is also used for mobile app's A/B testing, and it loads all the feature flags at the bootstrap time.

The downside of IDs is that they are sequential, which makes it hard to link across multiple environments. Although it's addressable, for example, use a read-replica db for staging, or leverage a prod/staging manifests in code that map the features into flag ids - it's possible that you may already have that configuraiton file (mapping features to flag_ids) in code.

[vayan]

Thanks for the answer! I opened the PR to start the discussions I appreciate the insights.

I agree on all your points except that I still think that another non-db-sequential immutable key is still needed.
we actually don’t really need for it to be a string another integer should do the job without growing the response size too much and that also fix the naming issue.

I’m not sure I understand your last point, for now I’m mapping features to flag descriptions since I can’t know for sure the ID of the flag.

Hope we can find a solution 😊

Anyway thanks for project!

[volnt]

That would be useful!

[zhouzhuojie]

I’m not sure I understand your last point, for now I’m mapping features to flag descriptions since I can’t know for sure the ID of the flag.

Thanks for the discussion @vayan!

I'm interested in why you don't know the flag ID before implementing the feature. It's possible something I didn't consider before, but for us, it's very common that we just go ahead create the flag first, and then implement the feature and refer the flag ID created before.

[vayan]

why you don't know the flag ID before implementing the feature

It's mostly because I don't know on which database my code will be run against so I have no control over the IDs.
There's also the case of integration testing, I'm creating/deleting lot of flags on the fly which again change the IDs a lot.

If when I create a flag I could set an unique identifier I would not have to care about databases id increments and stuff 😄

[marceloboeira]

@vayan we had the same issue and one of the ideas at the time was to just use the GET findFlags endpoint with the description_like field and the identifier...

[vayan]

@marceloboeira That's what I'm doing right now too 😄 but the description is so easily editable and not unique, it's kind of flaky

[zhouzhuojie]

Related to #154

I know it won't address all the cons of not having unique string based IDs, hopefully it will make setting up Flagr in tests or staging easier.

Back to the story of unique labels. If your code doesn't know which database is running, it requires not only CRUD endpoints to support unique lables, but also the evaluation endpoints, right? That requires a change for example,

curl --request POST \
       --url https://try-flagr.herokuapp.com/api/v1/evaluation \
       --header 'content-type: application/json' \
       --data '{
           "entityID": "127",
           "entityType": "user",
           "entityContext": {
             "state": "NY"
           },
           "flagID": 1,
           "flagKey": "some_flag_key",   <-- (or flagLable, and we need to check if the payload has either ID or Key)
           "enableDebug": true
       }'

Also, this applies to not only flags, but also all the modes. May be we can change the base gorm model for all the entities later, for example, adding a unique Key field for every entity.

[vayan]

but also the evaluation endpoints, right?

Right! I didn't want to change everything in the same PR 😄

for example, adding a unique Key field for every entity.

Sure, but adding one to the Flag can be a start, that way we'd be able to evaluate them without the DB ID.

[zhouzhuojie]

Hi @vayan

Yesterday, I was just about to approve this pr, but figured out there are so many moving parts for this change, thus I created another pr that adds the same thing (including all endpoints affected by flagKey), just the naming will be key not label, since we are already using key for variants.

#159

[vayan]

Awesome! Thanks @zhouzhuojie. Your PR is much more complete than mine np 😄