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.

Sign up for GitHub

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

Jump to bottom

bugfix: API: encode empty Vector/Matrix as [] not null #13997

Merged
merged 7 commits into from
Apr 29, 2024
Merged

bugfix: API: encode empty Vector/Matrix as [] not null #13997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2c4a363
tests: API: simplify check of error response
bboreham Apr 28, 2024
5a339ba
tests: API: Use jsoniter when encoding
bboreham Apr 28, 2024
c8aed6b
tests: API: Let nil expected response mean skip check
bboreham Apr 28, 2024
66a1c3d
refactor: API: be explicit that we marshal empty objects
bboreham Apr 28, 2024
e0a00f4
refactor: API: separate typed and unsafe marshalling
bboreham Apr 28, 2024
00247b5
test: API: check empty responses
bboreham Apr 28, 2024
5c8ffaa
bugfix: API: encode empty Vector/Matrix as []
bboreham Apr 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 26 additions & 14 deletions web/api/v1/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package v1

import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
Expand All @@ -35,6 +34,7 @@ import (
"github.com/prometheus/prometheus/util/testutil"

"github.com/go-kit/log"
jsoniter "github.com/json-iterator/go"
"github.com/prometheus/client_golang/prometheus"
config_util "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
Expand Down Expand Up @@ -910,6 +910,7 @@ func TestStats(t *testing.T) {
require.IsType(t, &QueryData{}, i)
qd := i.(*QueryData)
require.NotNil(t, qd.Stats)
json := jsoniter.ConfigCompatibleWithStandardLibrary
j, err := json.Marshal(qd.Stats)
require.NoError(t, err)
require.JSONEq(t, `{"custom":"Custom Value"}`, string(j))
Expand Down Expand Up @@ -1171,6 +1172,25 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
},
},
},
// Test empty vector result
{
endpoint: api.query,
query: url.Values{
"query": []string{"bottomk(2, notExists)"},
},
responseAsJSON: `{"resultType":"vector","result":[]}`,
},
// Test empty matrix result
{
endpoint: api.queryRange,
query: url.Values{
"query": []string{"bottomk(2, notExists)"},
"start": []string{"0"},
"end": []string{"2"},
"step": []string{"1"},
},
responseAsJSON: `{"resultType":"matrix","result":[]}`,
},
// Missing query params in range queries.
{
endpoint: api.queryRange,
Expand Down Expand Up @@ -2891,10 +2911,13 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
if test.zeroFunc != nil {
test.zeroFunc(res.data)
}
assertAPIResponse(t, res.data, test.response)
if test.response != nil {
assertAPIResponse(t, res.data, test.response)
}
}

if test.responseAsJSON != "" {
json := jsoniter.ConfigCompatibleWithStandardLibrary
s, err := json.Marshal(res.data)
require.NoError(t, err)
require.JSONEq(t, test.responseAsJSON, string(s))
Expand Down Expand Up @@ -3292,18 +3315,7 @@ func TestRespondError(t *testing.T) {
require.Equal(t, want, have, "Return code %d expected in error response but got %d", want, have)
h := resp.Header.Get("Content-Type")
require.Equal(t, "application/json", h, "Expected Content-Type %q but got %q", "application/json", h)

var res Response
err = json.Unmarshal(body, &res)
require.NoError(t, err, "Error unmarshaling JSON body")

exp := &Response{
Status: statusError,
Data: "test",
ErrorType: errorTimeout,
Error: "message",
}
require.Equal(t, exp, &res)
require.JSONEq(t, `{"status": "error", "data": "test", "errorType": "timeout", "error": "message"}`, string(body))
}

func TestParseTimeParam(t *testing.T) {
Expand Down
75 changes: 51 additions & 24 deletions web/api/v1/json_codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ import (
)

func init() {
jsoniter.RegisterTypeEncoderFunc("promql.Series", marshalSeriesJSON, marshalSeriesJSONIsEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.Sample", marshalSampleJSON, marshalSampleJSONIsEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.FPoint", marshalFPointJSON, marshalPointJSONIsEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.HPoint", marshalHPointJSON, marshalPointJSONIsEmpty)
jsoniter.RegisterTypeEncoderFunc("exemplar.Exemplar", marshalExemplarJSON, marshalExemplarJSONEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.Vector", unsafeMarshalVectorJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.Matrix", unsafeMarshalMatrixJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.Series", unsafeMarshalSeriesJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.Sample", unsafeMarshalSampleJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.FPoint", unsafeMarshalFPointJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("promql.HPoint", unsafeMarshalHPointJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("exemplar.Exemplar", marshalExemplarJSON, neverEmpty)
jsoniter.RegisterTypeEncoderFunc("labels.Labels", unsafeMarshalLabelsJSON, labelsIsEmpty)
}

Expand Down Expand Up @@ -66,8 +68,12 @@ func (j JSONCodec) Encode(resp *Response) ([]byte, error) {
// < more histograms >
// ],
// },
func marshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
func unsafeMarshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
s := *((*promql.Series)(ptr))
marshalSeriesJSON(s, stream)
}

func marshalSeriesJSON(s promql.Series, stream *jsoniter.Stream) {
stream.WriteObjectStart()
stream.WriteObjectField(`metric`)
marshalLabelsJSON(s.Metric, stream)
Expand All @@ -78,7 +84,7 @@ func marshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
stream.WriteObjectField(`values`)
stream.WriteArrayStart()
}
marshalFPointJSON(unsafe.Pointer(&p), stream)
marshalFPointJSON(p, stream)
}
if len(s.Floats) > 0 {
stream.WriteArrayEnd()
Expand All @@ -89,15 +95,16 @@ func marshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
stream.WriteObjectField(`histograms`)
stream.WriteArrayStart()
}
marshalHPointJSON(unsafe.Pointer(&p), stream)
marshalHPointJSON(p, stream)
}
if len(s.Histograms) > 0 {
stream.WriteArrayEnd()
}
stream.WriteObjectEnd()
}

func marshalSeriesJSONIsEmpty(unsafe.Pointer) bool {
// In the Prometheus API we render an empty object as `[]` or similar.
func neverEmpty(unsafe.Pointer) bool {
return false
}

Expand All @@ -122,8 +129,12 @@ func marshalSeriesJSONIsEmpty(unsafe.Pointer) bool {
// },
// "histogram": [ 1435781451.781, { < histogram, see jsonutil.MarshalHistogram > } ]
// },
func marshalSampleJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
func unsafeMarshalSampleJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
s := *((*promql.Sample)(ptr))
marshalSampleJSON(s, stream)
}

func marshalSampleJSON(s promql.Sample, stream *jsoniter.Stream) {
stream.WriteObjectStart()
stream.WriteObjectField(`metric`)
marshalLabelsJSON(s.Metric, stream)
Expand All @@ -145,13 +156,13 @@ func marshalSampleJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
stream.WriteObjectEnd()
}

func marshalSampleJSONIsEmpty(unsafe.Pointer) bool {
return false
}

// marshalFPointJSON writes `[ts, "1.234"]`.
func marshalFPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
func unsafeMarshalFPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
p := *((*promql.FPoint)(ptr))
marshalFPointJSON(p, stream)
}

func marshalFPointJSON(p promql.FPoint, stream *jsoniter.Stream) {
stream.WriteArrayStart()
jsonutil.MarshalTimestamp(p.T, stream)
stream.WriteMore()
Expand All @@ -160,19 +171,19 @@ func marshalFPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
}

// marshalHPointJSON writes `[ts, { < histogram, see jsonutil.MarshalHistogram > } ]`.
func marshalHPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
func unsafeMarshalHPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
p := *((*promql.HPoint)(ptr))
marshalHPointJSON(p, stream)
}

func marshalHPointJSON(p promql.HPoint, stream *jsoniter.Stream) {
stream.WriteArrayStart()
jsonutil.MarshalTimestamp(p.T, stream)
stream.WriteMore()
jsonutil.MarshalHistogram(p.H, stream)
stream.WriteArrayEnd()
}

func marshalPointJSONIsEmpty(unsafe.Pointer) bool {
return false
}

// marshalExemplarJSON writes.
//
// {
Expand Down Expand Up @@ -201,10 +212,6 @@ func marshalExemplarJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
stream.WriteObjectEnd()
}

func marshalExemplarJSONEmpty(unsafe.Pointer) bool {
return false
}

func unsafeMarshalLabelsJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
labelsPtr := (*labels.Labels)(ptr)
marshalLabelsJSON(*labelsPtr, stream)
Expand All @@ -229,3 +236,23 @@ func labelsIsEmpty(ptr unsafe.Pointer) bool {
labelsPtr := (*labels.Labels)(ptr)
return labelsPtr.IsEmpty()
}

// Marshal a Vector as `[sample,sample,...]` - empty Vector is `[]`.
func unsafeMarshalVectorJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
v := *((*promql.Vector)(ptr))
stream.WriteArrayStart()
for _, s := range v {
marshalSampleJSON(s, stream)
}
stream.WriteArrayEnd()
}

// Marshal a Matrix as `[series,series,...]` - empty Matrix is `[]`.
func unsafeMarshalMatrixJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
m := *((*promql.Matrix)(ptr))
stream.WriteArrayStart()
for _, s := range m {
marshalSeriesJSON(s, stream)
}
stream.WriteArrayEnd()
}