Skip to content

Commit

Permalink
fix query type switching when creating alerts in Grafana (#246)
Browse files Browse the repository at this point in the history 
* fix setting the default query type

* fix response parsing for the alerting rules

* cover function by tests

* docs: fix query type switching

---------

Co-authored-by: dmitryk-dk <[email protected]>
  • Loading branch information
@Loori-R @dmitryk-dk
Loori-R and dmitryk-dk authored Jan 17, 2025
1 parent 79c7521 commit 11a08d1
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 62 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
Thanks to @chenlujjj for [the pull request](https://github.com/VictoriaMetrics/victoriametrics-datasource/pull/243).

* BUGFIX: fix issue with variables not working in adhoc filters. See [this issue](https://github.com/VictoriaMetrics/victoriametrics-datasource/issues/235).
* BUGFIX: fix query type switching when creating alerts in Grafana. See [this issue](https://github.com/VictoriaMetrics/victoriametrics-datasource/issues/237)

## [v0.10.3](https://github.com/VictoriaMetrics/victoriametrics-datasource/releases/tag/v0.10.3)

Expand Down
37 changes: 33 additions & 4 deletions pkg/plugin/datasource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"net/http"
"strconv"
"strings"
"sync"
"time"
Expand All @@ -20,6 +21,9 @@ import (
const (
defaultScrapeInterval = 15 * time.Second
health = "/-/healthy"
// it is weird logic to pass an identifier for an alert request in the headers
// but Grafana decided to do so, so we need to follow this
requestFromAlert = "FromAlert"
)

// NewDatasource creates a new datasource instance.
Expand Down Expand Up @@ -62,22 +66,28 @@ func (d *Datasource) Dispose() {
// contains Frames ([]*Frame).
func (d *Datasource) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
response := backend.NewQueryDataResponse()
headers := req.Headers

forAlerting, err := d.checkAlertingRequest(headers)
if err != nil {
return nil, err
}

var wg sync.WaitGroup
for _, q := range req.Queries {
wg.Add(1)
go func(q backend.DataQuery) {
go func(q backend.DataQuery, forAlerting bool) {
defer wg.Done()
response.Responses[q.RefID] = d.query(ctx, q)
}(q)
response.Responses[q.RefID] = d.query(ctx, q, forAlerting)
}(q, forAlerting)
}
wg.Wait()

return response, nil
}

// query process backend.Query and return response
func (d *Datasource) query(ctx context.Context, query backend.DataQuery) backend.DataResponse {
func (d *Datasource) query(ctx context.Context, query backend.DataQuery, forAlerting bool) backend.DataResponse {
var q Query
if err := json.Unmarshal(query.JSON, &q); err != nil {
err = fmt.Errorf("failed to parse query json: %s", err)
Expand Down Expand Up @@ -152,6 +162,8 @@ func (d *Datasource) query(ctx context.Context, query backend.DataQuery) backend
return newResponseError(err, backend.StatusInternal)
}

r.ForAlerting = forAlerting

frames, err := r.getDataFrames()
if err != nil {
err = fmt.Errorf("failed to prepare data from response: %w", err)
Expand Down Expand Up @@ -196,6 +208,23 @@ func (d *Datasource) CheckHealth(ctx context.Context, _ *backend.CheckHealthRequ
}, nil
}

func (d *Datasource) checkAlertingRequest(headers map[string]string) (bool, error) {
var forAlerting bool
if val, ok := headers[requestFromAlert]; ok {
if val == "" {
return false, nil
}

boolValue, err := strconv.ParseBool(val)
if err != nil {
return false, fmt.Errorf("failed to parse %s header value: %s", requestFromAlert, val)
}

forAlerting = boolValue
}
return forAlerting, nil
}

// newHealthCheckErrorf returns a new *backend.CheckHealthResult with its status set to backend.HealthStatusError
// and the specified message, which is formatted with Sprintf.
func newHealthCheckErrorf(format string, args ...interface{}) *backend.CheckHealthResult {
Expand Down
53 changes: 53 additions & 0 deletions pkg/plugin/datasource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,56 @@ func TestDatasourceQueryRequestWithRetry(t *testing.T) {
expValue(2) // 1 - fail, 2 - retry
expErr("EOF") // 3, 4 - retries
}

func TestDatasource_checkAlertingRequest(t *testing.T) {
tests := []struct {
name string
headers map[string]string
want bool
wantErr bool
}{
{
name: "no alerting header",
headers: map[string]string{},
want: false,
wantErr: false,
},
{
name: "alerting header",
headers: map[string]string{"FromAlert": "true"},
want: true,
wantErr: false,
},
{
name: "invalid alerting header",
headers: map[string]string{"FromAlert": "invalid"},
want: false,
wantErr: true,
},
{
name: "false alerting header",
headers: map[string]string{"FromAlert": "false"},
want: false,
wantErr: false,
},
{
name: "irrelevant header",
headers: map[string]string{"SomeOtherHeader": "true"},
want: false,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := &Datasource{}
got, err := d.checkAlertingRequest(tt.headers)
if (err != nil) != tt.wantErr {
t.Errorf("checkAlertingRequest() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("checkAlertingRequest() got = %v, want %v", got, tt.want)
}
})
}
}
23 changes: 21 additions & 2 deletions pkg/plugin/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ type Data struct {

// Response contains fields from query response
type Response struct {
Status string `json:"status"`
Data Data `json:"data"`
Status string `json:"status"`
Data Data `json:"data"`
ForAlerting bool `json:"-"`
}

type promInstant struct {
Expand All @@ -61,6 +62,21 @@ func (pi promInstant) dataframes() (data.Frames, error) {
return frames, nil
}

func (pi *promInstant) alertingDataFrames() (data.Frames, error) {
frames := make(data.Frames, len(pi.Result))
for i, res := range pi.Result {
f, err := strconv.ParseFloat(res.Value[1].(string), 64)
if err != nil {
return nil, fmt.Errorf("metric %v, unable to parse float64 from %s: %w", res, res.Value[1], err)
}

frames[i] = data.NewFrame("",
data.NewField(data.TimeSeriesValueFieldName, data.Labels(res.Labels), []float64{f}))
}

return frames, nil
}

type promRange struct {
Result []Result `json:"result"`
}
Expand Down Expand Up @@ -120,6 +136,9 @@ func (r *Response) getDataFrames() (data.Frames, error) {
if err := json.Unmarshal(r.Data.Result, &pi.Result); err != nil {
return nil, fmt.Errorf("unmarshal err %s; \n %#v", err, string(r.Data.Result))
}
if r.ForAlerting {
return pi.alertingDataFrames()
}
return pi.dataframes()
case matrix:
var pr promRange
Expand Down
114 changes: 61 additions & 53 deletions pkg/plugin/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,21 @@ import (
)

func TestResponse_getDataFrames(t *testing.T) {
type fields struct {
Status string
Data Data
}
tests := []struct {
name string
fields fields
query Query
want func() data.Frames
wantErr bool
name string
status string
data Data
forAlerting bool
query Query
want func() data.Frames
wantErr bool
}{
{
name: "empty data",
fields: fields{
Status: "",
Data: Data{
ResultType: "",
Result: nil,
},
name: "empty data",
status: "",
data: Data{
ResultType: "",
Result: nil,
},
query: Query{},
want: func() data.Frames {
Expand All @@ -36,13 +32,11 @@ func TestResponse_getDataFrames(t *testing.T) {
wantErr: true,
},
{
name: "incorrect result type",
fields: fields{
Status: "success",
Data: Data{
ResultType: "abc",
Result: nil,
},
name: "incorrect result type",
status: "success",
data: Data{
ResultType: "abc",
Result: nil,
},
query: Query{LegendFormat: "legend {{app}}"},
want: func() data.Frames {
Expand All @@ -51,13 +45,11 @@ func TestResponse_getDataFrames(t *testing.T) {
wantErr: true,
},
{
name: "bad json",
fields: fields{
Status: "success",
Data: Data{
ResultType: "success",
Result: []byte("{"),
},
name: "bad json",
status: "success",
data: Data{
ResultType: "success",
Result: []byte("{"),
},
query: Query{LegendFormat: "legend {{app}}"},
want: func() data.Frames {
Expand All @@ -66,13 +58,11 @@ func TestResponse_getDataFrames(t *testing.T) {
wantErr: true,
},
{
name: "scalar response",
fields: fields{
Status: "success",
Data: Data{
ResultType: "scalar",
Result: []byte(`[1583786142, "1"]`),
},
name: "scalar response",
status: "success",
data: Data{
ResultType: "scalar",
Result: []byte(`[1583786142, "1"]`),
},
query: Query{LegendFormat: "legend {{app}}"},
want: func() data.Frames {
Expand All @@ -86,13 +76,11 @@ func TestResponse_getDataFrames(t *testing.T) {
wantErr: false,
},
{
name: "vector response",
fields: fields{
Status: "success",
Data: Data{
ResultType: "vector",
Result: []byte(`[{"metric":{"__name__":"vm_rows"},"value":[1583786142,"13763"]},{"metric":{"__name__":"vm_requests"},"value":[1583786140,"2000"]}]`),
},
name: "vector response",
status: "success",
data: Data{
ResultType: "vector",
Result: []byte(`[{"metric":{"__name__":"vm_rows"},"value":[1583786142,"13763"]},{"metric":{"__name__":"vm_requests"},"value":[1583786140,"2000"]}]`),
},
query: Query{LegendFormat: "legend {{app}}"},
want: func() data.Frames {
Expand All @@ -110,13 +98,11 @@ func TestResponse_getDataFrames(t *testing.T) {
wantErr: false,
},
{
name: "matrix response",
fields: fields{
Status: "success",
Data: Data{
ResultType: "matrix",
Result: []byte(`[{"metric":{"__name__":"ingress_nginx_request_qps","status":"100"},"values":[[1670324477.542,"1"]]}, {"metric":{"__name__":"ingress_nginx_request_qps","status":"500"},"values":[[1670324477.542,"2"]]}, {"metric":{"__name__":"ingress_nginx_request_qps","status":"200"},"values":[[1670324477.542,"3"]]}]`),
},
name: "matrix response",
status: "success",
data: Data{
ResultType: "matrix",
Result: []byte(`[{"metric":{"__name__":"ingress_nginx_request_qps","status":"100"},"values":[[1670324477.542,"1"]]}, {"metric":{"__name__":"ingress_nginx_request_qps","status":"500"},"values":[[1670324477.542,"2"]]}, {"metric":{"__name__":"ingress_nginx_request_qps","status":"200"},"values":[[1670324477.542,"3"]]}]`),
},
query: Query{LegendFormat: "legend {{app}}"},
want: func() data.Frames {
Expand All @@ -137,12 +123,34 @@ func TestResponse_getDataFrames(t *testing.T) {
},
wantErr: false,
},
{
name: "vector response for alerting",
status: "success",
data: Data{
ResultType: "vector",
Result: []byte(`[{"metric":{"__name__":"vm_rows"},"value":[1583786142,"13763"]},{"metric":{"__name__":"vm_requests"},"value":[1583786140,"2000"]}]`),
},
forAlerting: true,
query: Query{LegendFormat: "legend {{app}}"},
want: func() data.Frames {
return []*data.Frame{
data.NewFrame("",
data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "vm_rows"}, []float64{13763}),
),
data.NewFrame("",
data.NewField(data.TimeSeriesValueFieldName, data.Labels{"__name__": "vm_requests"}, []float64{2000}),
),
}
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := &Response{
Status: tt.fields.Status,
Data: tt.fields.Data,
Status: tt.status,
Data: tt.data,
ForAlerting: tt.forAlerting,
}
got, err := r.getDataFrames()
if (err != nil) != tt.wantErr {
Expand Down
7 changes: 4 additions & 3 deletions src/querybuilder/state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ export function getQueryWithDefaults(query: PromQuery, app: CoreApp | undefined)
}

// Unified Alerting does not support "both" for query type – fall back to "range".
if (app === CoreApp.UnifiedAlerting) {
const isBothInstantAndRange = query.instant && query.range;
result = { ...result, range: isBothInstantAndRange, instant: !isBothInstantAndRange };
const isBothInstantAndRange = query.instant && query.range;
if (app === CoreApp.UnifiedAlerting && isBothInstantAndRange) {
result = { ...result, instant: false, range: true };
}

return result;
}

0 comments on commit 11a08d1

Please sign in to comment.