feat: return flag metadata as well #1476

aasifkhan7 · 2024-12-16T21:42:59Z

This PR

Adds functionality to return flag metadata as part of the response.

Related Issues

Fixes #1464

Notes

This PR ensures that flag metadata is included and handled correctly in responses.

netlify · 2024-12-16T21:43:16Z

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	`c1770b1`
🔍 Latest deploy log	https://app.netlify.com/sites/polite-licorice-3db33c/deploys/677d29009ec6a00008d81665

Signed-off-by: Aasif Khan <[email protected]>

beeme1mr · 2024-12-17T20:11:45Z

Hey @aasifkhan7, I tested this locally and wasn't able to see flag metadata returned as a response.

Here's the flag configuration I was using:

{
  "flags": {
    "myBoolFlag": {
      "state": "ENABLED",
      "variants": {
        "on": true,
        "off": false
      },
      "defaultVariant": "on"
    }
  },
  "metadata": {
    "id": "test",
    "version": "1"
  }
}

Here are the test evaluations I tried:

curl -X POST "localhost:8013/flagd.evaluation.v1.Service/ResolveBoolean" -d '{"flagKey":"myBoolFlag","context":{}}' -H "Content-Type: application/json"

and

curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags'

Please map the id and version to flagSetId and flagSetVersion respectively in the flag metadata. That will set us up nicely once this spec PR is merged. Thanks!

Signed-off-by: Aasif Khan <[email protected]>

aasifkhan7 · 2024-12-18T11:36:47Z

Hi @beeme1mr,

I've updated the commit to include the setting of flagSetId and flagSetVersion in the flag metadata as well. Could you please re-review the changes?

toddbaert

@aasifkhan7 nice job so far!

OFREP works as expected:

curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags' | jq

{
  "flags": [
    {
      "value": 1,
      "key": "myIntFlag",
      "reason": "STATIC",
      "variant": "one",
      "metadata": {
        "flagSetId": "test",
        "flagSetVersion": "1"
      },
    ...
    }
  ]
}

curl -X POST "localhost:8013/flagd.evaluation.v1.Service/ResolveBoolean" -d '{"flagKey":"myBoolFlag","context":{}}' -H "Content-Type: application/json" | jq


{
  "value": true,
  "reason": "STATIC",
  "variant": "on",
  "metadata": {
    "flagSetId": "test",
    "flagSetVersion": "1"
  }
}

I left some comments - I also think we need some basic test coverage for this, but nice job!

toddbaert · 2024-12-18T14:20:05Z

core/pkg/evaluator/json.go

+	if flagMetadata.ID != "" {
+		metadata[ID] = flagMetadata.ID
+	}
+	if flagMetadata.Version != "" {
+		metadata[VERSION] = flagMetadata.Version
+	}


I'm not sure what the value of these is, since we are not returning them in any responses. Maybe I'm confused.

@toddbaert The idea behind this is that each flag could also have its own specific metadata. For example, a flag configuration might look like this:

{ "flags": { "myBoolFlag": { "state": "ENABLED", "variants": { "on": true, "off": false }, "defaultVariant": "on", "metadata": { "id": "sso/dev", "version": "1.0.0" } } }, "metadata": { "id": "test", "version": "1" } }

Here, the flag-level metadata (id, version) allows for more granular identification and versioning of individual flags, in addition to the overarching metadata. Let me know your thoughts!

core/pkg/store/flags.go

Signed-off-by: Aasif Khan <[email protected]>

apodgorbunschih · 2025-01-07T11:59:19Z

👋 Hello folks,
@aasifkhan7 I had a question regarding the following MR.
Accordingly to that spec https://github.com/open-feature/flagd-schemas/pull/173/files the Metadata can be "Any additional key/value pair with value of type boolean, string, or number.". However in this MR the Metadata have a defined structure. Or this is something else and I am confusing 2 things ? 🤔 🙏
Thanks in advance!

Signed-off-by: Aasif Khan <[email protected]>

aasifkhan7 · 2025-01-07T13:17:12Z

👋 Hello folks, @aasifkhan7 I had a question regarding the following MR. Accordingly to that spec https://github.com/open-feature/flagd-schemas/pull/173/files the Metadata can be "Any additional key/value pair with value of type boolean, string, or number.". However in this MR the Metadata have a defined structure. Or this is something else and I am confusing 2 things ? 🤔 🙏 Thanks in advance!

@apodgorbunschih I've updated the PR to handle any fields that come in metadata.

Please re-review @beeme1mr @toddbaert

beeme1mr

Thanks, I'll manually test this. Could you please also ensure there are tests in the PR. Thanks!

beeme1mr · 2025-01-07T13:21:04Z

core/pkg/evaluator/json_model.go

-	Flags    map[string]model.Flag `json:"flags"`
-	MetaData Metadata              `json:"metadata"`
+	Flags    map[string]model.Flag  `json:"flags"`
+	MetaData map[string]interface{} `json:"metadata"`


Suggested change

MetaData map[string]interface{} `json:"metadata"`

Metadata map[string]interface{} `json:"metadata"`

This will also need to change everywhere it's being used.

beeme1mr · 2025-01-07T13:22:00Z

core/pkg/model/flag.go

+	Targeting      json.RawMessage        `json:"targeting,omitempty"`
+	Source         string                 `json:"source"`
+	Selector       string                 `json:"selector"`
+	MetaData       map[string]interface{} `json:"metadata"`


Suggested change

MetaData map[string]interface{} `json:"metadata"`

Metadata map[string]interface{} `json:"metadata"`

Update all references too please 🙏

beeme1mr · 2025-01-07T13:22:15Z

core/pkg/store/flags.go

@@ -22,7 +22,7 @@ type Flags struct {
 	Flags          map[string]model.Flag `json:"flags"`
 	FlagSources    []string
 	SourceMetadata map[string]SourceDetails
-	Metadata       model.Metadata `json:"metadata"`
+	MetaData       map[string]interface{} `json:"metadata"`


Suggested change

MetaData map[string]interface{} `json:"metadata"`

Metadata map[string]interface{} `json:"metadata"`

beeme1mr · 2025-01-07T13:28:29Z

I've also addressed the unrelated license compliance issue.

aasifkhan7 requested a review from a team as a code owner December 16, 2024 21:43

dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 16, 2024

aasifkhan7 changed the title ~~return flag metadata as well~~ feat: return flag metadata as well Dec 17, 2024

aasifkhan7 force-pushed the feat/flag-metadata branch 3 times, most recently from 3b6e501 to c55537f Compare December 17, 2024 05:20

return flag metadata as well

931c120

Signed-off-by: Aasif Khan <[email protected]>

aasifkhan7 force-pushed the feat/flag-metadata branch from c55537f to 931c120 Compare December 17, 2024 05:22

aasifkhan7 added 2 commits December 17, 2024 10:54

Merge branch 'main' into feat/flag-metadata

29bd3f7

added a json name for flagsetmetadata

5d8173e

Signed-off-by: Aasif Khan <[email protected]>

fix: start returning flagSetId, flagSetVersion as well in flag metadata

dd48ee3

Signed-off-by: Aasif Khan <[email protected]>

aasifkhan7 force-pushed the feat/flag-metadata branch from f75d5b5 to dd48ee3 Compare December 18, 2024 11:33

toddbaert reviewed Dec 18, 2024

View reviewed changes

core/pkg/store/flags.go Outdated Show resolved Hide resolved

toddbaert requested review from hairyhenderson, Kavindu-Dodan, aepfli, toddbaert, warber, craigpastro, gruebel and thisthat December 18, 2024 14:22

aasifkhan7 added 3 commits December 20, 2024 13:28

fetching flag metadata with a single lock

5cb824c

Signed-off-by: Aasif Khan <[email protected]>

change id of flagSet -> id

bb2ca43

Signed-off-by: Aasif Khan <[email protected]>

Merge branch 'open-feature:main' into feat/flag-metadata

9480943

metadata can be any generic value

c1770b1

Signed-off-by: Aasif Khan <[email protected]>

beeme1mr reviewed Jan 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: return flag metadata as well #1476

feat: return flag metadata as well #1476

aasifkhan7 commented Dec 16, 2024 •

edited

Loading

netlify bot commented Dec 16, 2024 •

edited

Loading

beeme1mr commented Dec 17, 2024

aasifkhan7 commented Dec 18, 2024

toddbaert left a comment

toddbaert Dec 18, 2024

aasifkhan7 Dec 18, 2024 •

edited

Loading

apodgorbunschih commented Jan 7, 2025

aasifkhan7 commented Jan 7, 2025

beeme1mr left a comment

beeme1mr Jan 7, 2025

beeme1mr Jan 7, 2025

beeme1mr Jan 7, 2025

beeme1mr commented Jan 7, 2025

	MetaData map[string]interface{} `json:"metadata"`
	Metadata map[string]interface{} `json:"metadata"`

feat: return flag metadata as well #1476

Are you sure you want to change the base?

feat: return flag metadata as well #1476

Conversation

aasifkhan7 commented Dec 16, 2024 • edited Loading

This PR

Related Issues

Notes

netlify bot commented Dec 16, 2024 • edited Loading

✅ Deploy Preview for polite-licorice-3db33c canceled.

beeme1mr commented Dec 17, 2024

aasifkhan7 commented Dec 18, 2024

toddbaert left a comment

Choose a reason for hiding this comment

toddbaert Dec 18, 2024

Choose a reason for hiding this comment

aasifkhan7 Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

apodgorbunschih commented Jan 7, 2025

aasifkhan7 commented Jan 7, 2025

beeme1mr left a comment

Choose a reason for hiding this comment

beeme1mr Jan 7, 2025

Choose a reason for hiding this comment

beeme1mr Jan 7, 2025

Choose a reason for hiding this comment

beeme1mr Jan 7, 2025

Choose a reason for hiding this comment

beeme1mr commented Jan 7, 2025

aasifkhan7 commented Dec 16, 2024 •

edited

Loading

netlify bot commented Dec 16, 2024 •

edited

Loading

aasifkhan7 Dec 18, 2024 •

edited

Loading