Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] There is no "value" key in the evaluation response #1116

Closed
zeezdev opened this issue Jan 3, 2024 · 2 comments · Fixed by #1117
Closed

[BUG] There is no "value" key in the evaluation response #1116

zeezdev opened this issue Jan 3, 2024 · 2 comments · Fixed by #1117
Assignees
Labels
bug Something isn't working

Comments

@zeezdev
Copy link

zeezdev commented Jan 3, 2024

Observed behavior

When a request to a new endpoint flagd.evaluation.v1.Service/ResolveBoolean (#1083) for a boolean flag evaluates to a negative variant, then the response is missing the value key:

{"reason":"TARGETING_MATCH", "variant":"off", "metadata":{}}

Expected Behavior

The evaluation response contains value key for both negative and positive variants:

{"value":false, "reason":"TARGETING_MATCH", "variant":"off", "metadata":{}}
{"value":true, "reason":"TARGETING_MATCH", "variant":"on", "metadata":{}}

Steps to reproduce

The flags definition (test.json):

{
  "flags": {
    "EXPERIMENT__CREATE": {
      "variants": {
        "on": true,
        "off": false
      },
      "state": "ENABLED",
      "defaultVariant": "off",
      "targeting": {
        "if": [
          {
            "==": [
              false,
              {
                "var": [
                  "user.is_superuser"
                ]
              }
            ]
          },
          "on",
          "off"
        ]
      }
    }
  }
}

Up flagd (v0.8.0):

docker run \
  --rm -it \
  --name flagd \
  -p 8013:8013 \
  -v $(pwd):/etc/flagd/ \
  ghcr.io/open-feature/flagd:v0.8.0 \
  start --uri file:/etc/flagd/test.json --debug

Request for a new endpoint (flagd.evaluation.v1.Service/ResolveBoolean) with a context for which the variant off is expected:

curl -X POST 'http://localhost:8013/flagd.evaluation.v1.Service/ResolveBoolean' \
--header 'Content-Type: application/json' \
--data '
{
	"flagKey": "EXPERIMENT__CREATE",
	"context": {
		"user": {
			"is_superuser": true
		}
	}
}'

The response does not contain value attribute:

{"reason":"TARGETING_MATCH", "variant":"off", "metadata":{}}

Request for a new endpoint (flagd.evaluation.v1.Service/ResolveBoolean) with a context for which the variant on is expected:

curl -X POST 'http://localhost:8013/flagd.evaluation.v1.Service/ResolveBoolean' \
--header 'Content-Type: application/json' \
--data '
{
	"flagKey": "EXPERIMENT__CREATE",
	"context": {
		"user": {
			"is_superuser": false
		}
	}
}'

The value attribute is present in the response:

{"value":true, "reason":"TARGETING_MATCH", "variant":"on", "metadata":{}}

When a request is made to the old endpoint by both contexts (off & on variants are expected), the response contains the value attribute:

curl -X POST 'http://localhost:8013/schema.v1.Service/ResolveBoolean' \
--header 'Content-Type: application/json' \
--data '
{
	"flagKey": "EXPERIMENT__CREATE",
	"context": {
		"user": {
			"is_superuser": false
		}
	}
}'
{"value":true, "reason":"TARGETING_MATCH", "variant":"on", "metadata":{}}
curl -X POST 'http://localhost:8013/schema.v1.Service/ResolveBoolean' \
--header 'Content-Type: application/json' \
--data '
{
	"flagKey": "EXPERIMENT__CREATE",
	"context": {
		"user": {
			"is_superuser": true
		}
	}
}'
{"value":false, "reason":"TARGETING_MATCH", "variant":"off", "metadata":{}}
@zeezdev zeezdev added bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer labels Jan 3, 2024
@beeme1mr
Copy link
Member

beeme1mr commented Jan 3, 2024

Hey @zeezdev, thanks for reporting this. I'll work on a fix right await.

@beeme1mr beeme1mr removed the Needs Triage This issue needs to be investigated by a maintainer label Jan 3, 2024
@beeme1mr beeme1mr self-assigned this Jan 3, 2024
@beeme1mr beeme1mr linked a pull request Jan 3, 2024 that will close this issue
beeme1mr added a commit that referenced this issue Jan 4, 2024
## This PR

- add custom marshalling options

### Related Issues

Fixes #1116

### Notes

I manually tested to confirm the fix is working. However, it's currently
not automatically tested because the issue only affects HTTP-based
requests, and the E2E tests use gRPC. I'll create a follow-up task to
expand the E2E test suite to include HTTP tests.

Signed-off-by: Michael Beemer <[email protected]>
@beeme1mr
Copy link
Member

beeme1mr commented Jan 4, 2024

Fixed in v0.8.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants