bugfix: add missing comma in vector/matrix array

[amirvejahat]

Fixes #14040
Fixes #14011

[ArthurSens]

We require that all commits are properly signed, could you run git commit --amend -s and force push again?

Also, we want to include this fix on the release branch release-2.52. Could you update the base branch for this PR?

[bboreham]

Ideally we would add a test which failed on the previous code too.

[amirvejahat]

We require that all commits are properly signed, could you run git commit --amend -s and force push again?

Also, we want to include this fix on the release branch release-2.52. Could you update the base branch for this PR?

Sure, will do

[ArthurSens]

I've tested the change locally and it works, but it looks like you've got some extra commits in here by accident 😅

What you wanna do is:

• Clone the repo and checkout the branch release-2.52
• Create a new branch from release-2.52
• Add your changes in a commit (remember to sign, using the -s flag)
• Open a Pull Request with release-2.52 as the base branch.

You can do this in a new PR, or you can force push commits over here. It's up to you

[amirvejahat]

Thanks for the help @ArthurSens . I've fixed the branching issue. please note that circleci has failed due to project suspension, I'm not sure if there's anything I could do to make it work.

[ArthurSens]

Thanks @amirvejahat, one last thing.

As Bryan mentioned, it would be nice to have a test case to make sure we don't make this mistake again in the future.

You can add a new test case in web/api/v1/json_codec.go, inside the test TestJsonCodec_Encode

{
			response: &QueryData{
				ResultType: parser.ValueTypeVector,
				Result: promql.Vector{
					promql.Sample{
						Metric: labels.FromStrings("__name__", "foo"),
						T:      1000,
						F:      1,
					},
					promql.Sample{
						Metric: labels.FromStrings("__name__", "bar"),
						T:      2000,
						F:      2,
					},
				},
			},
			expected: `{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"foo"},"value":[1,"1"]},{"metric":{"__name__":"bar"},"value":[2,"2"]}]}}`,
		},

[ArthurSens]

Thanks for the help @ArthurSens . I've fixed the branching issue. please note that circleci has failed due to project suspension, I'm not sure if there's anything I could do to make it work.

Yeah, the failing CI is not related to this PR... no need to worry about it :)

[gotjosh]

Hi @amirvejahat at the moment this is breaking main so it's rather sensitive - I'll aim to add some tests in the next 30m unless you respond first so that we can keep going.

Thank you again for your contribution!

[ArthurSens]

@gotjosh , I've just added the test. Do you mind reviewing it?

EDIT: Looks like another test could be added for matrixes 🤔

[gotjosh]

LGTM

Thank you @ArthurSens!

[ArthurSens]

Thanks @amirvejahat for the fix and sorry for taking over the PR, we just wanted to get this fixed soon :)

[gotjosh]

This now needs to be merged back into main.

[ArthurSens]

Yeah, I've just created the PR to prepare the new release and another PR merging it back to main will follow shortly after

[amirvejahat]

Thanks @ArthurSens for adding the test case, I was away from my keyboard. glad to see it's merged now. looking forward to future contribution.