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] flagd (transposeEvaluators fn) doesn't understand json without indentations #244

Closed
vadasambar opened this issue Jan 4, 2023 · 1 comment · Fixed by #250
Closed
Assignees
Labels
bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer

Comments

@vadasambar
Copy link
Contributor

vadasambar commented Jan 4, 2023

Observed behavior

transposeEvaluators doesn't behave correctly if the json passed to it doesn't have the right indentation.

Expected Behavior

transposeEvaluators should handle cases where the json passed to it doesn't have the right indentation.

Steps to reproduce

  1. Check out this PR's branch
  2. Put fmt.Println statements just after this line and after this line.
  3. Use json.Marshal instead of json.MarshalIndent here
  4. From flagd root directory run (example_flags.yaml can be found here):
$ go run main.go  start -f file:./config/samples/example_flags.yaml -e=yaml 
{"level":"info","ts":1672807996.7619019,"caller":"service/connect_service.go:108","msg":"metrics listening at 8014","component":"service"}
evalValue before {"in":["@faas.com",{"var":["email"]}]}
evalValue after "in":["@faas.com",{"var":["email"]}
{"level":"fatal","ts":1672807996.762957,"caller":"cmd/start.go:116","msg":"set state: unmarshal new state: invalid character '}' after array element","component":"start","stacktrace":"github.com/open-feature/flagd/cmd.glob..func1\n\t/home/suraj/sandbox/flagd/cmd/start.go:116\ngithub.com/spf13/cobra.(*Command).execute\n\t/home/suraj/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:860\ngithub.com/spf13/cobra.(*Command).ExecuteC\n\t/home/suraj/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974\ngithub.com/spf13/cobra.(*Command).Execute\n\t/home/suraj/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902\ngithub.com/open-feature/flagd/cmd.Execute\n\t/home/suraj/sandbox/flagd/cmd/root.go:38\nmain.main\n\t/home/suraj/sandbox/flagd/main.go:30\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250"}
exit status 1

If you check the above log,

evalValue before {"in":["@faas.com",{"var":["email"]}]}
evalValue after "in":["@faas.com",{"var":["email"]}

Because the trailing brackets are not on newline, flagd trims the last two trailing brackets which makes the json invalid. This problem doesn't happen if you use json.MarshalIndent like this

To contrast this with using a json config file,

$ go run main.go  start -f file:./config/samples/example_flags.json -e=json
{"level":"info","ts":1672808353.423182,"caller":"service/connect_service.go:108","msg":"metrics listening at 8014","component":"service"}
evalValue before {
          "in": ["@faas.com", {
            "var": ["email"]
          }]
    }
evalValue after 
          "in": ["@faas.com", {
            "var": ["email"]
          }]
   
{"level":"info","ts":1672808353.4240851,"caller":"runtime/runtime.go:84","msg":"configuration change (write) for flagKey myBoolFlag (./config/samples/example_flags.json)","component":"runtime"}
...

There is no error. Looking at the log above,

evalValue before {
          "in": ["@faas.com", {
            "var": ["email"]
          }]
    }
evalValue after 
          "in": ["@faas.com", {
            "var": ["email"]
          }]

Notice how only the trailing bracket on newline is removed (with the opening bracket on the first line). Resulting json is valid.

@vadasambar vadasambar added bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer labels Jan 4, 2023
@vadasambar
Copy link
Contributor Author

I am not sure if we want to fix this now or even fix this at all because we ran into this issue when we tried converting yaml to json (we might not have to do this anytime soon in the future) and we have a way to work around the issue using json.MarshalIndent instead of json.Marshal. I have created this issue for future reference in case anyone runs into a similar issue in the future.

@james-milligan james-milligan self-assigned this Jan 10, 2023
@beeme1mr beeme1mr linked a pull request Jan 10, 2023 that will close this issue
james-milligan added a commit that referenced this issue Jan 12, 2023
Signed-off-by: James Milligan <james@omnant.co.uk>

<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

- fixes transpose evaluator state parsing when state contains trailing
characters

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

#244

### Notes
<!-- any additional notes for this PR -->
switches from a hard coded `1:n-2` slice to use regex to match and
replace the first `{` and last `}` in the string.
### Follow-up Tasks
<!-- anything that is related to this PR but not done here should be
noted under this section -->
<!-- if there is a need for a new issue, please link it here -->

### How to test
<!-- if applicable, add testing instructions under this section -->

Signed-off-by: James Milligan <james@omnant.co.uk>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants