From 3e94dd8c8f83a11c4ac30e5a748b96adbc85cb40 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Wed, 25 Jan 2023 14:30:47 +1100 Subject: [PATCH 1/5] Add extension point for returning different content types from API endpoints Signed-off-by: Charles Korn --- web/api/v1/api.go | 64 ++++++-- web/api/v1/api_test.go | 290 ++++++++++++---------------------- web/api/v1/codec.go | 26 +++ web/api/v1/json_codec.go | 32 ++++ web/api/v1/json_codec_test.go | 178 +++++++++++++++++++++ 5 files changed, 391 insertions(+), 199 deletions(-) create mode 100644 web/api/v1/codec.go create mode 100644 web/api/v1/json_codec.go create mode 100644 web/api/v1/json_codec_test.go diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 894a8666a6..525bc446ae 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -80,6 +80,8 @@ const ( var LocalhostRepresentations = []string{"127.0.0.1", "localhost", "::1"} +var defaultCodec = JSONCodec{} + type apiError struct { typ errorType err error @@ -145,7 +147,8 @@ type RuntimeInfo struct { StorageRetention string `json:"storageRetention"` } -type response struct { +// Response contains a response to a HTTP API request. +type Response struct { Status status `json:"status"` Data interface{} `json:"data,omitempty"` ErrorType errorType `json:"errorType,omitempty"` @@ -208,6 +211,8 @@ type API struct { remoteWriteHandler http.Handler remoteReadHandler http.Handler + + codecs map[string]Codec } func init() { @@ -273,8 +278,12 @@ func NewAPI( statsRenderer: defaultStatsRenderer, remoteReadHandler: remote.NewReadHandler(logger, registerer, q, configFunc, remoteReadSampleLimit, remoteReadConcurrencyLimit, remoteReadMaxBytesInFrame), + + codecs: map[string]Codec{}, } + a.InstallCodec(defaultCodec) + if statsRenderer != nil { a.statsRenderer = statsRenderer } @@ -286,6 +295,16 @@ func NewAPI( return a } +// InstallCodec adds codec to this API's available codecs. +// If codec handles a content type handled by a codec already installed in this API, codec replaces the previous codec. +func (api *API) InstallCodec(codec Codec) { + if api.codecs == nil { + api.codecs = map[string]Codec{} + } + + api.codecs[codec.ContentType()] = codec +} + func setUnavailStatusOnTSDBNotReady(r apiFuncResult) apiFuncResult { if r.err != nil && errors.Cause(r.err.err) == tsdb.ErrNotReady { r.err.typ = errorUnavailable @@ -308,7 +327,7 @@ func (api *API) Register(r *route.Router) { } if result.data != nil { - api.respond(w, result.data, result.warnings) + api.respond(w, r, result.data, result.warnings) return } w.WriteHeader(http.StatusNoContent) @@ -1446,7 +1465,7 @@ func (api *API) serveWALReplayStatus(w http.ResponseWriter, r *http.Request) { if err != nil { api.respondError(w, &apiError{errorInternal, err}, nil) } - api.respond(w, walReplayStatus{ + api.respond(w, r, walReplayStatus{ Min: status.Min, Max: status.Max, Current: status.Current, @@ -1548,34 +1567,59 @@ func (api *API) cleanTombstones(r *http.Request) apiFuncResult { return apiFuncResult{nil, nil, nil, nil} } -func (api *API) respond(w http.ResponseWriter, data interface{}, warnings storage.Warnings) { +func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface{}, warnings storage.Warnings) { statusMessage := statusSuccess var warningStrings []string for _, warning := range warnings { warningStrings = append(warningStrings, warning.Error()) } - json := jsoniter.ConfigCompatibleWithStandardLibrary - b, err := json.Marshal(&response{ + + resp := &Response{ Status: statusMessage, Data: data, Warnings: warningStrings, - }) + } + + codec := api.negotiateCodec(req, resp) + b, err := codec.Encode(resp) if err != nil { - level.Error(api.logger).Log("msg", "error marshaling json response", "err", err) + level.Error(api.logger).Log("msg", "error marshaling response", "err", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Type", codec.ContentType()) w.WriteHeader(http.StatusOK) if n, err := w.Write(b); err != nil { level.Error(api.logger).Log("msg", "error writing response", "bytesWritten", n, "err", err) } } +// HTTP content negotiation is hard (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation). +// Ideally, we shouldn't be implementing this ourselves - https://github.com/golang/go/issues/19307 is an open proposal to add +// this to the Go stdlib and has links to a number of other implementations. +// +// This is an MVP, and doesn't support features like wildcards or weighting. +func (api *API) negotiateCodec(req *http.Request, resp *Response) Codec { + acceptHeader := req.Header.Get("Accept") + if acceptHeader == "" { + return defaultCodec + } + + for _, contentType := range strings.Split(acceptHeader, ",") { + codec, ok := api.codecs[strings.TrimSpace(contentType)] + if ok && codec.CanEncode(resp) { + return codec + } + } + + level.Warn(api.logger).Log("msg", "could not find suitable codec for response, falling back to default codec", "accept_header", acceptHeader) + return defaultCodec +} + func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) { json := jsoniter.ConfigCompatibleWithStandardLibrary - b, err := json.Marshal(&response{ + b, err := json.Marshal(&Response{ Status: statusError, ErrorType: apiErr.typ, Error: apiErr.err.Error(), diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 7e2dcbd8bb..617a8bdf3c 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -18,7 +18,6 @@ import ( "encoding/json" "fmt" "io" - "math" "net/http" "net/http/httptest" "net/url" @@ -30,7 +29,6 @@ import ( "testing" "time" - "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/util/stats" @@ -2765,39 +2763,93 @@ func TestAdminEndpoints(t *testing.T) { } func TestRespondSuccess(t *testing.T) { + api := API{ + logger: log.NewNopLogger(), + } + + api.InstallCodec(&testCodec{contentType: "test/cannot-encode", canEncode: false}) + api.InstallCodec(&testCodec{contentType: "test/can-encode", canEncode: true}) + api.InstallCodec(&testCodec{contentType: "test/can-encode-2", canEncode: true}) + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - api := API{} - api.respond(w, "test", nil) + api.respond(w, r, "test", nil) })) defer s.Close() - resp, err := http.Get(s.URL) - if err != nil { - t.Fatalf("Error on test request: %s", err) - } - body, err := io.ReadAll(resp.Body) - defer resp.Body.Close() - if err != nil { - t.Fatalf("Error reading response body: %s", err) - } + for _, tc := range []struct { + name string + acceptHeader string + expectedContentType string + expectedBody string + }{ + { + name: "no Accept header", + expectedContentType: "application/json", + expectedBody: `{"status":"success","data":"test"}`, + }, + { + name: "Accept header with single content type which is suitable", + acceptHeader: "test/can-encode", + expectedContentType: "test/can-encode", + expectedBody: `response from test/can-encode codec`, + }, + { + name: "Accept header with single content type which is not available", + acceptHeader: "test/not-registered", + expectedContentType: "application/json", + expectedBody: `{"status":"success","data":"test"}`, + }, + { + name: "Accept header with single content type which cannot encode the response payload", + acceptHeader: "test/cannot-encode", + expectedContentType: "application/json", + expectedBody: `{"status":"success","data":"test"}`, + }, + { + name: "Accept header with multiple content types, all of which are suitable", + acceptHeader: "test/can-encode, test/can-encode-2", + expectedContentType: "test/can-encode", + expectedBody: `response from test/can-encode codec`, + }, + { + name: "Accept header with multiple content types, only one of which is available", + acceptHeader: "test/not-registered, test/can-encode", + expectedContentType: "test/can-encode", + expectedBody: `response from test/can-encode codec`, + }, + { + name: "Accept header with multiple content types, only one of which can encode the response payload", + acceptHeader: "test/cannot-encode, test/can-encode", + expectedContentType: "test/can-encode", + expectedBody: `response from test/can-encode codec`, + }, + { + name: "Accept header with multiple content types, none of which are available", + acceptHeader: "test/not-registered, test/also-not-registered", + expectedContentType: "application/json", + expectedBody: `{"status":"success","data":"test"}`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, s.URL, nil) + require.NoError(t, err) - if resp.StatusCode != 200 { - t.Fatalf("Return code %d expected in success response but got %d", 200, resp.StatusCode) - } - if h := resp.Header.Get("Content-Type"); h != "application/json" { - t.Fatalf("Expected Content-Type %q but got %q", "application/json", h) - } + if tc.acceptHeader != "" { + req.Header.Set("Accept", tc.acceptHeader) + } - var res response - if err = json.Unmarshal([]byte(body), &res); err != nil { - t.Fatalf("Error unmarshaling JSON body: %s", err) - } + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) - exp := &response{ - Status: statusSuccess, - Data: "test", + body, err := io.ReadAll(resp.Body) + defer resp.Body.Close() + require.NoError(t, err) + + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, tc.expectedContentType, resp.Header.Get("Content-Type")) + require.Equal(t, tc.expectedBody, string(body)) + }) } - require.Equal(t, exp, &res) } func TestRespondError(t *testing.T) { @@ -2824,12 +2876,12 @@ func TestRespondError(t *testing.T) { t.Fatalf("Expected Content-Type %q but got %q", "application/json", h) } - var res response + var res Response if err = json.Unmarshal([]byte(body), &res); err != nil { t.Fatalf("Error unmarshaling JSON body: %s", err) } - exp := &response{ + exp := &Response{ Status: statusError, Data: "test", ErrorType: errorTimeout, @@ -3047,165 +3099,6 @@ func TestOptionsMethod(t *testing.T) { } } -func TestRespond(t *testing.T) { - cases := []struct { - response interface{} - expected string - }{ - { - response: &queryData{ - ResultType: parser.ValueTypeMatrix, - Result: promql.Matrix{ - promql.Series{ - Points: []promql.Point{{V: 1, T: 1000}}, - Metric: labels.FromStrings("__name__", "foo"), - }, - }, - }, - expected: `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"foo"},"values":[[1,"1"]]}]}}`, - }, - { - response: &queryData{ - ResultType: parser.ValueTypeMatrix, - Result: promql.Matrix{ - promql.Series{ - Points: []promql.Point{{H: &histogram.FloatHistogram{ - Schema: 2, - ZeroThreshold: 0.001, - ZeroCount: 12, - Count: 10, - Sum: 20, - PositiveSpans: []histogram.Span{ - {Offset: 3, Length: 2}, - {Offset: 1, Length: 3}, - }, - NegativeSpans: []histogram.Span{ - {Offset: 2, Length: 2}, - }, - PositiveBuckets: []float64{1, 2, 2, 1, 1}, - NegativeBuckets: []float64{2, 1}, - }, T: 1000}}, - Metric: labels.FromStrings("__name__", "foo"), - }, - }, - }, - expected: `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"foo"},"histograms":[[1,{"count":"10","sum":"20","buckets":[[1,"-1.6817928305074288","-1.414213562373095","1"],[1,"-1.414213562373095","-1.189207115002721","2"],[3,"-0.001","0.001","12"],[0,"1.414213562373095","1.6817928305074288","1"],[0,"1.6817928305074288","2","2"],[0,"2.378414230005442","2.82842712474619","2"],[0,"2.82842712474619","3.3635856610148576","1"],[0,"3.3635856610148576","4","1"]]}]]}]}}`, - }, - { - response: promql.Point{V: 0, T: 0}, - expected: `{"status":"success","data":[0,"0"]}`, - }, - { - response: promql.Point{V: 20, T: 1}, - expected: `{"status":"success","data":[0.001,"20"]}`, - }, - { - response: promql.Point{V: 20, T: 10}, - expected: `{"status":"success","data":[0.010,"20"]}`, - }, - { - response: promql.Point{V: 20, T: 100}, - expected: `{"status":"success","data":[0.100,"20"]}`, - }, - { - response: promql.Point{V: 20, T: 1001}, - expected: `{"status":"success","data":[1.001,"20"]}`, - }, - { - response: promql.Point{V: 20, T: 1010}, - expected: `{"status":"success","data":[1.010,"20"]}`, - }, - { - response: promql.Point{V: 20, T: 1100}, - expected: `{"status":"success","data":[1.100,"20"]}`, - }, - { - response: promql.Point{V: 20, T: 12345678123456555}, - expected: `{"status":"success","data":[12345678123456.555,"20"]}`, - }, - { - response: promql.Point{V: 20, T: -1}, - expected: `{"status":"success","data":[-0.001,"20"]}`, - }, - { - response: promql.Point{V: math.NaN(), T: 0}, - expected: `{"status":"success","data":[0,"NaN"]}`, - }, - { - response: promql.Point{V: math.Inf(1), T: 0}, - expected: `{"status":"success","data":[0,"+Inf"]}`, - }, - { - response: promql.Point{V: math.Inf(-1), T: 0}, - expected: `{"status":"success","data":[0,"-Inf"]}`, - }, - { - response: promql.Point{V: 1.2345678e6, T: 0}, - expected: `{"status":"success","data":[0,"1234567.8"]}`, - }, - { - response: promql.Point{V: 1.2345678e-6, T: 0}, - expected: `{"status":"success","data":[0,"0.0000012345678"]}`, - }, - { - response: promql.Point{V: 1.2345678e-67, T: 0}, - expected: `{"status":"success","data":[0,"1.2345678e-67"]}`, - }, - { - response: []exemplar.QueryResult{ - { - SeriesLabels: labels.FromStrings("foo", "bar"), - Exemplars: []exemplar.Exemplar{ - { - Labels: labels.FromStrings("traceID", "abc"), - Value: 100.123, - Ts: 1234, - }, - }, - }, - }, - expected: `{"status":"success","data":[{"seriesLabels":{"foo":"bar"},"exemplars":[{"labels":{"traceID":"abc"},"value":"100.123","timestamp":1.234}]}]}`, - }, - { - response: []exemplar.QueryResult{ - { - SeriesLabels: labels.FromStrings("foo", "bar"), - Exemplars: []exemplar.Exemplar{ - { - Labels: labels.FromStrings("traceID", "abc"), - Value: math.Inf(1), - Ts: 1234, - }, - }, - }, - }, - expected: `{"status":"success","data":[{"seriesLabels":{"foo":"bar"},"exemplars":[{"labels":{"traceID":"abc"},"value":"+Inf","timestamp":1.234}]}]}`, - }, - } - - for _, c := range cases { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - api := API{} - api.respond(w, c.response, nil) - })) - defer s.Close() - - resp, err := http.Get(s.URL) - if err != nil { - t.Fatalf("Error on test request: %s", err) - } - body, err := io.ReadAll(resp.Body) - defer resp.Body.Close() - if err != nil { - t.Fatalf("Error reading response body: %s", err) - } - - if string(body) != c.expected { - t.Fatalf("Expected response \n%v\n but got \n%v\n", c.expected, string(body)) - } - } -} - func TestTSDBStatus(t *testing.T) { tsdb := &fakeDB{} tsdbStatusAPI := func(api *API) apiFunc { return api.serveTSDBStatus } @@ -3281,6 +3174,8 @@ var testResponseWriter = httptest.ResponseRecorder{} func BenchmarkRespond(b *testing.B) { b.ReportAllocs() + request, err := http.NewRequest(http.MethodGet, "/does-not-matter", nil) + require.NoError(b, err) points := []promql.Point{} for i := 0; i < 10000; i++ { points = append(points, promql.Point{V: float64(i * 1000000), T: int64(i)}) @@ -3297,7 +3192,7 @@ func BenchmarkRespond(b *testing.B) { b.ResetTimer() api := API{} for n := 0; n < b.N; n++ { - api.respond(&testResponseWriter, response, nil) + api.respond(&testResponseWriter, request, response, nil) } } @@ -3408,3 +3303,20 @@ func TestGetGlobalURL(t *testing.T) { }) } } + +type testCodec struct { + contentType string + canEncode bool +} + +func (t *testCodec) ContentType() string { + return t.contentType +} + +func (t *testCodec) CanEncode(_ *Response) bool { + return t.canEncode +} + +func (t *testCodec) Encode(_ *Response) ([]byte, error) { + return []byte(fmt.Sprintf("response from %v codec", t.contentType)), nil +} diff --git a/web/api/v1/codec.go b/web/api/v1/codec.go new file mode 100644 index 0000000000..d11bb1fa01 --- /dev/null +++ b/web/api/v1/codec.go @@ -0,0 +1,26 @@ +// Copyright 2016 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1 + +// A Codec performs encoding of API responses. +type Codec interface { + // ContentType returns the MIME time that this Codec emits. + ContentType() string + + // CanEncode determines if this Codec can encode resp. + CanEncode(resp *Response) bool + + // Encode encodes resp, ready for transmission to an API consumer. + Encode(resp *Response) ([]byte, error) +} diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go new file mode 100644 index 0000000000..b38dab0385 --- /dev/null +++ b/web/api/v1/json_codec.go @@ -0,0 +1,32 @@ +// Copyright 2016 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1 + +import jsoniter "github.com/json-iterator/go" + +// JSONCodec is a Codec that encodes API responses as JSON. +type JSONCodec struct{} + +func (j JSONCodec) ContentType() string { + return "application/json" +} + +func (j JSONCodec) CanEncode(_ *Response) bool { + return true +} + +func (j JSONCodec) Encode(resp *Response) ([]byte, error) { + json := jsoniter.ConfigCompatibleWithStandardLibrary + return json.Marshal(resp) +} diff --git a/web/api/v1/json_codec_test.go b/web/api/v1/json_codec_test.go new file mode 100644 index 0000000000..c5b030ff9a --- /dev/null +++ b/web/api/v1/json_codec_test.go @@ -0,0 +1,178 @@ +// Copyright 2016 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1 + +import ( + "math" + "testing" + + "github.com/prometheus/prometheus/model/exemplar" + "github.com/prometheus/prometheus/model/histogram" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/promql/parser" +) + +func TestJsonCodec_Encode(t *testing.T) { + cases := []struct { + response interface{} + expected string + }{ + { + response: &queryData{ + ResultType: parser.ValueTypeMatrix, + Result: promql.Matrix{ + promql.Series{ + Points: []promql.Point{{V: 1, T: 1000}}, + Metric: labels.FromStrings("__name__", "foo"), + }, + }, + }, + expected: `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"foo"},"values":[[1,"1"]]}]}}`, + }, + { + response: &queryData{ + ResultType: parser.ValueTypeMatrix, + Result: promql.Matrix{ + promql.Series{ + Points: []promql.Point{{H: &histogram.FloatHistogram{ + Schema: 2, + ZeroThreshold: 0.001, + ZeroCount: 12, + Count: 10, + Sum: 20, + PositiveSpans: []histogram.Span{ + {Offset: 3, Length: 2}, + {Offset: 1, Length: 3}, + }, + NegativeSpans: []histogram.Span{ + {Offset: 2, Length: 2}, + }, + PositiveBuckets: []float64{1, 2, 2, 1, 1}, + NegativeBuckets: []float64{2, 1}, + }, T: 1000}}, + Metric: labels.FromStrings("__name__", "foo"), + }, + }, + }, + expected: `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"foo"},"histograms":[[1,{"count":"10","sum":"20","buckets":[[1,"-1.6817928305074288","-1.414213562373095","1"],[1,"-1.414213562373095","-1.189207115002721","2"],[3,"-0.001","0.001","12"],[0,"1.414213562373095","1.6817928305074288","1"],[0,"1.6817928305074288","2","2"],[0,"2.378414230005442","2.82842712474619","2"],[0,"2.82842712474619","3.3635856610148576","1"],[0,"3.3635856610148576","4","1"]]}]]}]}}`, + }, + { + response: promql.Point{V: 0, T: 0}, + expected: `{"status":"success","data":[0,"0"]}`, + }, + { + response: promql.Point{V: 20, T: 1}, + expected: `{"status":"success","data":[0.001,"20"]}`, + }, + { + response: promql.Point{V: 20, T: 10}, + expected: `{"status":"success","data":[0.010,"20"]}`, + }, + { + response: promql.Point{V: 20, T: 100}, + expected: `{"status":"success","data":[0.100,"20"]}`, + }, + { + response: promql.Point{V: 20, T: 1001}, + expected: `{"status":"success","data":[1.001,"20"]}`, + }, + { + response: promql.Point{V: 20, T: 1010}, + expected: `{"status":"success","data":[1.010,"20"]}`, + }, + { + response: promql.Point{V: 20, T: 1100}, + expected: `{"status":"success","data":[1.100,"20"]}`, + }, + { + response: promql.Point{V: 20, T: 12345678123456555}, + expected: `{"status":"success","data":[12345678123456.555,"20"]}`, + }, + { + response: promql.Point{V: 20, T: -1}, + expected: `{"status":"success","data":[-0.001,"20"]}`, + }, + { + response: promql.Point{V: math.NaN(), T: 0}, + expected: `{"status":"success","data":[0,"NaN"]}`, + }, + { + response: promql.Point{V: math.Inf(1), T: 0}, + expected: `{"status":"success","data":[0,"+Inf"]}`, + }, + { + response: promql.Point{V: math.Inf(-1), T: 0}, + expected: `{"status":"success","data":[0,"-Inf"]}`, + }, + { + response: promql.Point{V: 1.2345678e6, T: 0}, + expected: `{"status":"success","data":[0,"1234567.8"]}`, + }, + { + response: promql.Point{V: 1.2345678e-6, T: 0}, + expected: `{"status":"success","data":[0,"0.0000012345678"]}`, + }, + { + response: promql.Point{V: 1.2345678e-67, T: 0}, + expected: `{"status":"success","data":[0,"1.2345678e-67"]}`, + }, + { + response: []exemplar.QueryResult{ + { + SeriesLabels: labels.FromStrings("foo", "bar"), + Exemplars: []exemplar.Exemplar{ + { + Labels: labels.FromStrings("traceID", "abc"), + Value: 100.123, + Ts: 1234, + }, + }, + }, + }, + expected: `{"status":"success","data":[{"seriesLabels":{"foo":"bar"},"exemplars":[{"labels":{"traceID":"abc"},"value":"100.123","timestamp":1.234}]}]}`, + }, + { + response: []exemplar.QueryResult{ + { + SeriesLabels: labels.FromStrings("foo", "bar"), + Exemplars: []exemplar.Exemplar{ + { + Labels: labels.FromStrings("traceID", "abc"), + Value: math.Inf(1), + Ts: 1234, + }, + }, + }, + }, + expected: `{"status":"success","data":[{"seriesLabels":{"foo":"bar"},"exemplars":[{"labels":{"traceID":"abc"},"value":"+Inf","timestamp":1.234}]}]}`, + }, + } + + codec := JSONCodec{} + + for _, c := range cases { + body, err := codec.Encode(&Response{ + Status: statusSuccess, + Data: c.response, + }) + if err != nil { + t.Fatalf("Error encoding response body: %s", err) + } + + if string(body) != c.expected { + t.Fatalf("Expected response \n%v\n but got \n%v\n", c.expected, string(body)) + } + } +} From a0dd1468be609a0b4933ec094777f36e8d743e6c Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Tue, 31 Jan 2023 14:53:20 +1100 Subject: [PATCH 2/5] Move custom jsoniter code into json_codec.go. Signed-off-by: Charles Korn --- web/api/v1/api.go | 255 ------------------------------------- web/api/v1/json_codec.go | 262 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 261 insertions(+), 256 deletions(-) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 525bc446ae..e0ad76c5cb 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -27,7 +27,6 @@ import ( "strconv" "strings" "time" - "unsafe" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -40,8 +39,6 @@ import ( "golang.org/x/exp/slices" "github.com/prometheus/prometheus/config" - "github.com/prometheus/prometheus/model/exemplar" - "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/model/timestamp" @@ -54,7 +51,6 @@ import ( "github.com/prometheus/prometheus/tsdb" "github.com/prometheus/prometheus/tsdb/index" "github.com/prometheus/prometheus/util/httputil" - "github.com/prometheus/prometheus/util/jsonutil" "github.com/prometheus/prometheus/util/stats" ) @@ -215,13 +211,6 @@ type API struct { codecs map[string]Codec } -func init() { - jsoniter.RegisterTypeEncoderFunc("promql.Series", marshalSeriesJSON, marshalSeriesJSONIsEmpty) - jsoniter.RegisterTypeEncoderFunc("promql.Sample", marshalSampleJSON, marshalSampleJSONIsEmpty) - jsoniter.RegisterTypeEncoderFunc("promql.Point", marshalPointJSON, marshalPointJSONIsEmpty) - jsoniter.RegisterTypeEncoderFunc("exemplar.Exemplar", marshalExemplarJSON, marshalExemplarJSONEmpty) -} - // NewAPI returns an initialized API type. func NewAPI( qe QueryEngine, @@ -1724,247 +1713,3 @@ OUTER: } return matcherSets, nil } - -// marshalSeriesJSON writes something like the following: -// -// { -// "metric" : { -// "__name__" : "up", -// "job" : "prometheus", -// "instance" : "localhost:9090" -// }, -// "values": [ -// [ 1435781451.781, "1" ], -// < more values> -// ], -// "histograms": [ -// [ 1435781451.781, { < histogram, see below > } ], -// < more histograms > -// ], -// }, -func marshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { - s := *((*promql.Series)(ptr)) - stream.WriteObjectStart() - stream.WriteObjectField(`metric`) - m, err := s.Metric.MarshalJSON() - if err != nil { - stream.Error = err - return - } - stream.SetBuffer(append(stream.Buffer(), m...)) - - // We make two passes through the series here: In the first marshaling - // all value points, in the second marshaling all histogram - // points. That's probably cheaper than just one pass in which we copy - // out histogram Points into a newly allocated slice for separate - // marshaling. (Could be benchmarked, though.) - var foundValue, foundHistogram bool - for _, p := range s.Points { - if p.H == nil { - stream.WriteMore() - if !foundValue { - stream.WriteObjectField(`values`) - stream.WriteArrayStart() - } - foundValue = true - marshalPointJSON(unsafe.Pointer(&p), stream) - } else { - foundHistogram = true - } - } - if foundValue { - stream.WriteArrayEnd() - } - if foundHistogram { - firstHistogram := true - for _, p := range s.Points { - if p.H != nil { - stream.WriteMore() - if firstHistogram { - stream.WriteObjectField(`histograms`) - stream.WriteArrayStart() - } - firstHistogram = false - marshalPointJSON(unsafe.Pointer(&p), stream) - } - } - stream.WriteArrayEnd() - } - stream.WriteObjectEnd() -} - -func marshalSeriesJSONIsEmpty(ptr unsafe.Pointer) bool { - return false -} - -// marshalSampleJSON writes something like the following for normal value samples: -// -// { -// "metric" : { -// "__name__" : "up", -// "job" : "prometheus", -// "instance" : "localhost:9090" -// }, -// "value": [ 1435781451.781, "1" ] -// }, -// -// For histogram samples, it writes something like this: -// -// { -// "metric" : { -// "__name__" : "up", -// "job" : "prometheus", -// "instance" : "localhost:9090" -// }, -// "histogram": [ 1435781451.781, { < histogram, see below > } ] -// }, -func marshalSampleJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { - s := *((*promql.Sample)(ptr)) - stream.WriteObjectStart() - stream.WriteObjectField(`metric`) - m, err := s.Metric.MarshalJSON() - if err != nil { - stream.Error = err - return - } - stream.SetBuffer(append(stream.Buffer(), m...)) - stream.WriteMore() - if s.Point.H == nil { - stream.WriteObjectField(`value`) - } else { - stream.WriteObjectField(`histogram`) - } - marshalPointJSON(unsafe.Pointer(&s.Point), stream) - stream.WriteObjectEnd() -} - -func marshalSampleJSONIsEmpty(ptr unsafe.Pointer) bool { - return false -} - -// marshalPointJSON writes `[ts, "val"]`. -func marshalPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { - p := *((*promql.Point)(ptr)) - stream.WriteArrayStart() - jsonutil.MarshalTimestamp(p.T, stream) - stream.WriteMore() - if p.H == nil { - jsonutil.MarshalValue(p.V, stream) - } else { - marshalHistogram(p.H, stream) - } - stream.WriteArrayEnd() -} - -func marshalPointJSONIsEmpty(ptr unsafe.Pointer) bool { - return false -} - -// marshalHistogramJSON writes something like: -// -// { -// "count": "42", -// "sum": "34593.34", -// "buckets": [ -// [ 3, "-0.25", "0.25", "3"], -// [ 0, "0.25", "0.5", "12"], -// [ 0, "0.5", "1", "21"], -// [ 0, "2", "4", "6"] -// ] -// } -// -// The 1st element in each bucket array determines if the boundaries are -// inclusive (AKA closed) or exclusive (AKA open): -// -// 0: lower exclusive, upper inclusive -// 1: lower inclusive, upper exclusive -// 2: both exclusive -// 3: both inclusive -// -// The 2nd and 3rd elements are the lower and upper boundary. The 4th element is -// the bucket count. -func marshalHistogram(h *histogram.FloatHistogram, stream *jsoniter.Stream) { - stream.WriteObjectStart() - stream.WriteObjectField(`count`) - jsonutil.MarshalValue(h.Count, stream) - stream.WriteMore() - stream.WriteObjectField(`sum`) - jsonutil.MarshalValue(h.Sum, stream) - - bucketFound := false - it := h.AllBucketIterator() - for it.Next() { - bucket := it.At() - if bucket.Count == 0 { - continue // No need to expose empty buckets in JSON. - } - stream.WriteMore() - if !bucketFound { - stream.WriteObjectField(`buckets`) - stream.WriteArrayStart() - } - bucketFound = true - boundaries := 2 // Exclusive on both sides AKA open interval. - if bucket.LowerInclusive { - if bucket.UpperInclusive { - boundaries = 3 // Inclusive on both sides AKA closed interval. - } else { - boundaries = 1 // Inclusive only on lower end AKA right open. - } - } else { - if bucket.UpperInclusive { - boundaries = 0 // Inclusive only on upper end AKA left open. - } - } - stream.WriteArrayStart() - stream.WriteInt(boundaries) - stream.WriteMore() - jsonutil.MarshalValue(bucket.Lower, stream) - stream.WriteMore() - jsonutil.MarshalValue(bucket.Upper, stream) - stream.WriteMore() - jsonutil.MarshalValue(bucket.Count, stream) - stream.WriteArrayEnd() - } - if bucketFound { - stream.WriteArrayEnd() - } - stream.WriteObjectEnd() -} - -// marshalExemplarJSON writes. -// -// { -// labels: , -// value: "", -// timestamp: -// } -func marshalExemplarJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { - p := *((*exemplar.Exemplar)(ptr)) - stream.WriteObjectStart() - - // "labels" key. - stream.WriteObjectField(`labels`) - lbls, err := p.Labels.MarshalJSON() - if err != nil { - stream.Error = err - return - } - stream.SetBuffer(append(stream.Buffer(), lbls...)) - - // "value" key. - stream.WriteMore() - stream.WriteObjectField(`value`) - jsonutil.MarshalValue(p.Value, stream) - - // "timestamp" key. - stream.WriteMore() - stream.WriteObjectField(`timestamp`) - jsonutil.MarshalTimestamp(p.Ts, stream) - - stream.WriteObjectEnd() -} - -func marshalExemplarJSONEmpty(ptr unsafe.Pointer) bool { - return false -} diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go index b38dab0385..455af717d0 100644 --- a/web/api/v1/json_codec.go +++ b/web/api/v1/json_codec.go @@ -13,7 +13,23 @@ package v1 -import jsoniter "github.com/json-iterator/go" +import ( + "unsafe" + + jsoniter "github.com/json-iterator/go" + + "github.com/prometheus/prometheus/model/exemplar" + "github.com/prometheus/prometheus/model/histogram" + "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/util/jsonutil" +) + +func init() { + jsoniter.RegisterTypeEncoderFunc("promql.Series", marshalSeriesJSON, marshalSeriesJSONIsEmpty) + jsoniter.RegisterTypeEncoderFunc("promql.Sample", marshalSampleJSON, marshalSampleJSONIsEmpty) + jsoniter.RegisterTypeEncoderFunc("promql.Point", marshalPointJSON, marshalPointJSONIsEmpty) + jsoniter.RegisterTypeEncoderFunc("exemplar.Exemplar", marshalExemplarJSON, marshalExemplarJSONEmpty) +} // JSONCodec is a Codec that encodes API responses as JSON. type JSONCodec struct{} @@ -30,3 +46,247 @@ func (j JSONCodec) Encode(resp *Response) ([]byte, error) { json := jsoniter.ConfigCompatibleWithStandardLibrary return json.Marshal(resp) } + +// marshalSeriesJSON writes something like the following: +// +// { +// "metric" : { +// "__name__" : "up", +// "job" : "prometheus", +// "instance" : "localhost:9090" +// }, +// "values": [ +// [ 1435781451.781, "1" ], +// < more values> +// ], +// "histograms": [ +// [ 1435781451.781, { < histogram, see below > } ], +// < more histograms > +// ], +// }, +func marshalSeriesJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { + s := *((*promql.Series)(ptr)) + stream.WriteObjectStart() + stream.WriteObjectField(`metric`) + m, err := s.Metric.MarshalJSON() + if err != nil { + stream.Error = err + return + } + stream.SetBuffer(append(stream.Buffer(), m...)) + + // We make two passes through the series here: In the first marshaling + // all value points, in the second marshaling all histogram + // points. That's probably cheaper than just one pass in which we copy + // out histogram Points into a newly allocated slice for separate + // marshaling. (Could be benchmarked, though.) + var foundValue, foundHistogram bool + for _, p := range s.Points { + if p.H == nil { + stream.WriteMore() + if !foundValue { + stream.WriteObjectField(`values`) + stream.WriteArrayStart() + } + foundValue = true + marshalPointJSON(unsafe.Pointer(&p), stream) + } else { + foundHistogram = true + } + } + if foundValue { + stream.WriteArrayEnd() + } + if foundHistogram { + firstHistogram := true + for _, p := range s.Points { + if p.H != nil { + stream.WriteMore() + if firstHistogram { + stream.WriteObjectField(`histograms`) + stream.WriteArrayStart() + } + firstHistogram = false + marshalPointJSON(unsafe.Pointer(&p), stream) + } + } + stream.WriteArrayEnd() + } + stream.WriteObjectEnd() +} + +func marshalSeriesJSONIsEmpty(ptr unsafe.Pointer) bool { + return false +} + +// marshalSampleJSON writes something like the following for normal value samples: +// +// { +// "metric" : { +// "__name__" : "up", +// "job" : "prometheus", +// "instance" : "localhost:9090" +// }, +// "value": [ 1435781451.781, "1" ] +// }, +// +// For histogram samples, it writes something like this: +// +// { +// "metric" : { +// "__name__" : "up", +// "job" : "prometheus", +// "instance" : "localhost:9090" +// }, +// "histogram": [ 1435781451.781, { < histogram, see below > } ] +// }, +func marshalSampleJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { + s := *((*promql.Sample)(ptr)) + stream.WriteObjectStart() + stream.WriteObjectField(`metric`) + m, err := s.Metric.MarshalJSON() + if err != nil { + stream.Error = err + return + } + stream.SetBuffer(append(stream.Buffer(), m...)) + stream.WriteMore() + if s.Point.H == nil { + stream.WriteObjectField(`value`) + } else { + stream.WriteObjectField(`histogram`) + } + marshalPointJSON(unsafe.Pointer(&s.Point), stream) + stream.WriteObjectEnd() +} + +func marshalSampleJSONIsEmpty(ptr unsafe.Pointer) bool { + return false +} + +// marshalPointJSON writes `[ts, "val"]`. +func marshalPointJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { + p := *((*promql.Point)(ptr)) + stream.WriteArrayStart() + jsonutil.MarshalTimestamp(p.T, stream) + stream.WriteMore() + if p.H == nil { + jsonutil.MarshalValue(p.V, stream) + } else { + marshalHistogram(p.H, stream) + } + stream.WriteArrayEnd() +} + +func marshalPointJSONIsEmpty(ptr unsafe.Pointer) bool { + return false +} + +// marshalHistogramJSON writes something like: +// +// { +// "count": "42", +// "sum": "34593.34", +// "buckets": [ +// [ 3, "-0.25", "0.25", "3"], +// [ 0, "0.25", "0.5", "12"], +// [ 0, "0.5", "1", "21"], +// [ 0, "2", "4", "6"] +// ] +// } +// +// The 1st element in each bucket array determines if the boundaries are +// inclusive (AKA closed) or exclusive (AKA open): +// +// 0: lower exclusive, upper inclusive +// 1: lower inclusive, upper exclusive +// 2: both exclusive +// 3: both inclusive +// +// The 2nd and 3rd elements are the lower and upper boundary. The 4th element is +// the bucket count. +func marshalHistogram(h *histogram.FloatHistogram, stream *jsoniter.Stream) { + stream.WriteObjectStart() + stream.WriteObjectField(`count`) + jsonutil.MarshalValue(h.Count, stream) + stream.WriteMore() + stream.WriteObjectField(`sum`) + jsonutil.MarshalValue(h.Sum, stream) + + bucketFound := false + it := h.AllBucketIterator() + for it.Next() { + bucket := it.At() + if bucket.Count == 0 { + continue // No need to expose empty buckets in JSON. + } + stream.WriteMore() + if !bucketFound { + stream.WriteObjectField(`buckets`) + stream.WriteArrayStart() + } + bucketFound = true + boundaries := 2 // Exclusive on both sides AKA open interval. + if bucket.LowerInclusive { + if bucket.UpperInclusive { + boundaries = 3 // Inclusive on both sides AKA closed interval. + } else { + boundaries = 1 // Inclusive only on lower end AKA right open. + } + } else { + if bucket.UpperInclusive { + boundaries = 0 // Inclusive only on upper end AKA left open. + } + } + stream.WriteArrayStart() + stream.WriteInt(boundaries) + stream.WriteMore() + jsonutil.MarshalValue(bucket.Lower, stream) + stream.WriteMore() + jsonutil.MarshalValue(bucket.Upper, stream) + stream.WriteMore() + jsonutil.MarshalValue(bucket.Count, stream) + stream.WriteArrayEnd() + } + if bucketFound { + stream.WriteArrayEnd() + } + stream.WriteObjectEnd() +} + +// marshalExemplarJSON writes. +// +// { +// labels: , +// value: "", +// timestamp: +// } +func marshalExemplarJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) { + p := *((*exemplar.Exemplar)(ptr)) + stream.WriteObjectStart() + + // "labels" key. + stream.WriteObjectField(`labels`) + lbls, err := p.Labels.MarshalJSON() + if err != nil { + stream.Error = err + return + } + stream.SetBuffer(append(stream.Buffer(), lbls...)) + + // "value" key. + stream.WriteMore() + stream.WriteObjectField(`value`) + jsonutil.MarshalValue(p.Value, stream) + + // "timestamp" key. + stream.WriteMore() + stream.WriteObjectField(`timestamp`) + jsonutil.MarshalTimestamp(p.Ts, stream) + + stream.WriteObjectEnd() +} + +func marshalExemplarJSONEmpty(ptr unsafe.Pointer) bool { + return false +} From 857b23873f54afa642e4961071c326ea1e00eddf Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Thu, 2 Feb 2023 15:29:13 +1100 Subject: [PATCH 3/5] Expose QueryData so that implementations of Codec.CanEncode() can perform a type assertion against Response.Data. Signed-off-by: Charles Korn --- web/api/v1/api.go | 6 +++--- web/api/v1/api_test.go | 28 ++++++++++++++-------------- web/api/v1/json_codec_test.go | 4 ++-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index e0ad76c5cb..e9c1182ca1 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -384,7 +384,7 @@ func (api *API) Register(r *route.Router) { r.Put("/admin/tsdb/snapshot", wrapAgent(api.snapshot)) } -type queryData struct { +type QueryData struct { ResultType parser.ValueType `json:"resultType"` Result parser.Value `json:"result"` Stats stats.QueryStats `json:"stats,omitempty"` @@ -446,7 +446,7 @@ func (api *API) query(r *http.Request) (result apiFuncResult) { } qs := sr(ctx, qry.Stats(), r.FormValue("stats")) - return apiFuncResult{&queryData{ + return apiFuncResult{&QueryData{ ResultType: res.Value.Type(), Result: res.Value, Stats: qs, @@ -537,7 +537,7 @@ func (api *API) queryRange(r *http.Request) (result apiFuncResult) { } qs := sr(ctx, qry.Stats(), r.FormValue("stats")) - return apiFuncResult{&queryData{ + return apiFuncResult{&QueryData{ ResultType: res.Value.Type(), Result: res.Value, Stats: qs, diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 617a8bdf3c..455eae13f7 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -831,8 +831,8 @@ func TestStats(t *testing.T) { name: "stats is blank", param: "", expected: func(t *testing.T, i interface{}) { - require.IsType(t, i, &queryData{}) - qd := i.(*queryData) + require.IsType(t, i, &QueryData{}) + qd := i.(*QueryData) require.Nil(t, qd.Stats) }, }, @@ -840,8 +840,8 @@ func TestStats(t *testing.T) { name: "stats is true", param: "true", expected: func(t *testing.T, i interface{}) { - require.IsType(t, i, &queryData{}) - qd := i.(*queryData) + require.IsType(t, i, &QueryData{}) + qd := i.(*QueryData) require.NotNil(t, qd.Stats) qs := qd.Stats.Builtin() require.NotNil(t, qs.Timings) @@ -855,8 +855,8 @@ func TestStats(t *testing.T) { name: "stats is all", param: "all", expected: func(t *testing.T, i interface{}) { - require.IsType(t, i, &queryData{}) - qd := i.(*queryData) + require.IsType(t, i, &QueryData{}) + qd := i.(*QueryData) require.NotNil(t, qd.Stats) qs := qd.Stats.Builtin() require.NotNil(t, qs.Timings) @@ -876,8 +876,8 @@ func TestStats(t *testing.T) { }, param: "known", expected: func(t *testing.T, i interface{}) { - require.IsType(t, i, &queryData{}) - qd := i.(*queryData) + require.IsType(t, i, &QueryData{}) + qd := i.(*QueryData) require.NotNil(t, qd.Stats) j, err := json.Marshal(qd.Stats) require.NoError(t, err) @@ -1037,7 +1037,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E "query": []string{"2"}, "time": []string{"123.4"}, }, - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeScalar, Result: promql.Scalar{ V: 2, @@ -1051,7 +1051,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E "query": []string{"0.333"}, "time": []string{"1970-01-01T00:02:03Z"}, }, - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeScalar, Result: promql.Scalar{ V: 0.333, @@ -1065,7 +1065,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E "query": []string{"0.333"}, "time": []string{"1970-01-01T01:02:03+01:00"}, }, - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeScalar, Result: promql.Scalar{ V: 0.333, @@ -1078,7 +1078,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E query: url.Values{ "query": []string{"0.333"}, }, - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeScalar, Result: promql.Scalar{ V: 0.333, @@ -1094,7 +1094,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E "end": []string{"2"}, "step": []string{"1"}, }, - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeMatrix, Result: promql.Matrix{ promql.Series{ @@ -3180,7 +3180,7 @@ func BenchmarkRespond(b *testing.B) { for i := 0; i < 10000; i++ { points = append(points, promql.Point{V: float64(i * 1000000), T: int64(i)}) } - response := &queryData{ + response := &QueryData{ ResultType: parser.ValueTypeMatrix, Result: promql.Matrix{ promql.Series{ diff --git a/web/api/v1/json_codec_test.go b/web/api/v1/json_codec_test.go index c5b030ff9a..20b8ac3a0c 100644 --- a/web/api/v1/json_codec_test.go +++ b/web/api/v1/json_codec_test.go @@ -30,7 +30,7 @@ func TestJsonCodec_Encode(t *testing.T) { expected string }{ { - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeMatrix, Result: promql.Matrix{ promql.Series{ @@ -42,7 +42,7 @@ func TestJsonCodec_Encode(t *testing.T) { expected: `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"foo"},"values":[[1,"1"]]}]}}`, }, { - response: &queryData{ + response: &QueryData{ ResultType: parser.ValueTypeMatrix, Result: promql.Matrix{ promql.Series{ From deba5120ead5af493e8a4dac4eb0171596b983e3 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Sat, 11 Feb 2023 15:34:25 +0100 Subject: [PATCH 4/5] Address PR feeedback: reduce log level. Signed-off-by: Charles Korn --- web/api/v1/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index e9c1182ca1..6c912e9e8f 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -1602,7 +1602,7 @@ func (api *API) negotiateCodec(req *http.Request, resp *Response) Codec { } } - level.Warn(api.logger).Log("msg", "could not find suitable codec for response, falling back to default codec", "accept_header", acceptHeader) + level.Debug(api.logger).Log("msg", "could not find suitable codec for response, falling back to default codec", "accept_header", acceptHeader) return defaultCodec } From 46a28899a0e86b4b073bb3172a2ac4013ba87807 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Mon, 27 Feb 2023 13:27:09 +1100 Subject: [PATCH 5/5] Implement fully-featured content negotiation for API requests, and allow overriding the default API codec. Signed-off-by: Charles Korn --- go.mod | 2 +- web/api/v1/api.go | 78 ++++++++++++++++++++-------------------- web/api/v1/api_test.go | 41 ++++++++++++++++++--- web/api/v1/codec.go | 29 ++++++++++++++- web/api/v1/codec_test.go | 68 +++++++++++++++++++++++++++++++++++ web/api/v1/json_codec.go | 4 +-- 6 files changed, 175 insertions(+), 47 deletions(-) create mode 100644 web/api/v1/codec_test.go diff --git a/go.mod b/go.mod index ac94408e4b..a12b3505d8 100644 --- a/go.mod +++ b/go.mod @@ -160,7 +160,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/morikuni/aec v1.0.0 // indirect - github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.0.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 10cf6885db..cfac908fe8 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -32,6 +32,7 @@ import ( "github.com/go-kit/log/level" "github.com/grafana/regexp" jsoniter "github.com/json-iterator/go" + "github.com/munnerz/goautoneg" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/model" @@ -68,20 +69,19 @@ const ( type errorType string const ( - errorNone errorType = "" - errorTimeout errorType = "timeout" - errorCanceled errorType = "canceled" - errorExec errorType = "execution" - errorBadData errorType = "bad_data" - errorInternal errorType = "internal" - errorUnavailable errorType = "unavailable" - errorNotFound errorType = "not_found" + errorNone errorType = "" + errorTimeout errorType = "timeout" + errorCanceled errorType = "canceled" + errorExec errorType = "execution" + errorBadData errorType = "bad_data" + errorInternal errorType = "internal" + errorUnavailable errorType = "unavailable" + errorNotFound errorType = "not_found" + errorNotAcceptable errorType = "not_acceptable" ) var LocalhostRepresentations = []string{"127.0.0.1", "localhost", "::1"} -var defaultCodec = JSONCodec{} - type apiError struct { typ errorType err error @@ -212,7 +212,7 @@ type API struct { remoteWriteHandler http.Handler remoteReadHandler http.Handler - codecs map[string]Codec + codecs []Codec } // NewAPI returns an initialized API type. @@ -271,11 +271,9 @@ func NewAPI( statsRenderer: defaultStatsRenderer, remoteReadHandler: remote.NewReadHandler(logger, registerer, q, configFunc, remoteReadSampleLimit, remoteReadConcurrencyLimit, remoteReadMaxBytesInFrame), - - codecs: map[string]Codec{}, } - a.InstallCodec(defaultCodec) + a.InstallCodec(JSONCodec{}) if statsRenderer != nil { a.statsRenderer = statsRenderer @@ -289,13 +287,15 @@ func NewAPI( } // InstallCodec adds codec to this API's available codecs. -// If codec handles a content type handled by a codec already installed in this API, codec replaces the previous codec. +// Codecs installed first take precedence over codecs installed later when evaluating wildcards in Accept headers. +// The first installed codec is used as a fallback when the Accept header cannot be satisfied or if there is no Accept header. func (api *API) InstallCodec(codec Codec) { - if api.codecs == nil { - api.codecs = map[string]Codec{} - } + api.codecs = append(api.codecs, codec) +} - api.codecs[codec.ContentType()] = codec +// ClearCodecs removes all available codecs from this API, including the default codec installed by NewAPI. +func (api *API) ClearCodecs() { + api.codecs = nil } func setUnavailStatusOnTSDBNotReady(r apiFuncResult) apiFuncResult { @@ -1583,7 +1583,12 @@ func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface Warnings: warningStrings, } - codec := api.negotiateCodec(req, resp) + codec, err := api.negotiateCodec(req, resp) + if err != nil { + api.respondError(w, &apiError{errorNotAcceptable, err}, nil) + return + } + b, err := codec.Encode(resp) if err != nil { level.Error(api.logger).Log("msg", "error marshaling response", "err", err) @@ -1591,33 +1596,28 @@ func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface return } - w.Header().Set("Content-Type", codec.ContentType()) + w.Header().Set("Content-Type", codec.ContentType().String()) w.WriteHeader(http.StatusOK) if n, err := w.Write(b); err != nil { level.Error(api.logger).Log("msg", "error writing response", "bytesWritten", n, "err", err) } } -// HTTP content negotiation is hard (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation). -// Ideally, we shouldn't be implementing this ourselves - https://github.com/golang/go/issues/19307 is an open proposal to add -// this to the Go stdlib and has links to a number of other implementations. -// -// This is an MVP, and doesn't support features like wildcards or weighting. -func (api *API) negotiateCodec(req *http.Request, resp *Response) Codec { - acceptHeader := req.Header.Get("Accept") - if acceptHeader == "" { - return defaultCodec - } - - for _, contentType := range strings.Split(acceptHeader, ",") { - codec, ok := api.codecs[strings.TrimSpace(contentType)] - if ok && codec.CanEncode(resp) { - return codec +func (api *API) negotiateCodec(req *http.Request, resp *Response) (Codec, error) { + for _, clause := range goautoneg.ParseAccept(req.Header.Get("Accept")) { + for _, codec := range api.codecs { + if codec.ContentType().Satisfies(clause) && codec.CanEncode(resp) { + return codec, nil + } } } - level.Debug(api.logger).Log("msg", "could not find suitable codec for response, falling back to default codec", "accept_header", acceptHeader) - return defaultCodec + defaultCodec := api.codecs[0] + if !defaultCodec.CanEncode(resp) { + return nil, fmt.Errorf("cannot encode response as %s", defaultCodec.ContentType()) + } + + return defaultCodec, nil } func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) { @@ -1648,6 +1648,8 @@ func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data inter code = http.StatusInternalServerError case errorNotFound: code = http.StatusNotFound + case errorNotAcceptable: + code = http.StatusNotAcceptable default: code = http.StatusInternalServerError } diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 0531c4fe53..90cf084ac0 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -2769,9 +2769,11 @@ func TestRespondSuccess(t *testing.T) { logger: log.NewNopLogger(), } - api.InstallCodec(&testCodec{contentType: "test/cannot-encode", canEncode: false}) - api.InstallCodec(&testCodec{contentType: "test/can-encode", canEncode: true}) - api.InstallCodec(&testCodec{contentType: "test/can-encode-2", canEncode: true}) + api.ClearCodecs() + api.InstallCodec(JSONCodec{}) + api.InstallCodec(&testCodec{contentType: MIMEType{"test", "cannot-encode"}, canEncode: false}) + api.InstallCodec(&testCodec{contentType: MIMEType{"test", "can-encode"}, canEncode: true}) + api.InstallCodec(&testCodec{contentType: MIMEType{"test", "can-encode-2"}, canEncode: true}) s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { api.respond(w, r, "test", nil) @@ -2854,6 +2856,34 @@ func TestRespondSuccess(t *testing.T) { } } +func TestRespondSuccess_DefaultCodecCannotEncodeResponse(t *testing.T) { + api := API{ + logger: log.NewNopLogger(), + } + + api.ClearCodecs() + api.InstallCodec(&testCodec{contentType: MIMEType{"application", "default-format"}, canEncode: false}) + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + api.respond(w, r, "test", nil) + })) + defer s.Close() + + req, err := http.NewRequest(http.MethodGet, s.URL, nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + + body, err := io.ReadAll(resp.Body) + defer resp.Body.Close() + require.NoError(t, err) + + require.Equal(t, http.StatusNotAcceptable, resp.StatusCode) + require.Equal(t, "application/json", resp.Header.Get("Content-Type")) + require.Equal(t, `{"status":"error","errorType":"not_acceptable","error":"cannot encode response as application/default-format"}`, string(body)) +} + func TestRespondError(t *testing.T) { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { api := API{} @@ -3193,6 +3223,7 @@ func BenchmarkRespond(b *testing.B) { } b.ResetTimer() api := API{} + api.InstallCodec(JSONCodec{}) for n := 0; n < b.N; n++ { api.respond(&testResponseWriter, request, response, nil) } @@ -3307,11 +3338,11 @@ func TestGetGlobalURL(t *testing.T) { } type testCodec struct { - contentType string + contentType MIMEType canEncode bool } -func (t *testCodec) ContentType() string { +func (t *testCodec) ContentType() MIMEType { return t.contentType } diff --git a/web/api/v1/codec.go b/web/api/v1/codec.go index d11bb1fa01..492e00a74a 100644 --- a/web/api/v1/codec.go +++ b/web/api/v1/codec.go @@ -13,10 +13,12 @@ package v1 +import "github.com/munnerz/goautoneg" + // A Codec performs encoding of API responses. type Codec interface { // ContentType returns the MIME time that this Codec emits. - ContentType() string + ContentType() MIMEType // CanEncode determines if this Codec can encode resp. CanEncode(resp *Response) bool @@ -24,3 +26,28 @@ type Codec interface { // Encode encodes resp, ready for transmission to an API consumer. Encode(resp *Response) ([]byte, error) } + +type MIMEType struct { + Type string + SubType string +} + +func (m MIMEType) String() string { + return m.Type + "/" + m.SubType +} + +func (m MIMEType) Satisfies(accept goautoneg.Accept) bool { + if accept.Type == "*" && accept.SubType == "*" { + return true + } + + if accept.Type == m.Type && accept.SubType == "*" { + return true + } + + if accept.Type == m.Type && accept.SubType == m.SubType { + return true + } + + return false +} diff --git a/web/api/v1/codec_test.go b/web/api/v1/codec_test.go new file mode 100644 index 0000000000..911bf206e3 --- /dev/null +++ b/web/api/v1/codec_test.go @@ -0,0 +1,68 @@ +// Copyright 2016 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1 + +import ( + "testing" + + "github.com/munnerz/goautoneg" + "github.com/stretchr/testify/require" +) + +func TestMIMEType_String(t *testing.T) { + m := MIMEType{Type: "application", SubType: "json"} + + require.Equal(t, "application/json", m.String()) +} + +func TestMIMEType_Satisfies(t *testing.T) { + m := MIMEType{Type: "application", SubType: "json"} + + scenarios := map[string]struct { + accept goautoneg.Accept + expected bool + }{ + "exact match": { + accept: goautoneg.Accept{Type: "application", SubType: "json"}, + expected: true, + }, + "sub-type wildcard match": { + accept: goautoneg.Accept{Type: "application", SubType: "*"}, + expected: true, + }, + "full wildcard match": { + accept: goautoneg.Accept{Type: "*", SubType: "*"}, + expected: true, + }, + "inverted": { + accept: goautoneg.Accept{Type: "json", SubType: "application"}, + expected: false, + }, + "inverted sub-type wildcard": { + accept: goautoneg.Accept{Type: "json", SubType: "*"}, + expected: false, + }, + "complete mismatch": { + accept: goautoneg.Accept{Type: "text", SubType: "plain"}, + expected: false, + }, + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { + actual := m.Satisfies(scenario.accept) + require.Equal(t, scenario.expected, actual) + }) + } +} diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go index 455af717d0..79ebfee182 100644 --- a/web/api/v1/json_codec.go +++ b/web/api/v1/json_codec.go @@ -34,8 +34,8 @@ func init() { // JSONCodec is a Codec that encodes API responses as JSON. type JSONCodec struct{} -func (j JSONCodec) ContentType() string { - return "application/json" +func (j JSONCodec) ContentType() MIMEType { + return MIMEType{Type: "application", SubType: "json"} } func (j JSONCodec) CanEncode(_ *Response) bool {