From 2c862678abd8b37a02952290217b8f443d7f524b Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Fri, 16 Apr 2021 09:29:19 -0400 Subject: [PATCH] AlertingNG/SSE: Datasource UID and UID/ID only (drop name) (#33039) SSE still will support ID until dashboard/frontend always requests UID update *.http examples Co-authored-by: gotjosh --- pkg/expr/graph.go | 9 +++++-- pkg/expr/nodes.go | 14 ++++++++++- pkg/expr/translate/translate.go | 10 ++++---- .../ruler-grafana-recipient-unauthorized.http | 6 ----- .../test-data/ruler-grafana-recipient.http | 8 ------ pkg/services/ngalert/api/test-data/test.http | 4 +-- pkg/services/ngalert/models/alert_query.go | 15 ++--------- .../ngalert/models/alert_query_test.go | 25 ++----------------- pkg/services/ngalert/tests/database_test.go | 12 ++++----- pkg/services/ngalert/tests/util.go | 2 +- .../api/alerting/api_alertmanager_test.go | 6 ++--- 11 files changed, 40 insertions(+), 71 deletions(-) diff --git a/pkg/expr/graph.go b/pkg/expr/graph.go index d22a80e8526..1bb40e195f3 100644 --- a/pkg/expr/graph.go +++ b/pkg/expr/graph.go @@ -134,9 +134,14 @@ func (s *Service) buildGraph(req *backend.QueryDataRequest) (*simple.DirectedGra return nil, err } + dsUID, err := rn.GetDatasourceUid() + if err != nil { + return nil, err + } + var node graph.Node - switch dsName { - case DatasourceName: + switch { + case dsName == DatasourceName || dsUID == DatasourceUID: node, err = buildCMDNode(dp, rn) default: // If it's not an expression query, it's a data source query. node, err = s.buildDSNode(dp, rn, req.PluginContext.OrgID) diff --git a/pkg/expr/nodes.go b/pkg/expr/nodes.go index 225ce773d1f..32fcadfd858 100644 --- a/pkg/expr/nodes.go +++ b/pkg/expr/nodes.go @@ -30,7 +30,7 @@ type rawNode struct { func (rn *rawNode) GetDatasourceName() (string, error) { rawDs, ok := rn.Query["datasource"] if !ok { - return "", fmt.Errorf("no datasource in query for refId %v", rn.RefID) + return "", nil } dsName, ok := rawDs.(string) if !ok { @@ -39,6 +39,18 @@ func (rn *rawNode) GetDatasourceName() (string, error) { return dsName, nil } +func (rn *rawNode) GetDatasourceUid() (string, error) { + rawDs, ok := rn.Query["datasourceUid"] + if !ok { + return "", nil + } + dsUID, ok := rawDs.(string) + if !ok { + return "", fmt.Errorf("expected datasource identifier to be a string, got %T", rawDs) + } + return dsUID, nil +} + func (rn *rawNode) GetCommandType() (c CommandType, err error) { rawType, ok := rn.Query["type"] if !ok { diff --git a/pkg/expr/translate/translate.go b/pkg/expr/translate/translate.go index 4369e168e66..f85aebc0845 100644 --- a/pkg/expr/translate/translate.go +++ b/pkg/expr/translate/translate.go @@ -240,14 +240,14 @@ func (dc *dashConditionsJSON) GetNew(orgID int64) (*ngmodels.Condition, error) { ngCond.OrgID = orgID exprModel := struct { - Type string `json:"type"` - RefID string `json:"refId"` - Datasource string `json:"datasource"` - Conditions []classic.ClassicConditionJSON `json:"conditions"` + Type string `json:"type"` + RefID string `json:"refId"` + DatasourceUID string `json:"datasourceUid"` + Conditions []classic.ClassicConditionJSON `json:"conditions"` }{ "classic_conditions", ccRefID, - "__expr__", + expr.DatasourceUID, conditions, } diff --git a/pkg/services/ngalert/api/test-data/ruler-grafana-recipient-unauthorized.http b/pkg/services/ngalert/api/test-data/ruler-grafana-recipient-unauthorized.http index c23f6754d19..ea8cc00f888 100644 --- a/pkg/services/ngalert/api/test-data/ruler-grafana-recipient-unauthorized.http +++ b/pkg/services/ngalert/api/test-data/ruler-grafana-recipient-unauthorized.http @@ -77,7 +77,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "gdev-prometheus", "datasourceUid": "000000002", "expr": "http_request_duration_microseconds_count", "hide": false, @@ -96,7 +95,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "query", "hide": false, @@ -115,7 +113,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "$reduced > 10", "hide": false, @@ -162,7 +159,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "gdev-prometheus", "datasourceUid": "000000002", "expr": "http_request_duration_microseconds_count", "hide": false, @@ -181,7 +177,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "query", "hide": false, @@ -200,7 +195,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "$reduced > 42", "hide": false, diff --git a/pkg/services/ngalert/api/test-data/ruler-grafana-recipient.http b/pkg/services/ngalert/api/test-data/ruler-grafana-recipient.http index dd213243d1c..7ebe5a6360a 100644 --- a/pkg/services/ngalert/api/test-data/ruler-grafana-recipient.http +++ b/pkg/services/ngalert/api/test-data/ruler-grafana-recipient.http @@ -100,7 +100,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "query", "hide": false, @@ -119,7 +118,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "$reduced > 10", "hide": false, @@ -170,7 +168,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "gdev-prometheus", "datasourceUid": "000000002", "expr": "http_request_duration_microseconds_count", "hide": false, @@ -189,7 +186,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "query", "hide": false, @@ -208,7 +204,6 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "$reduced > 42", "hide": false, @@ -272,7 +267,6 @@ Content-Type: application/json "to": 10800 }, "model": { - "datasource": "gdev-prometheus", "datasourceUid": "000000002", "expr": "http_request_duration_microseconds_count", "hide": false, @@ -291,7 +285,6 @@ Content-Type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "query", "hide": false, @@ -310,7 +303,6 @@ Content-Type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", "datasourceUid": "-100", "expression": "$reduced > 42", "hide": false, diff --git a/pkg/services/ngalert/api/test-data/test.http b/pkg/services/ngalert/api/test-data/test.http index 9476cc0f146..3d3f311f0b9 100644 --- a/pkg/services/ngalert/api/test-data/test.http +++ b/pkg/services/ngalert/api/test-data/test.http @@ -16,7 +16,7 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"1 < 2" } @@ -68,7 +68,7 @@ content-type: application/json "to": 10800 }, "model": { - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"1 < 2" } diff --git a/pkg/services/ngalert/models/alert_query.go b/pkg/services/ngalert/models/alert_query.go index 712528f5625..c8245ebc531 100644 --- a/pkg/services/ngalert/models/alert_query.go +++ b/pkg/services/ngalert/models/alert_query.go @@ -86,8 +86,8 @@ func (aq *AlertQuery) setModelProps() error { return nil } -// setDatasource sets DatasourceID. -// If it's an expression sets DefaultExprDatasourceID. +// setDatasource sets DatasourceUID. +// If it's an expression sets DefaultExprDatasourceUID. func (aq *AlertQuery) setDatasource() error { if aq.modelProps == nil { err := aq.setModelProps() @@ -96,17 +96,6 @@ func (aq *AlertQuery) setDatasource() error { } } - dsName, ok := aq.modelProps["datasource"] - if !ok { - return fmt.Errorf("failed to get datasource from query model") - } - - if dsName == expr.DatasourceName { - aq.DatasourceUID = expr.DatasourceUID - aq.modelProps["datasourceUid"] = expr.DatasourceUID - return nil - } - i, ok := aq.modelProps["datasourceUid"] if !ok { return fmt.Errorf("failed to get datasourceUid from query model") diff --git a/pkg/services/ngalert/models/alert_query_test.go b/pkg/services/ngalert/models/alert_query_test.go index f3064eebc81..cef2a10ea36 100644 --- a/pkg/services/ngalert/models/alert_query_test.go +++ b/pkg/services/ngalert/models/alert_query_test.go @@ -27,13 +27,12 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "queryType": "metricQuery", "extraParam": "some text" }`), }, expectedIsExpression: true, - expectedDatasource: expr.DatasourceName, expectedDatasourceUID: expr.DatasourceUID, expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: int64(defaultIntervalMS), @@ -43,14 +42,12 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "extraParam": "some text" }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: int64(defaultIntervalMS), @@ -60,7 +57,6 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "maxDataPoints": 200, @@ -68,7 +64,6 @@ func TestAlertQuery(t *testing.T) { }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: 200, expectedIntervalMS: int64(defaultIntervalMS), @@ -78,7 +73,6 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "maxDataPoints": "invalid", @@ -86,7 +80,6 @@ func TestAlertQuery(t *testing.T) { }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: int64(defaultIntervalMS), @@ -96,7 +89,6 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "maxDataPoints": 0, @@ -104,7 +96,6 @@ func TestAlertQuery(t *testing.T) { }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: int64(defaultIntervalMS), @@ -114,7 +105,6 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "intervalMs": 2000, @@ -122,7 +112,6 @@ func TestAlertQuery(t *testing.T) { }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: 2000, @@ -132,7 +121,6 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "intervalMs": "invalid", @@ -140,7 +128,6 @@ func TestAlertQuery(t *testing.T) { }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: int64(defaultIntervalMS), @@ -150,7 +137,6 @@ func TestAlertQuery(t *testing.T) { alertQuery: AlertQuery{ RefID: "A", Model: json.RawMessage(`{ - "datasource": "my datasource", "datasourceUid": "000000001", "queryType": "metricQuery", "intervalMs": 0, @@ -158,7 +144,6 @@ func TestAlertQuery(t *testing.T) { }`), }, expectedIsExpression: false, - expectedDatasource: "my datasource", expectedDatasourceUID: "000000001", expectedMaxPoints: int64(defaultMaxDataPoints), expectedIntervalMS: int64(defaultIntervalMS), @@ -204,13 +189,7 @@ func TestAlertQuery(t *testing.T) { err = json.Unmarshal(blob, &model) require.NoError(t, err) - i, ok := model["datasource"] - require.True(t, ok) - datasource, ok := i.(string) - require.True(t, ok) - require.Equal(t, tc.expectedDatasource, datasource) - - i, ok = model["datasourceUid"] + i, ok := model["datasourceUid"] require.True(t, ok) datasourceUID, ok := i.(string) require.True(t, ok) diff --git a/pkg/services/ngalert/tests/database_test.go b/pkg/services/ngalert/tests/database_test.go index 0eee9a22408..3f2a4b079d0 100644 --- a/pkg/services/ngalert/tests/database_test.go +++ b/pkg/services/ngalert/tests/database_test.go @@ -88,7 +88,7 @@ func TestCreatingAlertDefinition(t *testing.T) { Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 3 > 1" }`), @@ -129,7 +129,7 @@ func TestCreatingConflictionAlertDefinition(t *testing.T) { Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 3 > 1" }`), @@ -167,7 +167,7 @@ func TestUpdatingAlertDefinition(t *testing.T) { Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 2 > 1" }`), @@ -257,7 +257,7 @@ func TestUpdatingAlertDefinition(t *testing.T) { Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 3 > 1" }`), @@ -341,7 +341,7 @@ func TestUpdatingConflictingAlertDefinition(t *testing.T) { Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 3 > 1" }`), @@ -430,7 +430,7 @@ func createTestAlertDefinition(t *testing.T, dbstore *store.DBstore, intervalSec Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 2 > 1" }`), diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 79a5435cf9d..662bb9620e7 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -83,7 +83,7 @@ func createTestAlertRule(t *testing.T, dbstore *store.DBstore, intervalSeconds i Data: []models.AlertQuery{ { Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type":"math", "expression":"2 + 2 > 1" }`), diff --git a/pkg/tests/api/alerting/api_alertmanager_test.go b/pkg/tests/api/alerting/api_alertmanager_test.go index 4d5bde345b3..167df9a3940 100644 --- a/pkg/tests/api/alerting/api_alertmanager_test.go +++ b/pkg/tests/api/alerting/api_alertmanager_test.go @@ -91,7 +91,7 @@ func TestAlertRuleCRUD(t *testing.T) { To: ngmodels.Duration(time.Duration(3) * time.Hour), }, Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type": "math", "expression": "2 + 3 > 1" }`), @@ -112,7 +112,7 @@ func TestAlertRuleCRUD(t *testing.T) { To: ngmodels.Duration(time.Duration(3) * time.Hour), }, Model: json.RawMessage(`{ - "datasource": "__expr__", + "datasourceUid": "-100", "type": "math", "expression": "2 + 3 > 1" }`), @@ -180,7 +180,6 @@ func TestAlertRuleCRUD(t *testing.T) { "to":10800 }, "model":{ - "datasource":"__expr__", "datasourceUid":"-100", "expression":"2 + 3 \u003e 1", "intervalMs":1000, @@ -216,7 +215,6 @@ func TestAlertRuleCRUD(t *testing.T) { "to":10800 }, "model":{ - "datasource":"__expr__", "datasourceUid":"-100", "expression":"2 + 3 \u003e 1", "intervalMs":1000,