From 2c4a36376d6522f6d82b756762b539b10b9b1ab6 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 19:57:48 +0100 Subject: [PATCH 1/7] tests: API: simplify check of error response Since we already use require.JSONEq in similar cases. Signed-off-by: Bryan Boreham --- web/api/v1/api_test.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index c383993815..9c45fd5d54 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -3292,18 +3292,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) { From 5a339ba3590d95477e729c2e293e61b670f5f1cd Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 20:02:18 +0100 Subject: [PATCH 2/7] tests: API: Use jsoniter when encoding So that tests use the same encoding as the api. Signed-off-by: Bryan Boreham --- web/api/v1/api_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 9c45fd5d54..dae408bcab 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -15,7 +15,6 @@ package v1 import ( "context" - "encoding/json" "errors" "fmt" "io" @@ -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" @@ -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)) @@ -2895,6 +2896,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E } if test.responseAsJSON != "" { + json := jsoniter.ConfigCompatibleWithStandardLibrary s, err := json.Marshal(res.data) require.NoError(t, err) require.JSONEq(t, test.responseAsJSON, string(s)) From c8aed6b0ecc2e60cb31e7169cfd57eaf3ccb6dd7 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 20:03:51 +0100 Subject: [PATCH 3/7] tests: API: Let nil expected response mean skip check When we want to check just the json encoding. Signed-off-by: Bryan Boreham --- web/api/v1/api_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index dae408bcab..044cd171db 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -2892,7 +2892,9 @@ 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 != "" { From 66a1c3daad625c00cebbe688e2d62d0f311c355b Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 20:26:51 +0100 Subject: [PATCH 4/7] refactor: API: be explicit that we marshal empty objects Signed-off-by: Bryan Boreham --- web/api/v1/json_codec.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go index f1a8104cc4..d30429706f 100644 --- a/web/api/v1/json_codec.go +++ b/web/api/v1/json_codec.go @@ -25,11 +25,11 @@ 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.Series", marshalSeriesJSON, neverEmpty) + jsoniter.RegisterTypeEncoderFunc("promql.Sample", marshalSampleJSON, neverEmpty) + jsoniter.RegisterTypeEncoderFunc("promql.FPoint", marshalFPointJSON, neverEmpty) + jsoniter.RegisterTypeEncoderFunc("promql.HPoint", marshalHPointJSON, neverEmpty) + jsoniter.RegisterTypeEncoderFunc("exemplar.Exemplar", marshalExemplarJSON, neverEmpty) jsoniter.RegisterTypeEncoderFunc("labels.Labels", unsafeMarshalLabelsJSON, labelsIsEmpty) } @@ -97,7 +97,8 @@ func marshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { 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 } @@ -145,10 +146,6 @@ 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) { p := *((*promql.FPoint)(ptr)) @@ -169,10 +166,6 @@ func marshalHPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { stream.WriteArrayEnd() } -func marshalPointJSONIsEmpty(unsafe.Pointer) bool { - return false -} - // marshalExemplarJSON writes. // // { @@ -201,10 +194,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) From e0a00f45db839a7f2f1e83895a815f74b5706e9a Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 20:29:03 +0100 Subject: [PATCH 5/7] refactor: API: separate typed and unsafe marshalling The typed versions are used when we call from one marshaller to another. Signed-off-by: Bryan Boreham --- web/api/v1/json_codec.go | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go index d30429706f..df3af66f0d 100644 --- a/web/api/v1/json_codec.go +++ b/web/api/v1/json_codec.go @@ -25,10 +25,10 @@ import ( ) func init() { - jsoniter.RegisterTypeEncoderFunc("promql.Series", marshalSeriesJSON, neverEmpty) - jsoniter.RegisterTypeEncoderFunc("promql.Sample", marshalSampleJSON, neverEmpty) - jsoniter.RegisterTypeEncoderFunc("promql.FPoint", marshalFPointJSON, neverEmpty) - jsoniter.RegisterTypeEncoderFunc("promql.HPoint", marshalHPointJSON, 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) } @@ -66,8 +66,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) @@ -78,7 +82,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() @@ -89,7 +93,7 @@ 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() @@ -123,8 +127,12 @@ func neverEmpty(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) @@ -147,8 +155,12 @@ func marshalSampleJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { } // 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() @@ -157,8 +169,12 @@ 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() From 00247b5d87d599a96acdcfb1c84c0bf99a7ac16f Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 20:33:52 +0100 Subject: [PATCH 6/7] test: API: check empty responses Signed-off-by: Bryan Boreham --- web/api/v1/api_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 044cd171db..bb2a73f6db 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -1172,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, From 5c8ffaa77ccf53a813e890066491a23f4965dd3f Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Apr 2024 20:35:01 +0100 Subject: [PATCH 7/7] bugfix: API: encode empty Vector/Matrix as [] If the underlying data is `nil` the default encoding will render `"null"` which is not accepted by (some) Prometheus client libraries. Signed-off-by: Bryan Boreham --- web/api/v1/json_codec.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go index df3af66f0d..dfcdf78f8a 100644 --- a/web/api/v1/json_codec.go +++ b/web/api/v1/json_codec.go @@ -25,6 +25,8 @@ import ( ) func init() { + 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) @@ -234,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() +}