Prometheus: Handle the response with different field key order (#74567)

* Handle the response with different field key order

* More unit tests to cover edge cases

* Cover more edge cases

* make it simpler

* Better test inputs

pkg/tsdb/prometheus/querydata/response_test.go

@@ -0,0 +1,58 @@
+package querydata
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/grafana/grafana/pkg/tsdb/prometheus/models"
+	"github.com/grafana/grafana/pkg/tsdb/prometheus/querydata/exemplar"
+)
+
+func TestQueryData_parseResponse(t *testing.T) {
+	qd := QueryData{exemplarSampler: exemplar.NewStandardDeviationSampler}
+
+	t.Run("resultType is before result the field must parsed normally", func(t *testing.T) {
+		resBody := `{"data":{"resultType":"vector", "result":[{"metric":{"__name__":"some_name","environment":"some_env","id":"some_id","instance":"some_instance:1234","job":"some_job","name":"another_name","region":"some_region"},"value":[1.1,"2"]}]},"status":"success"}`
+		res := &http.Response{Body: io.NopCloser(bytes.NewBufferString(resBody))}
+		result := qd.parseResponse(context.Background(), &models.Query{}, res)
+		assert.Nil(t, result.Error)
+		assert.Len(t, result.Frames, 1)
+	})
+
+	t.Run("resultType is after the result field must parsed normally", func(t *testing.T) {
+		resBody := `{"data":{"result":[{"metric":{"__name__":"some_name","environment":"some_env","id":"some_id","instance":"some_instance:1234","job":"some_job","name":"another_name","region":"some_region"},"value":[1.1,"2"]}],"resultType":"vector"},"status":"success"}`
+		res := &http.Response{Body: io.NopCloser(bytes.NewBufferString(resBody))}
+		result := qd.parseResponse(context.Background(), &models.Query{}, res)
+		assert.Nil(t, result.Error)
+		assert.Len(t, result.Frames, 1)
+	})
+
+	t.Run("no resultType is existed in the data", func(t *testing.T) {
+		resBody := `{"data":{"result":[{"metric":{"__name__":"some_name","environment":"some_env","id":"some_id","instance":"some_instance:1234","job":"some_job","name":"another_name","region":"some_region"},"value":[1.1,"2"]}]},"status":"success"}`
+		res := &http.Response{Body: io.NopCloser(bytes.NewBufferString(resBody))}
+		result := qd.parseResponse(context.Background(), &models.Query{}, res)
+		assert.Error(t, result.Error)
+		assert.Equal(t, result.Error.Error(), "no resultType found")
+	})
+
+	t.Run("resultType is set as empty string before result", func(t *testing.T) {
+		resBody := `{"data":{"resultType":"", "result":[{"metric":{"__name__":"some_name","environment":"some_env","id":"some_id","instance":"some_instance:1234","job":"some_job","name":"another_name","region":"some_region"},"value":[1.1,"2"]}]},"status":"success"}`
+		res := &http.Response{Body: io.NopCloser(bytes.NewBufferString(resBody))}
+		result := qd.parseResponse(context.Background(), &models.Query{}, res)
+		assert.Error(t, result.Error)
+		assert.Equal(t, result.Error.Error(), "unknown result type: ")
+	})
+
+	t.Run("resultType is set as empty string after result", func(t *testing.T) {
+		resBody := `{"data":{"result":[{"metric":{"__name__":"some_name","environment":"some_env","id":"some_id","instance":"some_instance:1234","job":"some_job","name":"another_name","region":"some_region"},"value":[1.1,"2"]}],"resultType":""},"status":"success"}`
+		res := &http.Response{Body: io.NopCloser(bytes.NewBufferString(resBody))}
+		result := qd.parseResponse(context.Background(), &models.Query{}, res)
+		assert.Error(t, result.Error)
+		assert.Equal(t, result.Error.Error(), "unknown result type: ")
+	})
+}

pkg/util/converter/jsonitere/jsonitere.go

@@ -1,11 +1,13 @@
-// package jsonitere wraps json-iterator/go's Iterator methods with error returns
+// Package jsonitere wraps json-iterator/go's Iterator methods with error returns
 // so linting can catch unchecked errors.
 // The underlying iterator's Error property is returned and not reset.
 // See json-iterator/go for method documentation and additional methods that
 // can be added to this library.
 package jsonitere
 
-import j "github.com/json-iterator/go"
+import (
+	j "github.com/json-iterator/go"
+)
 
 type Iterator struct {
 	// named property instead of embedded so there is no
@@ -46,6 +48,10 @@ func (iter *Iterator) Skip() error {
 	return iter.i.Error
 }
 
+func (iter *Iterator) SkipAndReturnBytes() []byte {
+	return iter.i.SkipAndReturnBytes()
+}
+
 func (iter *Iterator) ReadVal(obj any) error {
 	iter.i.ReadVal(obj)
 	return iter.i.Error

pkg/util/converter/prom.go

@@ -8,8 +8,9 @@ import (
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
-	"github.com/grafana/grafana/pkg/util/converter/jsonitere"
 	jsoniter "github.com/json-iterator/go"
+
+	"github.com/grafana/grafana/pkg/util/converter/jsonitere"
 )
 
 // helpful while debugging all the options that may appear
@@ -153,6 +154,8 @@ func readPrometheusData(iter *jsonitere.Iterator, opt Options) backend.DataRespo
 	}
 
 	resultType := ""
+	resultTypeFound := false
+	var resultBytes []byte
 
 l1Fields:
 	for l1Field, err := iter.ReadObject(); ; l1Field, err = iter.ReadObject() {
@@ -165,35 +168,22 @@ l1Fields:
 			if err != nil {
 				return rspErr(err)
 			}
+			resultTypeFound = true
+
+			// if we have saved resultBytes we will parse them here
+			// we saved them because when we had them we don't know the resultType
+			if len(resultBytes) > 0 {
+				ji := jsonitere.NewIterator(jsoniter.ParseBytes(jsoniter.ConfigDefault, resultBytes))
+				rsp = readResult(resultType, rsp, ji, opt)
+			}
 		case "result":
-			switch resultType {
+			// for some rare cases resultType is coming after the result.
-			case "matrix", "vector":
+			// when that happens we save the bytes and parse them after reading resultType
-				rsp = readMatrixOrVectorMulti(iter, resultType, opt)
+			// see: https://github.com/grafana/grafana/issues/64693
-				if rsp.Error != nil {
+			if resultTypeFound {
-					return rsp
+				rsp = readResult(resultType, rsp, iter, opt)
-				}
+			} else {
-			case "streams":
+				resultBytes = iter.SkipAndReturnBytes()
-				rsp = readStream(iter)
-				if rsp.Error != nil {
-					return rsp
-				}
-			case "string":
-				rsp = readString(iter)
-				if rsp.Error != nil {
-					return rsp
-				}
-			case "scalar":
-				rsp = readScalar(iter)
-				if rsp.Error != nil {
-					return rsp
-				}
-			default:
-				if err = iter.Skip(); err != nil {
-					return rspErr(err)
-				}
-				rsp = backend.DataResponse{
-					Error: fmt.Errorf("unknown result type: %s", resultType),
-				}
 			}
 
 		case "stats":
@@ -216,6 +206,9 @@ l1Fields:
 			if err != nil {
 				return rspErr(err)
 			}
+			if !resultTypeFound {
+				return rspErr(fmt.Errorf("no resultType found"))
+			}
 			break l1Fields
 
 		default:
@@ -230,6 +223,40 @@ l1Fields:
 	return rsp
 }
 
+// will read the result object based on the resultType and return a DataResponse
+func readResult(resultType string, rsp backend.DataResponse, iter *jsonitere.Iterator, opt Options) backend.DataResponse {
+	switch resultType {
+	case "matrix", "vector":
+		rsp = readMatrixOrVectorMulti(iter, resultType, opt)
+		if rsp.Error != nil {
+			return rsp
+		}
+	case "streams":
+		rsp = readStream(iter)
+		if rsp.Error != nil {
+			return rsp
+		}
+	case "string":
+		rsp = readString(iter)
+		if rsp.Error != nil {
+			return rsp
+		}
+	case "scalar":
+		rsp = readScalar(iter)
+		if rsp.Error != nil {
+			return rsp
+		}
+	default:
+		if err := iter.Skip(); err != nil {
+			return rspErr(err)
+		}
+		rsp = backend.DataResponse{
+			Error: fmt.Errorf("unknown result type: %s", resultType),
+		}
+	}
+	return rsp
+}
+
 // will return strings or exemplars
 func readArrayData(iter *jsonitere.Iterator) backend.DataResponse {
 	lookup := make(map[string]*data.Field)