feat(): add string type support

[a-karev-vsc]

Description

Adds free-form string feature flags alongside boolean and percentile, plus an explicit --type flag for set.

• models: String type, ParseFeatureType, ParseValueForType, StringValue().
• client: new GetString(feature) (returns "" if absent or not a string); added to IFace.
• cli: --type/-t flag (boolean|percentile|string). Boolean/percentile still inferred when omitted; string requires --type=string. Clearer validation errors.
• docs: README updated for the string type, GetString, and --type.

ParseValueAndFeatureType now infers String as the catch-all (was Invalid); the CLI uses that to require --type=string. Invalid is still used by ParseFeatureType and as the controller's error sentinel.

Why

It can simplify string dcdr flags (like potential min-log-level for the new structured logging in godel).
Dcdr previously only accepted booleans and 0.0–1.0 floats, so there was no clean way to store a string value.

Usage

dcdr set -n min-log-level -v debug -t string # string needs -t

dcdr set -n kill-switch -v true # boolean/percentage unchanged

level := dcdr.GetString("min-log-level")

Compatibility

• Existing Consul/Redis flags untouched - strings just serialize as "feature_type":"string". No migration.
• Older clients reading a string flag fall through IsAvailable/ScaleValue defaults (false/min), never panic.
• --type requirement prevents a typo'd bool/percentile from silently becoming a string.
• IFace gains GetString; concrete types and the mock inherit it via embedding - only hand-rolled IFace implementations would need it.

Tests

• Unit tests for the new model/client/CLI paths, plus round-trip tests proving a string survives KVsToFeatureMap → served JSON → UpdateFeatures → GetString.
• go build, go vet, and go test

[tdarci]

cool.
What are the implications for our Consul backing store for this data?

Also, there is the part of the api that clients call to get all the dcdr values. Is the change reflected there?

[a-karev-vsc]

cool. What are the implications for our Consul backing store for this data?

Also, there is the part of the api that clients call to get all the dcdr values. Is the change reflected there?

No Consul (or Redis) implications - features are stored as opaque JSON bytes of the Feature struct.
Everything is fully compatible.

[trevorrosenkilde]

Not opposed to this, but do have to say we have string functionality in Growthbook IIRC. Any reason we're leaning on dcdr over Growthbook here? Perhaps just the barrier to entry (i.e. dcdr is on all our services whereas GB is not?)

[a-karev-vsc]

Not opposed to this, but do have to say we have string functionality in Growthbook IIRC. Any reason we're leaning on dcdr over Growthbook here? Perhaps just the barrier to entry (i.e. dcdr is on all our services whereas GB is not?)

Yep, that's the reason.
We have multiple potential options:

• just use env var (which would require a pod restart if we need to change min log level, e.g. enable debug logs, but that could loose in-memory state and potentially not have issue we want to debug anymore)
• dcdr
• GB (in the future)

I think we can switch to GB later if we want to, but we gonna need to add GB and GB API key to some base kube template so we can inject that data to every service and worker (+ change our GrowthBook interface to add String support too).

Another reason - for such low-level stuff we don't have any vendor locks / third-party dependencies.

[trevorrosenkilde]

Not opposed to this, but do have to say we have string functionality in Growthbook IIRC. Any reason we're leaning on dcdr over Growthbook here? Perhaps just the barrier to entry (i.e. dcdr is on all our services whereas GB is not?)

Yep, that's the reason. We have multiple potential options:

• just use env var (which would require a pod restart if we need to change min log level, e.g. enable debug logs, but that could loose in-memory state and potentially not have issue we want to debug anymore)
• dcdr
• GB (in the future)

I think we can switch to GB later if we want to, but we gonna need to add GB and GB API key to some base kube template so we can inject that data to every service and worker (+ change our GrowthBook interface to add String support too).

Another reason - for such low-level stuff we don't have any vendor locks / third-party dependencies.

Makes total sense 👍🏻

[trevorrosenkilde]

Also, there is the part of the api that clients call to get all the dcdr values. Is the change reflected there?

Good call out, @tdarci.

@a-karev-vsc - you may want to check out the dcdr package in godel (which is the dcdr-api). We may need to be careful with returning these new string values - clients may be brittle against this.

I don't think we need to return these string values there - may be safest/easiest to just exclude those in the response.

[a-karev-vsc]

Also, there is the part of the api that clients call to get all the dcdr values. Is the change reflected there?

Good call out, @tdarci.

@a-karev-vsc - you may want to check out the dcdr package in godel (which is the dcdr-api). We may need to be careful with returning these new string values - clients may be brittle against this.

I don't think we need to return these string values there - may be safest/easiest to just exclude those in the response.

Yep, already replied above, it's fully compatible and safe :)

[trevorrosenkilde]

Thanks for the validations!

[tdarci]

Let's make sure the dcdr-api deployment that's in godel still works. Handler here: https://github.com/vsco/godel/blob/4ddbfd38c257a6e2c3c62d691848dc206b9b5b62/dcdr/api/handlers/features_handler.go#L39

<-- sounds like you have already verified this? I think I missed the comment

[tdarci]

🚀

[a-karev-vsc]

Phase 1 testing (local Consul, new and old dcdr binary):

// new binary:

>  ./bin/dcdr list
[dcdr] no feature flags found in namespace: dcdr

>  ./bin/dcdr set -n test-str -v debug -t string
[dcdr] set flag 'dcdr/features/default/test-str'

>  ./bin/dcdr list

Name      Type    Value  Scope    Updated By       Comment
test-str  string  debug  default  alex-local-test

>  ./bin/dcdr set -n test-str-literal -v true -t string
[dcdr] set flag 'dcdr/features/default/test-str-literal'

>  ./bin/dcdr list

Name              Type    Value  Scope    Updated By       Comment
test-str          string  debug  default  alex-local-test
test-str-literal  string  true   default  alex-local-test

>  ./bin/dcdr set -n test-str-pad -v "  debug  " -t string
[dcdr] set flag 'dcdr/features/default/test-str-pad'

>  ./bin/dcdr list

Name              Type    Value  Scope    Updated By       Comment
test-str          string  debug  default  alex-local-test
test-str-literal  string  true   default  alex-local-test
test-str-pad      string  debug  default  alex-local-test

>  ./bin/dcdr set -n test-str-empty -v "   " -t string
[dcdr error] parse error: invalid -value for string. must not be empty or whitespace-only

>  ./bin/dcdr list

Name              Type    Value  Scope    Updated By       Comment
test-str          string  debug  default  alex-local-test
test-str-literal  string  true   default  alex-local-test
test-str-pad      string  debug  default  alex-local-test

>  ./bin/dcdr set -n test-bool -v true
[dcdr] set flag 'dcdr/features/default/test-bool'

>  ./bin/dcdr set -n test-pct -v 0.5
[dcdr] set flag 'dcdr/features/default/test-pct'

>  ./bin/dcdr list

Name              Type        Value  Scope    Updated By       Comment
test-bool         boolean     true   default  alex-local-test
test-pct          percentile  0.5    default  alex-local-test
test-str          string      debug  default  alex-local-test
test-str-literal  string      true   default  alex-local-test
test-str-pad      string      debug  default  alex-local-test

>  ./bin/dcdr set -n test-bad -v debug
[dcdr error] parse error: -type=string is required for non-numeric, non-boolean values

>  ./bin/dcdr set -n test-str -v 0.5
[dcdr error] set error: cannot change existing feature types

// old binary via main.go:

> go get github.com/vsco/dcdr@v0.3.5-0.20240314201651-e97228a0bc3a
...
> go run .
features: map[test-bool:true test-pct:0.5 test-str:debug test-str-literal:true test-str-pad:debug]

IsAvailable(test-str)        = false
IsAvailable(test-bool)       = true
IsAvailableForID(test-pct,5) = true
ScaleValue(test-pct,0,10)    = 5
FeatureExists(test-str)      = true

[a-karev-vsc]

Phase 2 testing (dev):

> kubectl port-forward -n consul svc/consul-server 8500:8500
Forwarding from 127.0.0.1:8500 -> 8500
Forwarding from [::1]:8500 -> 8500

// new binary set string value:

> ./bin/dcdr set -n test-str-ak-delete-me -v debug -t string
[dcdr] set flag 'dcdr/features/default/test-str-ak-delete-me'

// new binary read value:

> ./bin/dcdr list -p test-str-ak

Name                   Type    Value  Scope    Updated By       Comment  
test-str-ak-delete-me  string  debug  default  alex-local-test 

// old binary read value:

> kubectl exec deploy/dcdr -c dcdr -n vsco -- cat /etc/dcdr/decider.json | grep test-str-ak
        "test-str-ak-delete-me": "debug",

dcdr list works for both (new and old) binaries