diff --git a/apps/dashboard/pkg/migration/conversion/testdata/input/v1beta1.bom-in-links.json b/apps/dashboard/pkg/migration/conversion/testdata/input/v1beta1.bom-in-links.json new file mode 100644 index 00000000000..86992c3380c --- /dev/null +++ b/apps/dashboard/pkg/migration/conversion/testdata/input/v1beta1.bom-in-links.json @@ -0,0 +1,142 @@ +{ + "kind": "Dashboard", + "apiVersion": "dashboard.grafana.app/v1beta1", + "metadata": { + "name": "bom-in-links-test", + "namespace": "org-1", + "labels": { + "test": "bom-stripping" + } + }, + "spec": { + "title": "BOM Stripping Test Dashboard", + "description": "Testing that BOM characters are stripped from URLs during conversion", + "schemaVersion": 42, + "tags": ["test", "bom"], + "editable": true, + "links": [ + { + "title": "Dashboard link with BOM", + "type": "link", + "url": "http://example.com?var=${datasource}&other=value", + "targetBlank": true, + "icon": "external link" + } + ], + "panels": [ + { + "id": 1, + "type": "table", + "title": "Panel with BOM in field config override links", + "gridPos": { + "h": 8, + "w": 12, + "x": 0, + "y": 0 + }, + "fieldConfig": { + "defaults": { + "custom": {}, + "mappings": [], + "thresholds": { + "mode": "absolute", + "steps": [ + {"color": "green"}, + {"color": "red", "value": 80} + ] + } + }, + "overrides": [ + { + "matcher": { + "id": "byName", + "options": "server" + }, + "properties": [ + { + "id": "links", + "value": [ + { + "title": "Override link with BOM", + "url": "http://localhost:3000/d/test?var-datacenter=${__data.fields[datacenter]}&var-server=${__value.raw}" + } + ] + } + ] + } + ] + }, + "links": [ + { + "title": "Panel data link with BOM", + "url": "http://example.com/${__data.fields.cluster}&var=value", + "targetBlank": true + } + ], + "targets": [ + { + "refId": "A", + "datasource": { + "type": "prometheus", + "uid": "test-ds" + } + } + ] + }, + { + "id": 2, + "type": "timeseries", + "title": "Panel with BOM in options dataLinks", + "gridPos": { + "h": 8, + "w": 12, + "x": 12, + "y": 0 + }, + "options": { + "legend": { + "showLegend": true, + "displayMode": "list", + "placement": "bottom" + }, + "dataLinks": [ + { + "title": "Options data link with BOM", + "url": "http://example.com?series=${__series.name}&time=${__value.time}", + "targetBlank": true + } + ] + }, + "fieldConfig": { + "defaults": { + "links": [ + { + "title": "Field config default link with BOM", + "url": "http://example.com?field=${__field.name}&value=${__value.raw}", + "targetBlank": false + } + ] + }, + "overrides": [] + }, + "targets": [ + { + "refId": "A", + "datasource": { + "type": "prometheus", + "uid": "test-ds" + } + } + ] + } + ], + "time": { + "from": "now-6h", + "to": "now" + }, + "timepicker": { + "refresh_intervals": ["5s", "10s", "30s", "1m", "5m"] + } + } +} + diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2alpha1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2alpha1.json index 89857905689..b1dbd3de041 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2alpha1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2alpha1.json @@ -120,7 +120,7 @@ "value": [ { "title": "filter", - "url": "http://localhost:3000/d/-Y-tnEDWk/templating-nested-template-variables?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" + "url": "http://localhost:3000/d/-Y-tnEDWk/templating-nested-template-variables?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" } ] } diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2beta1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2beta1.json index 13320b47904..9089dd1d1fb 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2beta1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/feature-templating/v0alpha1.testdata-nested-variables.v42.v2beta1.json @@ -124,7 +124,7 @@ "value": [ { "title": "filter", - "url": "http://localhost:3000/d/-Y-tnEDWk/templating-nested-template-variables?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" + "url": "http://localhost:3000/d/-Y-tnEDWk/templating-nested-template-variables?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" } ] } diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v1beta1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v1beta1.json index e04d448a5b8..66ce1cd0f3a 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v1beta1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v1beta1.json @@ -2051,4 +2051,4 @@ "storedVersion": "v0alpha1" } } -} +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2alpha1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2alpha1.json index 0e6e3e13da5..95850646c59 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2alpha1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2alpha1.json @@ -2691,4 +2691,4 @@ "storedVersion": "v0alpha1" } } -} +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2beta1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2beta1.json index ad2b8ca0385..fda0d31e71b 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2beta1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_new.v42.v2beta1.json @@ -2764,4 +2764,4 @@ "storedVersion": "v0alpha1" } } -} +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v1beta1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v1beta1.json index 1d9f7e56513..2dddd657c5f 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v1beta1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v1beta1.json @@ -1173,4 +1173,4 @@ "storedVersion": "v0alpha1" } } -} +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2alpha1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2alpha1.json index 7b3f601b5cf..db19ac588c1 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2alpha1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2alpha1.json @@ -1618,4 +1618,4 @@ "storedVersion": "v0alpha1" } } -} +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2beta1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2beta1.json index 534e7a1600c..8ddc6feb297 100644 --- a/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2beta1.json +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/migrated_dev_dashboards/panel-gauge/v0alpha1.gauge_tests_old_to_new.v42.v2beta1.json @@ -1670,4 +1670,4 @@ "storedVersion": "v0alpha1" } } -} +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v0alpha1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v0alpha1.json new file mode 100644 index 00000000000..449e76f1173 --- /dev/null +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v0alpha1.json @@ -0,0 +1,161 @@ +{ + "kind": "Dashboard", + "apiVersion": "dashboard.grafana.app/v0alpha1", + "metadata": { + "name": "bom-in-links-test", + "namespace": "org-1", + "labels": { + "test": "bom-stripping" + } + }, + "spec": { + "description": "Testing that BOM characters are stripped from URLs during conversion", + "editable": true, + "links": [ + { + "icon": "external link", + "targetBlank": true, + "title": "Dashboard link with BOM", + "type": "link", + "url": "http://example.com?var=${datasource}\u0026other=value" + } + ], + "panels": [ + { + "fieldConfig": { + "defaults": { + "custom": {}, + "mappings": [], + "thresholds": { + "mode": "absolute", + "steps": [ + { + "color": "green" + }, + { + "color": "red", + "value": 80 + } + ] + } + }, + "overrides": [ + { + "matcher": { + "id": "byName", + "options": "server" + }, + "properties": [ + { + "id": "links", + "value": [ + { + "title": "Override link with BOM", + "url": "http://localhost:3000/d/test?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" + } + ] + } + ] + } + ] + }, + "gridPos": { + "h": 8, + "w": 12, + "x": 0, + "y": 0 + }, + "id": 1, + "links": [ + { + "targetBlank": true, + "title": "Panel data link with BOM", + "url": "http://example.com/${__data.fields.cluster}\u0026var=value" + } + ], + "targets": [ + { + "datasource": { + "type": "prometheus", + "uid": "test-ds" + }, + "refId": "A" + } + ], + "title": "Panel with BOM in field config override links", + "type": "table" + }, + { + "fieldConfig": { + "defaults": { + "links": [ + { + "targetBlank": false, + "title": "Field config default link with BOM", + "url": "http://example.com?field=${__field.name}\u0026value=${__value.raw}" + } + ] + }, + "overrides": [] + }, + "gridPos": { + "h": 8, + "w": 12, + "x": 12, + "y": 0 + }, + "id": 2, + "options": { + "dataLinks": [ + { + "targetBlank": true, + "title": "Options data link with BOM", + "url": "http://example.com?series=${__series.name}\u0026time=${__value.time}" + } + ], + "legend": { + "displayMode": "list", + "placement": "bottom", + "showLegend": true + } + }, + "targets": [ + { + "datasource": { + "type": "prometheus", + "uid": "test-ds" + }, + "refId": "A" + } + ], + "title": "Panel with BOM in options dataLinks", + "type": "timeseries" + } + ], + "schemaVersion": 42, + "tags": [ + "test", + "bom" + ], + "time": { + "from": "now-6h", + "to": "now" + }, + "timepicker": { + "refresh_intervals": [ + "5s", + "10s", + "30s", + "1m", + "5m" + ] + }, + "title": "BOM Stripping Test Dashboard" + }, + "status": { + "conversion": { + "failed": false, + "storedVersion": "v1beta1" + } + } +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v2alpha1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v2alpha1.json new file mode 100644 index 00000000000..38547ea5b8e --- /dev/null +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v2alpha1.json @@ -0,0 +1,242 @@ +{ + "kind": "Dashboard", + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": { + "name": "bom-in-links-test", + "namespace": "org-1", + "labels": { + "test": "bom-stripping" + } + }, + "spec": { + "annotations": [], + "cursorSync": "Off", + "description": "Testing that BOM characters are stripped from URLs during conversion", + "editable": true, + "elements": { + "panel-1": { + "kind": "Panel", + "spec": { + "id": 1, + "title": "Panel with BOM in field config override links", + "description": "", + "links": [ + { + "title": "Panel data link with BOM", + "url": "http://example.com/${__data.fields.cluster}\u0026var=value", + "targetBlank": true + } + ], + "data": { + "kind": "QueryGroup", + "spec": { + "queries": [ + { + "kind": "PanelQuery", + "spec": { + "query": { + "kind": "prometheus", + "spec": {} + }, + "datasource": { + "type": "prometheus", + "uid": "test-ds" + }, + "refId": "A", + "hidden": false + } + } + ], + "transformations": [], + "queryOptions": {} + } + }, + "vizConfig": { + "kind": "table", + "spec": { + "pluginVersion": "", + "options": {}, + "fieldConfig": { + "defaults": { + "thresholds": { + "mode": "absolute", + "steps": [ + { + "value": null, + "color": "green" + }, + { + "value": 80, + "color": "red" + } + ] + } + }, + "overrides": [ + { + "matcher": { + "id": "byName", + "options": "server" + }, + "properties": [ + { + "id": "links", + "value": [ + { + "title": "Override link with BOM", + "url": "http://localhost:3000/d/test?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" + } + ] + } + ] + } + ] + } + } + } + } + }, + "panel-2": { + "kind": "Panel", + "spec": { + "id": 2, + "title": "Panel with BOM in options dataLinks", + "description": "", + "links": [], + "data": { + "kind": "QueryGroup", + "spec": { + "queries": [ + { + "kind": "PanelQuery", + "spec": { + "query": { + "kind": "prometheus", + "spec": {} + }, + "datasource": { + "type": "prometheus", + "uid": "test-ds" + }, + "refId": "A", + "hidden": false + } + } + ], + "transformations": [], + "queryOptions": {} + } + }, + "vizConfig": { + "kind": "timeseries", + "spec": { + "pluginVersion": "", + "options": { + "dataLinks": [ + { + "targetBlank": true, + "title": "Options data link with BOM", + "url": "http://example.com?series=${__series.name}\u0026time=${__value.time}" + } + ], + "legend": { + "displayMode": "list", + "placement": "bottom", + "showLegend": true + } + }, + "fieldConfig": { + "defaults": { + "links": [ + { + "targetBlank": false, + "title": "Field config default link with BOM", + "url": "http://example.com?field=${__field.name}\u0026value=${__value.raw}" + } + ] + }, + "overrides": [] + } + } + } + } + } + }, + "layout": { + "kind": "GridLayout", + "spec": { + "items": [ + { + "kind": "GridLayoutItem", + "spec": { + "x": 0, + "y": 0, + "width": 12, + "height": 8, + "element": { + "kind": "ElementReference", + "name": "panel-1" + } + } + }, + { + "kind": "GridLayoutItem", + "spec": { + "x": 12, + "y": 0, + "width": 12, + "height": 8, + "element": { + "kind": "ElementReference", + "name": "panel-2" + } + } + } + ] + } + }, + "links": [ + { + "title": "Dashboard link with BOM", + "type": "link", + "icon": "external link", + "tooltip": "", + "url": "http://example.com?var=${datasource}\u0026other=value", + "tags": [], + "asDropdown": false, + "targetBlank": true, + "includeVars": false, + "keepTime": false + } + ], + "liveNow": false, + "preload": false, + "tags": [ + "test", + "bom" + ], + "timeSettings": { + "timezone": "browser", + "from": "now-6h", + "to": "now", + "autoRefresh": "", + "autoRefreshIntervals": [ + "5s", + "10s", + "30s", + "1m", + "5m" + ], + "hideTimepicker": false, + "fiscalYearStartMonth": 0 + }, + "title": "BOM Stripping Test Dashboard", + "variables": [] + }, + "status": { + "conversion": { + "failed": false, + "storedVersion": "v1beta1" + } + } +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v2beta1.json b/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v2beta1.json new file mode 100644 index 00000000000..d85da89fe7a --- /dev/null +++ b/apps/dashboard/pkg/migration/conversion/testdata/output/v1beta1.bom-in-links.v2beta1.json @@ -0,0 +1,246 @@ +{ + "kind": "Dashboard", + "apiVersion": "dashboard.grafana.app/v2beta1", + "metadata": { + "name": "bom-in-links-test", + "namespace": "org-1", + "labels": { + "test": "bom-stripping" + } + }, + "spec": { + "annotations": [], + "cursorSync": "Off", + "description": "Testing that BOM characters are stripped from URLs during conversion", + "editable": true, + "elements": { + "panel-1": { + "kind": "Panel", + "spec": { + "id": 1, + "title": "Panel with BOM in field config override links", + "description": "", + "links": [ + { + "title": "Panel data link with BOM", + "url": "http://example.com/${__data.fields.cluster}\u0026var=value", + "targetBlank": true + } + ], + "data": { + "kind": "QueryGroup", + "spec": { + "queries": [ + { + "kind": "PanelQuery", + "spec": { + "query": { + "kind": "DataQuery", + "group": "prometheus", + "version": "v0", + "datasource": { + "name": "test-ds" + }, + "spec": {} + }, + "refId": "A", + "hidden": false + } + } + ], + "transformations": [], + "queryOptions": {} + } + }, + "vizConfig": { + "kind": "VizConfig", + "group": "table", + "version": "", + "spec": { + "options": {}, + "fieldConfig": { + "defaults": { + "thresholds": { + "mode": "absolute", + "steps": [ + { + "value": null, + "color": "green" + }, + { + "value": 80, + "color": "red" + } + ] + } + }, + "overrides": [ + { + "matcher": { + "id": "byName", + "options": "server" + }, + "properties": [ + { + "id": "links", + "value": [ + { + "title": "Override link with BOM", + "url": "http://localhost:3000/d/test?var-datacenter=${__data.fields[datacenter]}\u0026var-server=${__value.raw}" + } + ] + } + ] + } + ] + } + } + } + } + }, + "panel-2": { + "kind": "Panel", + "spec": { + "id": 2, + "title": "Panel with BOM in options dataLinks", + "description": "", + "links": [], + "data": { + "kind": "QueryGroup", + "spec": { + "queries": [ + { + "kind": "PanelQuery", + "spec": { + "query": { + "kind": "DataQuery", + "group": "prometheus", + "version": "v0", + "datasource": { + "name": "test-ds" + }, + "spec": {} + }, + "refId": "A", + "hidden": false + } + } + ], + "transformations": [], + "queryOptions": {} + } + }, + "vizConfig": { + "kind": "VizConfig", + "group": "timeseries", + "version": "", + "spec": { + "options": { + "dataLinks": [ + { + "targetBlank": true, + "title": "Options data link with BOM", + "url": "http://example.com?series=${__series.name}\u0026time=${__value.time}" + } + ], + "legend": { + "displayMode": "list", + "placement": "bottom", + "showLegend": true + } + }, + "fieldConfig": { + "defaults": { + "links": [ + { + "targetBlank": false, + "title": "Field config default link with BOM", + "url": "http://example.com?field=${__field.name}\u0026value=${__value.raw}" + } + ] + }, + "overrides": [] + } + } + } + } + } + }, + "layout": { + "kind": "GridLayout", + "spec": { + "items": [ + { + "kind": "GridLayoutItem", + "spec": { + "x": 0, + "y": 0, + "width": 12, + "height": 8, + "element": { + "kind": "ElementReference", + "name": "panel-1" + } + } + }, + { + "kind": "GridLayoutItem", + "spec": { + "x": 12, + "y": 0, + "width": 12, + "height": 8, + "element": { + "kind": "ElementReference", + "name": "panel-2" + } + } + } + ] + } + }, + "links": [ + { + "title": "Dashboard link with BOM", + "type": "link", + "icon": "external link", + "tooltip": "", + "url": "http://example.com?var=${datasource}\u0026other=value", + "tags": [], + "asDropdown": false, + "targetBlank": true, + "includeVars": false, + "keepTime": false + } + ], + "liveNow": false, + "preload": false, + "tags": [ + "test", + "bom" + ], + "timeSettings": { + "timezone": "browser", + "from": "now-6h", + "to": "now", + "autoRefresh": "", + "autoRefreshIntervals": [ + "5s", + "10s", + "30s", + "1m", + "5m" + ], + "hideTimepicker": false, + "fiscalYearStartMonth": 0 + }, + "title": "BOM Stripping Test Dashboard", + "variables": [] + }, + "status": { + "conversion": { + "failed": false, + "storedVersion": "v1beta1" + } + } +} \ No newline at end of file diff --git a/apps/dashboard/pkg/migration/conversion/v1beta1_to_v2alpha1.go b/apps/dashboard/pkg/migration/conversion/v1beta1_to_v2alpha1.go index 224f222ae33..b63e0146cc2 100644 --- a/apps/dashboard/pkg/migration/conversion/v1beta1_to_v2alpha1.go +++ b/apps/dashboard/pkg/migration/conversion/v1beta1_to_v2alpha1.go @@ -229,6 +229,36 @@ func getBoolField(m map[string]interface{}, key string, defaultValue bool) bool return defaultValue } +// stripBOM removes Byte Order Mark (BOM) characters from a string. +// BOMs (U+FEFF) can be introduced through copy/paste from certain editors +// and cause CUE validation errors ("illegal byte order mark"). +func stripBOM(s string) string { + return strings.ReplaceAll(s, "\ufeff", "") +} + +// stripBOMFromInterface recursively strips BOM characters from all strings +// in an interface{} value (map, slice, or string). +func stripBOMFromInterface(v interface{}) interface{} { + switch val := v.(type) { + case string: + return stripBOM(val) + case map[string]interface{}: + result := make(map[string]interface{}, len(val)) + for k, v := range val { + result[k] = stripBOMFromInterface(v) + } + return result + case []interface{}: + result := make([]interface{}, len(val)) + for i, item := range val { + result[i] = stripBOMFromInterface(item) + } + return result + default: + return v + } +} + func getUnionField[T ~string](m map[string]interface{}, key string) *T { if val, ok := m[key]; ok { if str, ok := val.(string); ok && str != "" { @@ -393,7 +423,8 @@ func transformLinks(dashboard map[string]interface{}) []dashv2alpha1.DashboardDa // Optional field - only set if present if url, exists := linkMap["url"]; exists { if urlStr, ok := url.(string); ok { - dashLink.Url = &urlStr + cleanUrl := stripBOM(urlStr) + dashLink.Url = &cleanUrl } } @@ -2239,7 +2270,7 @@ func transformDataLinks(panelMap map[string]interface{}) []dashv2alpha1.Dashboar if linkMap, ok := link.(map[string]interface{}); ok { dataLink := dashv2alpha1.DashboardDataLink{ Title: schemaversion.GetStringValue(linkMap, "title"), - Url: schemaversion.GetStringValue(linkMap, "url"), + Url: stripBOM(schemaversion.GetStringValue(linkMap, "url")), } if _, exists := linkMap["targetBlank"]; exists { targetBlank := getBoolField(linkMap, "targetBlank", false) @@ -2331,6 +2362,12 @@ func buildVizConfig(panelMap map[string]interface{}) dashv2alpha1.DashboardVizCo } } + // Strip BOMs from options (may contain dataLinks with URLs that have BOMs) + cleanedOptions := stripBOMFromInterface(options) + if cleanedMap, ok := cleanedOptions.(map[string]interface{}); ok { + options = cleanedMap + } + // Build field config by mapping each field individually fieldConfigSource := extractFieldConfigSource(fieldConfig) @@ -2474,9 +2511,14 @@ func extractFieldConfigDefaults(defaults map[string]interface{}) dashv2alpha1.Da hasDefaults = true } - // Extract array field + // Extract array field - strip BOMs from link URLs if linksArray, ok := extractArrayField(defaults, "links"); ok { - fieldConfigDefaults.Links = linksArray + cleanedLinks := stripBOMFromInterface(linksArray) + if cleanedArray, ok := cleanedLinks.([]interface{}); ok { + fieldConfigDefaults.Links = cleanedArray + } else { + fieldConfigDefaults.Links = linksArray + } hasDefaults = true } @@ -2762,9 +2804,11 @@ func extractFieldConfigOverrides(fieldConfig map[string]interface{}) []dashv2alp fieldOverride.Properties = make([]dashv2alpha1.DashboardDynamicConfigValue, 0, len(propertiesArray)) for _, property := range propertiesArray { if propertyMap, ok := property.(map[string]interface{}); ok { + // Strip BOMs from property values (may contain links with URLs) + cleanedValue := stripBOMFromInterface(propertyMap["value"]) fieldOverride.Properties = append(fieldOverride.Properties, dashv2alpha1.DashboardDynamicConfigValue{ Id: schemaversion.GetStringValue(propertyMap, "id"), - Value: propertyMap["value"], + Value: cleanedValue, }) } } diff --git a/e2e-playwright/panels-suite/canvas-scene.spec.ts b/e2e-playwright/panels-suite/canvas-scene.spec.ts index b1fc028f3ae..c0b392d544b 100644 --- a/e2e-playwright/panels-suite/canvas-scene.spec.ts +++ b/e2e-playwright/panels-suite/canvas-scene.spec.ts @@ -2,18 +2,16 @@ import { Locator } from '@playwright/test'; import { test, expect } from '@grafana/plugin-e2e'; -import { setVisualization } from './vizpicker-utils'; - test.use({ featureToggles: { canvasPanelPanZoom: true, }, }); test.describe('Canvas Panel - Scene Tests', () => { - test.beforeEach(async ({ page, gotoDashboardPage, selectors }) => { + test.beforeEach(async ({ page, gotoDashboardPage }) => { const dashboardPage = await gotoDashboardPage({}); const panelEditPage = await dashboardPage.addPanel(); - await setVisualization(panelEditPage, 'Canvas', selectors); + await panelEditPage.setVisualization('Canvas'); // Wait for canvas panel to load await page.waitForSelector('[data-testid="canvas-scene-pan-zoom"]', { timeout: 10000 }); diff --git a/e2e-playwright/panels-suite/vizpicker-utils.ts b/e2e-playwright/panels-suite/vizpicker-utils.ts deleted file mode 100644 index 1785dd7e04a..00000000000 --- a/e2e-playwright/panels-suite/vizpicker-utils.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { expect, E2ESelectorGroups, PanelEditPage } from '@grafana/plugin-e2e'; - -// this replaces the panelEditPage.setVisualization method used previously in tests, since it -// does not know how to use the updated 12.4 viz picker UI to set the visualization -export const setVisualization = async (panelEditPage: PanelEditPage, vizName: string, selectors: E2ESelectorGroups) => { - const vizPicker = panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.toggleVizPicker); - await expect(vizPicker, '"Change" button should be visible').toBeVisible(); - await vizPicker.click(); - - const allVizTabBtn = panelEditPage.getByGrafanaSelector(selectors.components.Tab.title('All visualizations')); - await expect(allVizTabBtn, '"All visualiations" button should be visible').toBeVisible(); - await allVizTabBtn.click(); - - const vizItem = panelEditPage.getByGrafanaSelector(selectors.components.PluginVisualization.item(vizName)); - await expect(vizItem, `"${vizName}" item should be visible`).toBeVisible(); - await vizItem.scrollIntoViewIfNeeded(); - await vizItem.click(); - - await expect(vizPicker, '"Change" button should be visible again').toBeVisible(); - await expect( - panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.header), - 'Panel header should have the new viz type name' - ).toHaveText(vizName); -}; diff --git a/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelDataAssertion.spec.ts b/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelDataAssertion.spec.ts index 336dbef0a29..0133a3e3712 100644 --- a/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelDataAssertion.spec.ts +++ b/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelDataAssertion.spec.ts @@ -1,6 +1,5 @@ import { expect, test } from '@grafana/plugin-e2e'; -import { setVisualization } from '../../../panels-suite/vizpicker-utils'; import { formatExpectError } from '../errors'; import { successfulDataQuery } from '../mocks/queries'; @@ -25,10 +24,10 @@ test.describe( ).toContainText(['Field', 'Max', 'Mean', 'Last']); }); - test('table panel data assertions', async ({ panelEditPage, selectors }) => { + test('table panel data assertions', async ({ panelEditPage }) => { await panelEditPage.mockQueryDataResponse(successfulDataQuery, 200); await panelEditPage.datasource.set('gdev-testdata'); - await setVisualization(panelEditPage, 'Table', selectors); + await panelEditPage.setVisualization('Table'); await panelEditPage.refreshPanel(); await expect( panelEditPage.panel.locator, @@ -44,10 +43,10 @@ test.describe( ).toContainText(['val1', 'val2', 'val3', 'val4']); }); - test('timeseries panel - table view assertions', async ({ panelEditPage, selectors }) => { + test('timeseries panel - table view assertions', async ({ panelEditPage }) => { await panelEditPage.mockQueryDataResponse(successfulDataQuery, 200); await panelEditPage.datasource.set('gdev-testdata'); - await setVisualization(panelEditPage, 'Time series', selectors); + await panelEditPage.setVisualization('Time series'); await panelEditPage.refreshPanel(); await panelEditPage.toggleTableView(); await expect( diff --git a/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelEditPage.spec.ts b/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelEditPage.spec.ts index 93e0525ab0e..46c36277848 100644 --- a/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelEditPage.spec.ts +++ b/e2e-playwright/plugin-e2e/plugin-e2e-api-tests/as-admin-user/panelEditPage.spec.ts @@ -1,6 +1,5 @@ import { expect, test } from '@grafana/plugin-e2e'; -import { setVisualization } from '../../../panels-suite/vizpicker-utils'; import { formatExpectError } from '../errors'; import { successfulDataQuery } from '../mocks/queries'; import { scenarios } from '../mocks/resources'; @@ -54,10 +53,10 @@ test.describe( ).toHaveText(scenarios.map((s) => s.name)); }); - test('mocked query data response', async ({ panelEditPage, page, selectors }) => { + test('mocked query data response', async ({ panelEditPage, page }) => { await panelEditPage.mockQueryDataResponse(successfulDataQuery, 200); await panelEditPage.datasource.set('gdev-testdata'); - await setVisualization(panelEditPage, TABLE_VIZ_NAME, selectors); + await panelEditPage.setVisualization(TABLE_VIZ_NAME); await panelEditPage.refreshPanel(); await expect( panelEditPage.panel.getErrorIcon(), @@ -76,7 +75,7 @@ test.describe( selectors, page, }) => { - await setVisualization(panelEditPage, TABLE_VIZ_NAME, selectors); + await panelEditPage.setVisualization(TABLE_VIZ_NAME); await expect( panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.header), formatExpectError('Expected panel visualization to be set to table') @@ -93,8 +92,8 @@ test.describe( ).toBeVisible(); }); - test('Select time zone in timezone picker', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('Select time zone in timezone picker', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const axisOptions = await panelEditPage.getCustomOptions('Axis'); const timeZonePicker = axisOptions.getSelect('Time zone'); @@ -102,8 +101,8 @@ test.describe( await expect(timeZonePicker).toHaveSelected('Europe/Stockholm'); }); - test('select unit in unit picker', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('select unit in unit picker', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const standardOptions = panelEditPage.getStandardOptions(); const unitPicker = standardOptions.getUnitPicker('Unit'); @@ -112,8 +111,8 @@ test.describe( await expect(unitPicker).toHaveSelected('Pixels'); }); - test('enter value in number input', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('enter value in number input', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const axisOptions = panelEditPage.getCustomOptions('Axis'); const lineWith = axisOptions.getNumberInput('Soft min'); @@ -122,8 +121,8 @@ test.describe( await expect(lineWith).toHaveValue('10'); }); - test('enter value in slider', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('enter value in slider', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const graphOptions = panelEditPage.getCustomOptions('Graph styles'); const lineWidth = graphOptions.getSliderInput('Line width'); @@ -132,8 +131,8 @@ test.describe( await expect(lineWidth).toHaveValue('10'); }); - test('select value in single value select', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('select value in single value select', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const standardOptions = panelEditPage.getStandardOptions(); const colorSchemeSelect = standardOptions.getSelect('Color scheme'); @@ -141,8 +140,8 @@ test.describe( await expect(colorSchemeSelect).toHaveSelected('Classic palette'); }); - test('clear input', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('clear input', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const panelOptions = panelEditPage.getPanelOptions(); const title = panelOptions.getTextInput('Title'); @@ -151,8 +150,8 @@ test.describe( await expect(title).toHaveValue(''); }); - test('enter value in input', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('enter value in input', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const panelOptions = panelEditPage.getPanelOptions(); const description = panelOptions.getTextInput('Description'); @@ -161,8 +160,8 @@ test.describe( await expect(description).toHaveValue('This is a panel'); }); - test('unchecking switch', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('unchecking switch', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const axisOptions = panelEditPage.getCustomOptions('Axis'); const showBorder = axisOptions.getSwitch('Show border'); @@ -174,8 +173,8 @@ test.describe( await expect(showBorder).toBeChecked({ checked: false }); }); - test('checking switch', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('checking switch', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const axisOptions = panelEditPage.getCustomOptions('Axis'); const showBorder = axisOptions.getSwitch('Show border'); @@ -184,8 +183,8 @@ test.describe( await expect(showBorder).toBeChecked(); }); - test('re-selecting value in radio button group', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('re-selecting value in radio button group', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const axisOptions = panelEditPage.getCustomOptions('Axis'); const placement = axisOptions.getRadioGroup('Placement'); @@ -196,8 +195,8 @@ test.describe( await expect(placement).toHaveChecked('Auto'); }); - test('selecting value in radio button group', async ({ panelEditPage, selectors }) => { - await setVisualization(panelEditPage, TIME_SERIES_VIZ_NAME, selectors); + test('selecting value in radio button group', async ({ panelEditPage }) => { + await panelEditPage.setVisualization(TIME_SERIES_VIZ_NAME); const axisOptions = panelEditPage.getCustomOptions('Axis'); const placement = axisOptions.getRadioGroup('Placement'); diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 981b10dfb1c..04b0b28847c 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -421,6 +421,10 @@ export interface FeatureToggles { */ jitterAlertRulesWithinGroups?: boolean; /** + * Enable audit logging with Kubernetes under app platform + */ + auditLoggingAppPlatform?: boolean; + /** * Enable the secrets management API and services under app platform */ secretsManagementAppPlatform?: boolean; diff --git a/pkg/apiserver/auditing/event.go b/pkg/apiserver/auditing/event.go new file mode 100644 index 00000000000..dc5829096e6 --- /dev/null +++ b/pkg/apiserver/auditing/event.go @@ -0,0 +1,88 @@ +package auditing + +import ( + "encoding/json" + "time" +) + +type Event struct { + // The namespace the action was performed in. + Namespace string `json:"namespace"` + + // When it happened. + ObservedAt time.Time `json:"-"` // see MarshalJSON for why this is omitted + + // Who/what performed the action. + SubjectName string `json:"subjectName"` + SubjectUID string `json:"subjectUID"` + + // What was performed. + Verb string `json:"verb"` + + // The object the action was performed on. For verbs like "list" this will be empty. + Object string `json:"object,omitempty"` + + // API information. + APIGroup string `json:"apiGroup,omitempty"` + APIVersion string `json:"apiVersion,omitempty"` + Kind string `json:"kind,omitempty"` + + // Outcome of the action. + Outcome EventOutcome `json:"outcome"` + + // Extra fields to add more context to the event. + Extra map[string]string `json:"extra,omitempty"` +} + +func (e Event) Time() time.Time { + return e.ObservedAt +} + +func (e Event) MarshalJSON() ([]byte, error) { + type Alias Event + return json.Marshal(&struct { + FormattedTimestamp string `json:"observedAt"` + Alias + }{ + FormattedTimestamp: e.ObservedAt.UTC().Format(time.RFC3339Nano), + Alias: (Alias)(e), + }) +} + +func (e Event) KVPairs() []any { + args := []any{ + "audit", true, + "namespace", e.Namespace, + "observedAt", e.ObservedAt.UTC().Format(time.RFC3339Nano), + "subjectName", e.SubjectName, + "subjectUID", e.SubjectUID, + "verb", e.Verb, + "object", e.Object, + "apiGroup", e.APIGroup, + "apiVersion", e.APIVersion, + "kind", e.Kind, + "outcome", e.Outcome, + } + + if len(e.Extra) > 0 { + extraArgs := make([]any, 0, len(e.Extra)*2) + + for k, v := range e.Extra { + extraArgs = append(extraArgs, "extra_"+k, v) + } + + args = append(args, extraArgs...) + } + + return args +} + +type EventOutcome string + +const ( + EventOutcomeUnknown EventOutcome = "unknown" + EventOutcomeSuccess EventOutcome = "success" + EventOutcomeFailureUnauthorized EventOutcome = "failure_unauthorized" + EventOutcomeFailureNotFound EventOutcome = "failure_not_found" + EventOutcomeFailureGeneric EventOutcome = "failure_generic" +) diff --git a/pkg/apiserver/auditing/event_test.go b/pkg/apiserver/auditing/event_test.go new file mode 100644 index 00000000000..3267936b02a --- /dev/null +++ b/pkg/apiserver/auditing/event_test.go @@ -0,0 +1,64 @@ +package auditing_test + +import ( + "encoding/json" + "strconv" + "strings" + "testing" + "time" + + "github.com/grafana/grafana/pkg/apiserver/auditing" + "github.com/stretchr/testify/require" +) + +func TestEvent_MarshalJSON(t *testing.T) { + t.Parallel() + + t.Run("marshals the event", func(t *testing.T) { + t.Parallel() + + now := time.Now() + + event := auditing.Event{ + ObservedAt: now, + Extra: map[string]string{"k1": "v1", "k2": "v2"}, + } + + data, err := json.Marshal(event) + require.NoError(t, err) + + var result map[string]any + require.NoError(t, json.Unmarshal(data, &result)) + + require.Equal(t, event.Time().UTC().Format(time.RFC3339Nano), result["observedAt"]) + require.NotNil(t, result["extra"]) + require.Len(t, result["extra"], 2) + }) +} + +func TestEvent_KVPairs(t *testing.T) { + t.Parallel() + + t.Run("records extra fields", func(t *testing.T) { + t.Parallel() + + extraFields := 2 + extra := make(map[string]string, 0) + for i := 0; i < extraFields; i++ { + extra[strconv.Itoa(i)] = "value" + } + + event := auditing.Event{Extra: extra} + + kvPairs := event.KVPairs() + + extraCount := 0 + for i := 0; i < len(kvPairs); i += 2 { + if strings.HasPrefix(kvPairs[i].(string), "extra_") { + extraCount++ + } + } + + require.Equal(t, extraCount, extraFields) + }) +} diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index d6f2bcbec2e..22e832034bc 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -688,6 +688,14 @@ var ( HideFromDocs: true, RequiresRestart: true, }, + { + Name: "auditLoggingAppPlatform", + Description: "Enable audit logging with Kubernetes under app platform", + Stage: FeatureStageExperimental, + Owner: grafanaOperatorExperienceSquad, + HideFromDocs: true, + RequiresRestart: true, + }, { Name: "secretsManagementAppPlatform", Description: "Enable the secrets management API and services under app platform", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 179568aa0c4..87001f263f8 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -95,6 +95,7 @@ kubernetesFeatureToggles,experimental,@grafana/grafana-operator-experience-squad cloudRBACRoles,preview,@grafana/identity-access-team,false,true,false alertingQueryOptimization,GA,@grafana/alerting-squad,false,false,false jitterAlertRulesWithinGroups,preview,@grafana/alerting-squad,false,true,false +auditLoggingAppPlatform,experimental,@grafana/grafana-operator-experience-squad,false,true,false secretsManagementAppPlatform,experimental,@grafana/grafana-operator-experience-squad,false,false,false secretsManagementAppPlatformUI,experimental,@grafana/grafana-operator-experience-squad,false,false,false alertingSaveStatePeriodic,privatePreview,@grafana/alerting-squad,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 2797b046d57..6543d31dba5 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -279,6 +279,10 @@ const ( // Distributes alert rule evaluations more evenly over time, including spreading out rules within the same group. Disables sequential evaluation if enabled. FlagJitterAlertRulesWithinGroups = "jitterAlertRulesWithinGroups" + // FlagAuditLoggingAppPlatform + // Enable audit logging with Kubernetes under app platform + FlagAuditLoggingAppPlatform = "auditLoggingAppPlatform" + // FlagSecretsManagementAppPlatform // Enable the secrets management API and services under app platform FlagSecretsManagementAppPlatform = "secretsManagementAppPlatform" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 42922ecf82d..5bea1b2e40f 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -658,6 +658,20 @@ "frontend": true } }, + { + "metadata": { + "name": "auditLoggingAppPlatform", + "resourceVersion": "1767013056996", + "creationTimestamp": "2025-12-29T12:57:36Z" + }, + "spec": { + "description": "Enable audit logging with Kubernetes under app platform", + "stage": "experimental", + "codeowner": "@grafana/grafana-operator-experience-squad", + "requiresRestart": true, + "hideFromDocs": true + } + }, { "metadata": { "name": "authZGRPCServer", diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index eefbb6dea30..e2b60ad6e39 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -161,7 +161,7 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { authz: ruleAuthzService, evaluator: api.EvaluatorFactory, cfg: &api.Cfg.UnifiedAlerting, - backtesting: backtesting.NewEngine(api.AppUrl, api.EvaluatorFactory, api.Tracer), + backtesting: backtesting.NewEngine(api.AppUrl, api.EvaluatorFactory, api.Tracer, api.Cfg.UnifiedAlerting, api.FeatureManager), featureManager: api.FeatureManager, appUrl: api.AppUrl, tracer: api.Tracer, diff --git a/pkg/services/ngalert/api/api_testing.go b/pkg/services/ngalert/api/api_testing.go index 3bda2e3f28f..13bc1a96c24 100644 --- a/pkg/services/ngalert/api/api_testing.go +++ b/pkg/services/ngalert/api/api_testing.go @@ -34,7 +34,6 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/state" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/util" ) type folderService interface { @@ -230,54 +229,27 @@ func (srv TestingApiSrv) BacktestAlertRule(c *contextmodel.ReqContext, cmd apimo return ErrResp(http.StatusNotFound, nil, "Backgtesting API is not enabled") } - if cmd.From.After(cmd.To) { - return ErrResp(400, nil, "From cannot be greater than To") - } - - noDataState, err := ngmodels.NoDataStateFromString(string(cmd.NoDataState)) - + rule, err := apivalidation.ValidateBacktestConfig(c.GetOrgID(), cmd, apivalidation.RuleLimitsFromConfig(srv.cfg, srv.featureManager)) if err != nil { - return ErrResp(400, err, "") - } - forInterval := time.Duration(cmd.For) - if forInterval < 0 { - return ErrResp(400, nil, "Bad For interval") + return ErrResp(http.StatusBadRequest, err, "") } - intervalSeconds, err := apivalidation.ValidateInterval(time.Duration(cmd.Interval), srv.cfg.BaseInterval) - if err != nil { - return ErrResp(400, err, "") - } - - queries := AlertQueriesFromApiAlertQueries(cmd.Data) - if err := srv.authz.AuthorizeDatasourceAccessForRule(c.Req.Context(), c.SignedInUser, &ngmodels.AlertRule{Data: queries}); err != nil { + if err := srv.authz.AuthorizeDatasourceAccessForRule(c.Req.Context(), c.SignedInUser, rule); err != nil { return errorToResponse(err) } - rule := &ngmodels.AlertRule{ - // ID: 0, - // Updated: time.Time{}, - // Version: 0, - // NamespaceUID: "", - // DashboardUID: nil, - // PanelID: nil, - // RuleGroup: "", - // RuleGroupIndex: 0, - // ExecErrState: "", - Title: cmd.Title, - // prefix backtesting- is to distinguish between executions of regular rule and backtesting in logs (like expression engine, evaluator, state manager etc) - UID: "backtesting-" + util.GenerateShortUID(), - OrgID: c.GetOrgID(), - Condition: cmd.Condition, - Data: queries, - IntervalSeconds: intervalSeconds, - NoDataState: noDataState, - For: forInterval, - Annotations: cmd.Annotations, - Labels: cmd.Labels, + // Fetch folder path for alert labels, fallback to "Backtesting" if not available + var folderTitle string + if cmd.NamespaceUID != "" { + f, err := srv.folderService.GetNamespaceByUID(c.Req.Context(), cmd.NamespaceUID, c.OrgID, c.SignedInUser) + if err != nil { + srv.log.FromContext(c.Req.Context()).Warn("Failed to fetch folder path for alert labels", "error", err) + } else { + folderTitle = f.Fullpath + } } - result, err := srv.backtesting.Test(c.Req.Context(), c.SignedInUser, rule, cmd.From, cmd.To) + result, err := srv.backtesting.Test(c.Req.Context(), c.SignedInUser, rule, cmd.From, cmd.To, folderTitle) if err != nil { if errors.Is(err, backtesting.ErrInvalidInputData) { return ErrResp(400, err, "Failed to evaluate") @@ -285,9 +257,5 @@ func (srv TestingApiSrv) BacktestAlertRule(c *contextmodel.ReqContext, cmd apimo return ErrResp(500, err, "Failed to evaluate") } - body, err := data.FrameToJSON(result, data.IncludeAll) - if err != nil { - return ErrResp(500, err, "Failed to convert frame to JSON") - } - return response.JSON(http.StatusOK, body) + return response.JSONStreaming(http.StatusOK, result) } diff --git a/pkg/services/ngalert/api/authorization.go b/pkg/services/ngalert/api/authorization.go index 1c107db22c6..7f8b42bb3a4 100644 --- a/pkg/services/ngalert/api/authorization.go +++ b/pkg/services/ngalert/api/authorization.go @@ -81,9 +81,15 @@ func (api *API) authorize(method, path string) web.Handler { // additional authorization is done in the request handler eval = ac.EvalPermission(ac.ActionAlertingRuleRead) // Grafana Rules Testing Paths - case http.MethodPost + "/api/v1/rule/backtest": + case http.MethodPost + "/api/v1/rule/backtest": // TODO (yuri) this should be protected by dedicated permission // additional authorization is done in the request handler - eval = ac.EvalPermission(ac.ActionAlertingRuleRead) + eval = ac.EvalAll( + ac.EvalPermission(ac.ActionAlertingRuleRead), + ac.EvalAny( + ac.EvalPermission(ac.ActionAlertingRuleUpdate), + ac.EvalPermission(ac.ActionAlertingRuleCreate), + ), + ) case http.MethodPost + "/api/v1/eval": // additional authorization is done in the request handler eval = ac.EvalPermission(ac.ActionAlertingRuleRead) diff --git a/pkg/services/ngalert/api/tooling/definitions/testing.go b/pkg/services/ngalert/api/tooling/definitions/testing.go index 2c228e94758..e094c8515b3 100644 --- a/pkg/services/ngalert/api/tooling/definitions/testing.go +++ b/pkg/services/ngalert/api/tooling/definitions/testing.go @@ -221,15 +221,21 @@ type BacktestConfig struct { To time.Time `json:"to"` Interval model.Duration `json:"interval,omitempty"` - Condition string `json:"condition"` - Data []AlertQuery `json:"data"` - For model.Duration `json:"for,omitempty"` + Condition string `json:"condition"` + Data []AlertQuery `json:"data"` + For *model.Duration `json:"for,omitempty"` + KeepFiringFor *model.Duration `json:"keep_firing_for,omitempty"` - Title string `json:"title"` - Labels map[string]string `json:"labels,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` + Title string `json:"title"` + Labels map[string]string `json:"labels,omitempty"` - NoDataState NoDataState `json:"no_data_state"` + NoDataState NoDataState `json:"no_data_state"` + ExecErrState ExecutionErrorState `json:"exec_err_state"` + MissingSeriesEvalsToResolve *int64 `json:"missing_series_evals_to_resolve,omitempty"` + + UID string `json:"uid,omitempty"` + RuleGroup string `json:"rule_group,omitempty"` + NamespaceUID string `json:"namespace_uid,omitempty"` } // swagger:model diff --git a/pkg/services/ngalert/api/validation/api_ruler_validation.go b/pkg/services/ngalert/api/validation/api_ruler_validation.go index 5a74c58f90e..09e601a4711 100644 --- a/pkg/services/ngalert/api/validation/api_ruler_validation.go +++ b/pkg/services/ngalert/api/validation/api_ruler_validation.go @@ -249,6 +249,21 @@ func ValidateCondition(condition string, queries []apimodels.AlertQuery, canPatc return nil } +func validateGroupInterval(incoming prommodels.Duration, limits RuleLimits) (time.Duration, error) { + interval := time.Duration(incoming) + if interval == 0 { + // if group interval is 0 (undefined) then we automatically fall back to the default interval + interval = limits.DefaultRuleEvaluationInterval + } + + if interval < 0 || int64(interval.Seconds())%int64(limits.BaseInterval.Seconds()) != 0 { + return 0, fmt.Errorf("rule evaluation interval (%d second) should be positive number that is multiple of the base interval of %d seconds", int64(interval.Seconds()), int64(limits.BaseInterval.Seconds())) + } + + // TODO should we validate that interval is >= cfg.MinInterval? Currently, we allow to save but fix the specified interval if it is < cfg.MinInterval + return interval, nil +} + func ValidateInterval(interval, baseInterval time.Duration) (int64, error) { intervalSeconds := int64(interval.Seconds()) @@ -336,18 +351,11 @@ func ValidateRuleGroup( return nil, fmt.Errorf("rule group name is too long. Max length is %d", store.AlertRuleMaxRuleGroupNameLength) } - interval := time.Duration(ruleGroupConfig.Interval) - if interval == 0 { - // if group interval is 0 (undefined) then we automatically fall back to the default interval - interval = limits.DefaultRuleEvaluationInterval + interval, err := validateGroupInterval(ruleGroupConfig.Interval, limits) + if err != nil { + return nil, err } - if interval < 0 || int64(interval.Seconds())%int64(limits.BaseInterval.Seconds()) != 0 { - return nil, fmt.Errorf("rule evaluation interval (%d second) should be positive number that is multiple of the base interval of %d seconds", int64(interval.Seconds()), int64(limits.BaseInterval.Seconds())) - } - - // TODO should we validate that interval is >= cfg.MinInterval? Currently, we allow to save but fix the specified interval if it is < cfg.MinInterval - // If the rule group is reserved for no-group rules, we cannot have multiple rules in it. if isNoGroupRuleGroup && len(ruleGroupConfig.Rules) > 1 { return nil, fmt.Errorf("rule group %s is reserved for no-group rules and cannot be used for rule groups with multiple rules", ruleGroupConfig.Name) @@ -410,3 +418,32 @@ func ValidateNotificationSettings(n *apimodels.AlertRuleNotificationSettings) ([ s, }, nil } + +func ValidateBacktestConfig(orgId int64, config apimodels.BacktestConfig, limits RuleLimits) (*ngmodels.AlertRule, error) { + if config.From.After(config.To) { + return nil, fmt.Errorf("invalid testing range: from %s must be before to %s", config.From, config.To) + } + + interval, err := validateGroupInterval(config.Interval, limits) + if err != nil { + return nil, err + } + + return ValidateRuleNode(&apimodels.PostableExtendedRuleNode{ + ApiRuleNode: &apimodels.ApiRuleNode{ + For: config.For, + KeepFiringFor: config.KeepFiringFor, + Labels: config.Labels, + Annotations: nil, + }, + GrafanaManagedAlert: &apimodels.PostableGrafanaRule{ + Title: config.Title, + Condition: config.Condition, + Data: config.Data, + UID: config.UID, + NoDataState: config.NoDataState, + ExecErrState: config.ExecErrState, + MissingSeriesEvalsToResolve: config.MissingSeriesEvalsToResolve, + }, + }, config.RuleGroup, interval, orgId, config.NamespaceUID, limits) +} diff --git a/pkg/services/ngalert/backtesting/engine.go b/pkg/services/ngalert/backtesting/engine.go index 31c968eb234..b4a534fe134 100644 --- a/pkg/services/ngalert/backtesting/engine.go +++ b/pkg/services/ngalert/backtesting/engine.go @@ -15,10 +15,16 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/schedule" + "github.com/grafana/grafana/pkg/services/ngalert/schedule/ticker" "github.com/grafana/grafana/pkg/services/ngalert/state" + "github.com/grafana/grafana/pkg/services/ngalert/state/historian" + history_model "github.com/grafana/grafana/pkg/services/ngalert/state/historian/model" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" ) var ( @@ -28,7 +34,7 @@ var ( backtestingEvaluatorFactory = newBacktestingEvaluator ) -type callbackFunc = func(evaluationIndex int, now time.Time, results eval.Results) error +type callbackFunc = func(evaluationIndex int, now time.Time, results eval.Results) (bool, error) type backtestingEvaluator interface { Eval(ctx context.Context, from time.Time, interval time.Duration, evaluations int, callback callbackFunc) error @@ -40,11 +46,17 @@ type stateManager interface { } type Engine struct { - evalFactory eval.EvaluatorFactory - createStateManager func() stateManager + evalFactory eval.EvaluatorFactory + createStateManager func() stateManager + disableGrafanaFolder bool + featureToggles featuremgmt.FeatureToggles + minInterval time.Duration + baseInterval time.Duration + jitterStrategy schedule.JitterStrategy + maxEvaluations int } -func NewEngine(appUrl *url.URL, evalFactory eval.EvaluatorFactory, tracer tracing.Tracer) *Engine { +func NewEngine(appUrl *url.URL, evalFactory eval.EvaluatorFactory, tracer tracing.Tracer, cfg setting.UnifiedAlertingSettings, toggles featuremgmt.FeatureToggles) *Engine { return &Engine{ evalFactory: evalFactory, createStateManager: func() stateManager { @@ -60,74 +72,139 @@ func NewEngine(appUrl *url.URL, evalFactory eval.EvaluatorFactory, tracer tracin } return state.NewManager(cfg, state.NewNoopPersister()) }, + disableGrafanaFolder: false, + featureToggles: toggles, + minInterval: cfg.MinInterval, + baseInterval: cfg.BaseInterval, + maxEvaluations: cfg.BacktestingMaxEvaluations, + jitterStrategy: schedule.JitterStrategyFrom(cfg, toggles), } } -func (e *Engine) Test(ctx context.Context, user identity.Requester, rule *models.AlertRule, from, to time.Time) (*data.Frame, error) { - ruleCtx := models.WithRuleKey(ctx, rule.GetKey()) - logger := logger.FromContext(ctx) - +func (e *Engine) Test(ctx context.Context, user identity.Requester, rule *models.AlertRule, from, to time.Time, folderTitle string) (res *data.Frame, err error) { + if rule == nil { + return nil, fmt.Errorf("%w: rule is not defined", ErrInvalidInputData) + } if !from.Before(to) { - return nil, fmt.Errorf("%w: invalid interval of the backtesting [%d,%d]", ErrInvalidInputData, from.Unix(), to.Unix()) + return nil, fmt.Errorf("%w: invalid interval [%d,%d]", ErrInvalidInputData, from.Unix(), to.Unix()) } - if to.Sub(from).Seconds() < float64(rule.IntervalSeconds) { - return nil, fmt.Errorf("%w: interval of the backtesting [%d,%d] is less than evaluation interval [%ds]", ErrInvalidInputData, from.Unix(), to.Unix(), rule.IntervalSeconds) + + ruleCtx := models.WithRuleKey(ctx, rule.GetKey()) + logger := logger.FromContext(ruleCtx).New("backtesting", util.GenerateShortUID()) + + var warns []string + if rule.GetInterval() < e.minInterval { + logger.Warn("Interval adjusted to minimal interval", "originalInterval", rule.GetInterval(), "adjustedInterval", e.minInterval) + rule = rule.Copy() + rule.IntervalSeconds = int64(e.minInterval.Seconds()) + warns = append(warns, fmt.Sprintf("Interval adjusted to minimal interval %ds", rule.IntervalSeconds)) } - length := int(to.Sub(from).Seconds()) / int(rule.IntervalSeconds) - stateManager := e.createStateManager() + effectiveStrategy := e.jitterStrategy + if e.jitterStrategy == schedule.JitterByGroup && (rule.RuleGroup == "" || rule.NamespaceUID == "") || + e.jitterStrategy == schedule.JitterByRule && rule.UID == "" { + logger.Warn(fmt.Sprintf("Jitter strategy is set to %s, but rule group or namespace is not set. Ignore jitter", e.jitterStrategy)) + warns = append(warns, fmt.Sprintf("Jitter strategy is set to %s, but rule group or namespace is not set. Ignore jitter. The results of testing will be different than real evaluations", e.jitterStrategy)) + effectiveStrategy = schedule.JitterNever + } + jitterOffset := schedule.JitterOffsetInDuration(rule, e.baseInterval, effectiveStrategy) + firstEval, err := getFirstEvaluationTime(from, rule, e.baseInterval, jitterOffset) + if err != nil { + return nil, fmt.Errorf("%w: %s", ErrInvalidInputData, err) + } - evaluator, err := backtestingEvaluatorFactory(ruleCtx, e.evalFactory, user, rule.GetEvalCondition().WithSource("backtesting"), &schedule.AlertingResultsFromRuleState{ - Manager: stateManager, - Rule: rule, - }) + evaluations := calculateNumberOfEvaluations(firstEval, to, rule.GetInterval()) + if e.maxEvaluations > 0 && evaluations > e.maxEvaluations { + logger.Warn("Evaluations adjusted to maximal number", "originalEvaluations", evaluations, "adjustedEvaluations", e.maxEvaluations) + warns = append(warns, fmt.Sprintf("Number of evaluations are adjusted to the limit of %d evaluations. Requested: %d", e.maxEvaluations, evaluations)) + evaluations = e.maxEvaluations + } + + start := time.Now() + defer func() { + if err == nil { + logger.Info("Rule testing finished successfully", "duration", time.Since(start)) + } else { + logger.Error("Rule testing finished with error", "duration", time.Since(start), "error", err) + } + }() + + stateMgr := e.createStateManager() + + evaluator, err := backtestingEvaluatorFactory(ruleCtx, + e.evalFactory, + user, + rule.GetEvalCondition().WithSource("backtesting"), + &schedule.AlertingResultsFromRuleState{ + Manager: stateMgr, + Rule: rule, + }, + ) if err != nil { return nil, errors.Join(ErrInvalidInputData, err) } - logger.Info("Start testing alert rule", "from", from, "to", to, "interval", rule.IntervalSeconds, "evaluations", length) + logger.Info("Start testing alert rule", "from", from, "to", to, "interval", rule.GetInterval(), "firstTick", firstEval, "evaluations", evaluations, "jitterOffset", jitterOffset, "jitterStrategy", effectiveStrategy) - start := time.Now() + var builder *historian.QueryResultBuilder - tsField := data.NewField("Time", nil, make([]time.Time, length)) - valueFields := make(map[data.Fingerprint]*data.Field) - - err = evaluator.Eval(ruleCtx, from, time.Duration(rule.IntervalSeconds)*time.Second, length, func(idx int, currentTime time.Time, results eval.Results) error { - if idx >= length { - logger.Info("Unexpected evaluation. Skipping", "from", from, "to", to, "interval", rule.IntervalSeconds, "evaluationTime", currentTime, "evaluationIndex", idx, "expectedEvaluations", length) - return nil - } - states := stateManager.ProcessEvalResults(ruleCtx, currentTime, rule, results, nil, nil) - tsField.Set(idx, currentTime) - for _, s := range states { - field, ok := valueFields[s.CacheID] - if !ok { - field = data.NewField("", s.Labels, make([]*string, length)) - valueFields[s.CacheID] = field - } - if s.State.State != eval.NoData { // set nil if NoData - value := s.State.State.String() - if s.StateReason != "" { - value += " (" + s.StateReason + ")" - } - field.Set(idx, &value) - continue - } - } - return nil - }) - fields := make([]*data.Field, 0, len(valueFields)+1) - fields = append(fields, tsField) - for _, f := range valueFields { - fields = append(fields, f) + ruleMeta := history_model.RuleMeta{ + ID: rule.ID, + OrgID: rule.OrgID, + UID: rule.UID, + Title: rule.Title, + Group: rule.RuleGroup, + NamespaceUID: rule.NamespaceUID, + // DashboardUID: "", + // PanelID: 0, + Condition: rule.Condition, } - result := data.NewFrame("Testing results", fields...) - + labels := map[string]string{ + historian.OrgIDLabel: fmt.Sprint(ruleMeta.OrgID), + historian.GroupLabel: fmt.Sprint(ruleMeta.Group), + historian.FolderUIDLabel: fmt.Sprint(rule.NamespaceUID), + } + labelsBytes, err := json.Marshal(labels) if err != nil { return nil, err } - logger.Info("Rule testing finished successfully", "duration", time.Since(start)) - return result, nil + + // Ensure fallback if empty string is passed + if folderTitle == "" { + folderTitle = "Backtesting" + } + extraLabels := state.GetRuleExtraLabels(logger, rule, folderTitle, !e.disableGrafanaFolder, e.featureToggles) + + processFn := func(idx int, currentTime time.Time, results eval.Results) (bool, error) { + // init the builder. Do the best guess for the size of the result + if builder == nil { + builder = historian.NewQueryResultBuilder(evaluations * len(results)) + for _, warn := range warns { + builder.AddWarn(warn) + } + } + states := stateMgr.ProcessEvalResults(ruleCtx, currentTime, rule, results, extraLabels, nil) + for _, s := range states { + if !historian.ShouldRecord(s) { + continue + } + entry := historian.StateTransitionToLokiEntry(ruleMeta, s) + err := builder.AddRow(currentTime, entry, labelsBytes) + if err != nil { + return false, err + } + } + return idx <= evaluations, nil + } + + err = evaluator.Eval(ruleCtx, firstEval, rule.GetInterval(), evaluations, processFn) + if err != nil { + return nil, err + } + if builder == nil { + return nil, errors.New("no results were produced") + } + return builder.ToFrame(), nil } func newBacktestingEvaluator(ctx context.Context, evalFactory eval.EvaluatorFactory, user identity.Requester, condition models.Condition, reader eval.AlertingResultsReader) (backtestingEvaluator, error) { @@ -173,3 +250,53 @@ type NoopImageService struct{} func (s *NoopImageService) NewImage(_ context.Context, _ *models.AlertRule) (*models.Image, error) { return &models.Image{}, nil } + +func getNextEvaluationTime(currentTime time.Time, rule *models.AlertRule, baseInterval time.Duration, jitterOffset time.Duration) (time.Time, error) { + if rule.IntervalSeconds%int64(baseInterval.Seconds()) != 0 { + return time.Time{}, fmt.Errorf("interval %ds is not divisible by base interval %ds", rule.IntervalSeconds, int64(baseInterval.Seconds())) + } + + freq := rule.IntervalSeconds / int64(baseInterval.Seconds()) + + firstTickNum := currentTime.Unix() / int64(baseInterval.Seconds()) + + jitterOffsetTicks := int64(jitterOffset / baseInterval) + + firstEvalTickNum := firstTickNum + (jitterOffsetTicks-(firstTickNum%freq)+freq)%freq + + return time.Unix(firstEvalTickNum*int64(baseInterval.Seconds()), 0), nil +} + +func getFirstEvaluationTime(from time.Time, rule *models.AlertRule, baseInterval time.Duration, jitterOffset time.Duration) (time.Time, error) { + // Now calculate the time of the tick the same way as in the scheduler + firstTick := ticker.GetStartTick(from, baseInterval) + + // calculate time of the first evaluation that is at or after the first tick + firstEval, err := getNextEvaluationTime(firstTick, rule, baseInterval, jitterOffset) + if err != nil { + return time.Time{}, err + } + + // Ensure firstEval is at or after from + // Calculate how many intervals to skip to get past 'from' + if firstEval.Before(from) { + diff := from.Sub(firstEval) + interval := rule.GetInterval() + // Ceiling division: how many intervals needed to cover the difference + intervalsToAdd := (diff + interval - 1) / interval + firstEval = firstEval.Add(interval * intervalsToAdd) + } + + return firstEval, nil +} + +func calculateNumberOfEvaluations(firstEval, to time.Time, interval time.Duration) int { + var evaluations int + if to.After(firstEval) { + evaluations = int(to.Sub(firstEval).Seconds()) / int(interval.Seconds()) + } + if evaluations == 0 { + evaluations = 1 + } + return evaluations +} diff --git a/pkg/services/ngalert/backtesting/engine_test.go b/pkg/services/ngalert/backtesting/engine_test.go index d2685e71535..33441d73f32 100644 --- a/pkg/services/ngalert/backtesting/engine_test.go +++ b/pkg/services/ngalert/backtesting/engine_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "math/rand" "testing" "time" @@ -14,9 +13,11 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/eval/eval_mocks" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/schedule" "github.com/grafana/grafana/pkg/services/ngalert/state" "github.com/grafana/grafana/pkg/util" ) @@ -158,16 +159,6 @@ func TestNewBacktestingEvaluator(t *testing.T) { } func TestEvaluatorTest(t *testing.T) { - states := []eval.State{eval.Normal, eval.Alerting, eval.Pending} - generateState := func(prefix string) *state.State { - labels := models.GenerateAlertLabels(rand.Intn(5)+1, prefix+"-") - return &state.State{ - CacheID: labels.Fingerprint(), - Labels: labels, - State: states[rand.Intn(len(states))], - } - } - randomResultCallback := func(now time.Time) (eval.Results, error) { return eval.GenerateResults(rand.Intn(5)+1, eval.ResultGen()), nil } @@ -189,84 +180,17 @@ func TestEvaluatorTest(t *testing.T) { createStateManager: func() stateManager { return manager }, + disableGrafanaFolder: false, + featureToggles: featuremgmt.WithFeatures(), + minInterval: 1 * time.Second, + baseInterval: 1 * time.Second, + jitterStrategy: schedule.JitterNever, + maxEvaluations: 10000, } gen := models.RuleGen rule := gen.With(gen.WithInterval(time.Second)).GenerateRef() ruleInterval := time.Duration(rule.IntervalSeconds) * time.Second - t.Run("should return data frame in specific format", func(t *testing.T) { - from := time.Unix(0, 0) - to := from.Add(5 * ruleInterval) - allStates := [...]eval.State{eval.Normal, eval.Alerting, eval.Pending, eval.NoData, eval.Error} - - var states []state.StateTransition - - for _, s := range allStates { - labels := models.GenerateAlertLabels(rand.Intn(5)+1, s.String()+"-") - states = append(states, state.StateTransition{ - State: &state.State{ - CacheID: labels.Fingerprint(), - Labels: labels, - State: s, - StateReason: util.GenerateShortUID(), - }, - }) - } - - manager.stateCallback = func(now time.Time) []state.StateTransition { - return states - } - - frame, err := engine.Test(context.Background(), nil, rule, from, to) - - require.NoError(t, err) - require.Len(t, frame.Fields, len(states)+1) // +1 - timestamp - - t.Run("should contain field Time", func(t *testing.T) { - timestampField, _ := frame.FieldByName("Time") - require.NotNil(t, timestampField, "frame does not contain field 'Time'") - require.Equal(t, data.FieldTypeTime, timestampField.Type()) - }) - - fieldByState := make(map[data.Fingerprint]*data.Field, len(states)) - - t.Run("should contain a field per state", func(t *testing.T) { - for _, s := range states { - var f *data.Field - for _, field := range frame.Fields { - if field.Labels.String() == s.Labels.String() { - f = field - break - } - } - require.NotNilf(t, f, "Cannot find a field by state labels") - fieldByState[s.CacheID] = f - } - }) - - t.Run("should be populated with correct values", func(t *testing.T) { - timestampField, _ := frame.FieldByName("Time") - expectedLength := timestampField.Len() - for _, field := range frame.Fields { - require.Equalf(t, expectedLength, field.Len(), "Field %s should have the size %d", field.Name, expectedLength) - } - for i := 0; i < expectedLength; i++ { - expectedTime := from.Add(time.Duration(int64(i)*rule.IntervalSeconds) * time.Second) - require.Equal(t, expectedTime, timestampField.At(i).(time.Time)) - for _, s := range states { - f := fieldByState[s.CacheID] - if s.State.State == eval.NoData { - require.Nil(t, f.At(i)) - } else { - v := f.At(i).(*string) - require.NotNilf(t, v, "Field [%s] value at index %d should not be nil", s.CacheID, i) - require.Equal(t, fmt.Sprintf("%s (%s)", s.State.State, s.StateReason), *v) - } - } - } - }) - }) - t.Run("should not fail if 'to-from' is not times of interval", func(t *testing.T) { from := time.Unix(0, 0) to := from.Add(5 * ruleInterval) @@ -287,84 +211,26 @@ func TestEvaluatorTest(t *testing.T) { return states } - frame, err := engine.Test(context.Background(), nil, rule, from, to) + frame, err := engine.Test(context.Background(), nil, rule, from, to, "") require.NoError(t, err) expectedLen := frame.Rows() for i := 0; i < 100; i++ { jitter := time.Duration(rand.Int63n(ruleInterval.Milliseconds())) * time.Millisecond - frame, err = engine.Test(context.Background(), nil, rule, from, to.Add(jitter)) + frame, err = engine.Test(context.Background(), nil, rule, from, to.Add(jitter), "") require.NoError(t, err) require.Equalf(t, expectedLen, frame.Rows(), "jitter %v caused result to be different that base-line", jitter) } }) - t.Run("should backfill field with nulls if a new dimension created in the middle", func(t *testing.T) { - from := time.Unix(0, 0) - - state1 := state.StateTransition{ - State: generateState("1"), - } - state2 := state.StateTransition{ - State: generateState("2"), - } - state3 := state.StateTransition{ - State: generateState("3"), - } - stateByTime := map[time.Time][]state.StateTransition{ - from: {state1, state2}, - from.Add(1 * ruleInterval): {state1, state2}, - from.Add(2 * ruleInterval): {state1, state2}, - from.Add(3 * ruleInterval): {state1, state2, state3}, - from.Add(4 * ruleInterval): {state1, state2, state3}, - } - to := from.Add(time.Duration(len(stateByTime)) * ruleInterval) - - manager.stateCallback = func(now time.Time) []state.StateTransition { - return stateByTime[now] - } - - frame, err := engine.Test(context.Background(), nil, rule, from, to) - require.NoError(t, err) - - var field3 *data.Field - for _, field := range frame.Fields { - if field.Labels.String() == state3.Labels.String() { - field3 = field - break - } - } - require.NotNilf(t, field3, "Result for state 3 was not found") - require.Equalf(t, len(stateByTime), field3.Len(), "State3 result has unexpected number of values") - - idx := 0 - for curTime, states := range stateByTime { - value := field3.At(idx).(*string) - if len(states) == 2 { - require.Nilf(t, value, "The result should be nil if state3 was not available for time %v", curTime) - } - } - }) - t.Run("should fail", func(t *testing.T) { manager.stateCallback = func(now time.Time) []state.StateTransition { return nil } - t.Run("when interval is not correct", func(t *testing.T) { from := time.Now() - t.Run("when from=to", func(t *testing.T) { - to := from - _, err := engine.Test(context.Background(), nil, rule, from, to) - require.ErrorIs(t, err, ErrInvalidInputData) - }) t.Run("when from > to", func(t *testing.T) { to := from.Add(-ruleInterval) - _, err := engine.Test(context.Background(), nil, rule, from, to) - require.ErrorIs(t, err, ErrInvalidInputData) - }) - t.Run("when to-from < interval", func(t *testing.T) { - to := from.Add(ruleInterval).Add(-time.Millisecond) - _, err := engine.Test(context.Background(), nil, rule, from, to) + _, err := engine.Test(context.Background(), nil, rule, from, to, "") require.ErrorIs(t, err, ErrInvalidInputData) }) }) @@ -376,7 +242,7 @@ func TestEvaluatorTest(t *testing.T) { } from := time.Now() to := from.Add(ruleInterval) - _, err := engine.Test(context.Background(), nil, rule, from, to) + _, err := engine.Test(context.Background(), nil, rule, from, to, "") require.ErrorIs(t, err, expectedError) }) }) @@ -404,10 +270,188 @@ func (f *fakeBacktestingEvaluator) Eval(_ context.Context, from time.Time, inter if err != nil { return err } - err = callback(idx, now, results) + c, err := callback(idx, now, results) if err != nil { return err } + if !c { + break + } } return nil } + +func TestGetNextEvaluationTime(t *testing.T) { + baseInterval := 10 * time.Second + + testCases := []struct { + name string + ruleInterval int64 + currentTimestamp int64 + jitterOffset time.Duration + expectError bool + expectedNext int64 + }{ + { + name: "interval not divisible by base interval", + ruleInterval: 15, + currentTimestamp: 0, + jitterOffset: 0, + expectError: true, + }, + { + name: "no jitter - from tick 0", + ruleInterval: 20, + currentTimestamp: 0, + jitterOffset: 0, + expectedNext: 0, + }, + { + name: "no jitter - from tick 1", + ruleInterval: 20, + currentTimestamp: 10, + jitterOffset: 0, + expectedNext: 20, + }, + { + name: "no jitter - from tick 2", + ruleInterval: 20, + currentTimestamp: 20, + jitterOffset: 0, + expectedNext: 20, + }, + { + name: "with 20s jitter - from tick 0", + ruleInterval: 60, + currentTimestamp: 0, + jitterOffset: 20 * time.Second, + expectedNext: 20, + }, + { + name: "with 20s jitter - from tick 2", + ruleInterval: 60, + currentTimestamp: 20, + jitterOffset: 20 * time.Second, + expectedNext: 20, + }, + { + name: "with 20s jitter - from tick 3", + ruleInterval: 60, + currentTimestamp: 30, + jitterOffset: 20 * time.Second, + expectedNext: 80, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rule := &models.AlertRule{IntervalSeconds: tc.ruleInterval} + currentTime := time.Unix(tc.currentTimestamp, 0) + result, err := getNextEvaluationTime(currentTime, rule, baseInterval, tc.jitterOffset) + + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), "is not divisible by base interval") + return + } + + require.NoError(t, err) + require.Equal(t, tc.expectedNext, result.Unix()) + }) + } +} + +func TestGetFirstEvaluationTime(t *testing.T) { + baseInterval := 10 * time.Second + + testCases := []struct { + name string + ruleInterval int64 + fromUnix int64 + jitterOffset time.Duration + expectError bool + expectedUnix int64 + }{ + { + name: "interval not divisible by base interval", + ruleInterval: 15, + fromUnix: 0, + jitterOffset: 0, + expectError: true, + }, + { + name: "no jitter - from at tick 0", + ruleInterval: 20, + fromUnix: 0, + jitterOffset: 0, + expectedUnix: 0, + }, + { + name: "no jitter - from at tick 1", + ruleInterval: 20, + fromUnix: 10, + jitterOffset: 0, + expectedUnix: 20, + }, + { + name: "no jitter - from before first tick", + ruleInterval: 20, + fromUnix: 5, + jitterOffset: 0, + expectedUnix: 20, + }, + { + name: "no jitter - from after first aligned tick", + ruleInterval: 20, + fromUnix: 25, + jitterOffset: 0, + expectedUnix: 40, + }, + { + name: "no jitter - from at tick boundary", + ruleInterval: 10, + fromUnix: 10, + jitterOffset: 0, + expectedUnix: 10, + }, + { + name: "with 20s jitter - from epoch", + ruleInterval: 60, + fromUnix: 0, + jitterOffset: 20 * time.Second, + expectedUnix: 20, + }, + { + name: "with 20s jitter - from 70s", + ruleInterval: 60, + fromUnix: 70, + jitterOffset: 20 * time.Second, + expectedUnix: 80, + }, + { + name: "with 50s jitter - from 25s", + ruleInterval: 60, + fromUnix: 25, + jitterOffset: 50 * time.Second, + expectedUnix: 50, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rule := &models.AlertRule{IntervalSeconds: tc.ruleInterval} + from := time.Unix(tc.fromUnix, 0) + result, err := getFirstEvaluationTime(from, rule, baseInterval, tc.jitterOffset) + + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), "is not divisible by base interval") + return + } + + require.NoError(t, err) + require.Equal(t, tc.expectedUnix, result.Unix()) + require.GreaterOrEqual(t, result.Unix(), from.Unix(), "first eval should be at or after from") + }) + } +} diff --git a/pkg/services/ngalert/backtesting/eval_data.go b/pkg/services/ngalert/backtesting/eval_data.go index 999c0bc6302..13827e6e757 100644 --- a/pkg/services/ngalert/backtesting/eval_data.go +++ b/pkg/services/ngalert/backtesting/eval_data.go @@ -85,10 +85,13 @@ func (d *dataEvaluator) Eval(_ context.Context, from time.Time, interval time.Du EvaluatedAt: now, }) } - err := callback(i, now, result) + cont, err := callback(i, now, result) if err != nil { return err } + if !cont { + break + } } return nil } diff --git a/pkg/services/ngalert/backtesting/eval_data_test.go b/pkg/services/ngalert/backtesting/eval_data_test.go index 864229b777c..3d80fa9337a 100644 --- a/pkg/services/ngalert/backtesting/eval_data_test.go +++ b/pkg/services/ngalert/backtesting/eval_data_test.go @@ -100,11 +100,11 @@ func TestDataEvaluator_Eval(t *testing.T) { resultsCount := int(to.Sub(from).Seconds() / interval.Seconds()) - err = evaluator.Eval(context.Background(), from, time.Second, resultsCount, func(idx int, now time.Time, res eval.Results) error { + err = evaluator.Eval(context.Background(), from, time.Second, resultsCount, func(idx int, now time.Time, res eval.Results) (bool, error) { r = append(r, results{ now, res, }) - return nil + return true, nil }) require.NoError(t, err) @@ -164,11 +164,11 @@ func TestDataEvaluator_Eval(t *testing.T) { size := to.Sub(from).Milliseconds() / interval.Milliseconds() r := make([]results, 0, size) - err = evaluator.Eval(context.Background(), from, interval, int(size), func(idx int, now time.Time, res eval.Results) error { + err = evaluator.Eval(context.Background(), from, interval, int(size), func(idx int, now time.Time, res eval.Results) (bool, error) { r = append(r, results{ now, res, }) - return nil + return true, nil }) currentRowIdx := 0 @@ -195,11 +195,11 @@ func TestDataEvaluator_Eval(t *testing.T) { size := int(to.Sub(from).Seconds() / interval.Seconds()) r := make([]results, 0, size) - err = evaluator.Eval(context.Background(), from, interval, size, func(idx int, now time.Time, res eval.Results) error { + err = evaluator.Eval(context.Background(), from, interval, size, func(idx int, now time.Time, res eval.Results) (bool, error) { r = append(r, results{ now, res, }) - return nil + return true, nil }) currentRowIdx := 0 @@ -230,11 +230,11 @@ func TestDataEvaluator_Eval(t *testing.T) { t.Run("should be noData until the frame interval", func(t *testing.T) { newFrom := from.Add(-10 * time.Second) r := make([]results, 0, int(to.Sub(newFrom).Seconds())) - err = evaluator.Eval(context.Background(), newFrom, time.Second, cap(r), func(idx int, now time.Time, res eval.Results) error { + err = evaluator.Eval(context.Background(), newFrom, time.Second, cap(r), func(idx int, now time.Time, res eval.Results) (bool, error) { r = append(r, results{ now, res, }) - return nil + return true, nil }) rowIdx := 0 @@ -258,11 +258,11 @@ func TestDataEvaluator_Eval(t *testing.T) { t.Run("should be the last value after the frame interval", func(t *testing.T) { newTo := to.Add(10 * time.Second) r := make([]results, 0, int(newTo.Sub(from).Seconds())) - err = evaluator.Eval(context.Background(), from, time.Second, cap(r), func(idx int, now time.Time, res eval.Results) error { + err = evaluator.Eval(context.Background(), from, time.Second, cap(r), func(idx int, now time.Time, res eval.Results) (bool, error) { r = append(r, results{ now, res, }) - return nil + return true, nil }) rowIdx := 0 @@ -282,12 +282,21 @@ func TestDataEvaluator_Eval(t *testing.T) { }) t.Run("should stop if callback error", func(t *testing.T) { expectedError := errors.New("error") - err = evaluator.Eval(context.Background(), from, time.Second, 6, func(idx int, now time.Time, res eval.Results) error { + err = evaluator.Eval(context.Background(), from, time.Second, 6, func(idx int, now time.Time, res eval.Results) (bool, error) { if idx == 5 { - return expectedError + return false, expectedError } - return nil + return true, nil }) require.ErrorIs(t, err, expectedError) }) + t.Run("should stop if callback does not want to continue", func(t *testing.T) { + evaluated := 0 + err = evaluator.Eval(context.Background(), from, time.Second, 6, func(idx int, now time.Time, res eval.Results) (bool, error) { + evaluated++ + return evaluated < 2, nil + }) + require.NoError(t, err) + require.Equal(t, 2, evaluated) + }) } diff --git a/pkg/services/ngalert/backtesting/eval_query.go b/pkg/services/ngalert/backtesting/eval_query.go index f53e3de86cb..07720f4f265 100644 --- a/pkg/services/ngalert/backtesting/eval_query.go +++ b/pkg/services/ngalert/backtesting/eval_query.go @@ -18,10 +18,13 @@ func (d *queryEvaluator) Eval(ctx context.Context, from time.Time, interval time if err != nil { return err } - err = callback(idx, now, results) + cont, err := callback(idx, now, results) if err != nil { return err } + if !cont { + break + } } return nil } diff --git a/pkg/services/ngalert/backtesting/eval_query_test.go b/pkg/services/ngalert/backtesting/eval_query_test.go index e88948971f0..4c9df9d25b1 100644 --- a/pkg/services/ngalert/backtesting/eval_query_test.go +++ b/pkg/services/ngalert/backtesting/eval_query_test.go @@ -31,9 +31,9 @@ func TestQueryEvaluator_Eval(t *testing.T) { intervals := make([]time.Time, times) - err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) error { + err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) (bool, error) { intervals[idx] = now - return nil + return true, nil }) require.NoError(t, err) require.Len(t, intervals, times) @@ -49,7 +49,7 @@ func TestQueryEvaluator_Eval(t *testing.T) { } }) - t.Run("should stop evaluation if error", func(t *testing.T) { + t.Run("should stop evaluation", func(t *testing.T) { t.Run("when evaluation fails", func(t *testing.T) { m := &eval_mocks.ConditionEvaluatorMock{} expectedResults := eval.Results{} @@ -62,9 +62,9 @@ func TestQueryEvaluator_Eval(t *testing.T) { intervals := make([]time.Time, 0, times) - err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) error { + err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) (bool, error) { intervals = append(intervals, now) - return nil + return true, nil }) require.ErrorIs(t, err, expectedError) require.Len(t, intervals, 3) @@ -81,14 +81,31 @@ func TestQueryEvaluator_Eval(t *testing.T) { intervals := make([]time.Time, 0, times) - err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) error { + err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) (bool, error) { if len(intervals) > 3 { - return expectedError + return false, expectedError } intervals = append(intervals, now) - return nil + return true, nil }) require.ErrorIs(t, err, expectedError) }) + + t.Run("when callback does not want to continue", func(t *testing.T) { + m := &eval_mocks.ConditionEvaluatorMock{} + expectedResults := eval.Results{} + m.EXPECT().Evaluate(mock.Anything, mock.Anything).Return(expectedResults, nil) + evaluator := queryEvaluator{ + eval: m, + } + + evaluated := 0 + err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) (bool, error) { + evaluated++ + return evaluated <= 2, nil + }) + require.NoError(t, err, nil) + require.Equal(t, 3, evaluated) + }) }) } diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index f7bb3d9fcfd..7d6e8a1fab5 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -480,6 +480,10 @@ func (alertRule *AlertRule) GetPanelID() int64 { return -1 } +func (alertRule *AlertRule) GetInterval() time.Duration { + return time.Duration(alertRule.IntervalSeconds) * time.Second +} + type LabelOption func(map[string]string) func WithoutInternalLabels() LabelOption { diff --git a/pkg/services/ngalert/schedule/jitter.go b/pkg/services/ngalert/schedule/jitter.go index 3d6c839f372..a805ab9b0b3 100644 --- a/pkg/services/ngalert/schedule/jitter.go +++ b/pkg/services/ngalert/schedule/jitter.go @@ -5,6 +5,7 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/services/featuremgmt" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/setting" @@ -13,6 +14,10 @@ import ( // JitterStrategy represents a modifier to alert rule timing that affects how evaluations are distributed. type JitterStrategy int +func (s JitterStrategy) String() string { + return [...]string{"never", "by group", "by rule"}[s] +} + const ( JitterNever JitterStrategy = iota JitterByGroup @@ -57,6 +62,11 @@ func jitterOffsetInTicks(r *ngmodels.AlertRule, baseInterval time.Duration, stra return res } +// JitterOffsetInDuration gives the jitter offset for a rule, in terms of a duration relative to its interval and a base interval. +func JitterOffsetInDuration(r *ngmodels.AlertRule, baseInterval time.Duration, strategy JitterStrategy) time.Duration { + return time.Duration(jitterOffsetInTicks(r, baseInterval, strategy)) * baseInterval +} + func jitterHash(r *ngmodels.AlertRule, strategy JitterStrategy) uint64 { ls := data.Labels{ "name": r.RuleGroup, diff --git a/pkg/services/ngalert/schedule/ticker/ticker.go b/pkg/services/ngalert/schedule/ticker/ticker.go index a52b9c4e559..c24dc13798a 100644 --- a/pkg/services/ngalert/schedule/ticker/ticker.go +++ b/pkg/services/ngalert/schedule/ticker/ticker.go @@ -44,7 +44,11 @@ func New(c clock.Clock, interval time.Duration, metric *Metrics, logger log.Logg } func getStartTick(clk clock.Clock, interval time.Duration) time.Time { - nano := clk.Now().UnixNano() + return GetStartTick(clk.Now(), interval) +} + +func GetStartTick(t time.Time, interval time.Duration) time.Time { + nano := t.UnixNano() return time.Unix(0, nano-(nano%interval.Nanoseconds())) } diff --git a/pkg/services/ngalert/state/historian/core.go b/pkg/services/ngalert/state/historian/core.go index eabb5214a9e..daf415be25a 100644 --- a/pkg/services/ngalert/state/historian/core.go +++ b/pkg/services/ngalert/state/historian/core.go @@ -17,7 +17,7 @@ import ( const StateHistoryWriteTimeout = time.Minute -func shouldRecord(transition state.StateTransition) bool { +func ShouldRecord(transition state.StateTransition) bool { if !transition.Changed() { return false } @@ -35,9 +35,9 @@ func shouldRecord(transition state.StateTransition) bool { } // ShouldRecordAnnotation returns true if an annotation should be created for a given state transition. -// This is stricter than shouldRecord to avoid cluttering panels with state transitions. +// This is stricter than ShouldRecord to avoid cluttering panels with state transitions. func ShouldRecordAnnotation(t state.StateTransition) bool { - if !shouldRecord(t) { + if !ShouldRecord(t) { return false } diff --git a/pkg/services/ngalert/state/historian/core_test.go b/pkg/services/ngalert/state/historian/core_test.go index f798bd26cef..a5f0c55d816 100644 --- a/pkg/services/ngalert/state/historian/core_test.go +++ b/pkg/services/ngalert/state/historian/core_test.go @@ -92,7 +92,7 @@ func TestShouldRecord(t *testing.T) { } t.Run(fmt.Sprintf("%s -> %s should be %v", trans.PreviousFormatted(), trans.Formatted(), !ok), func(t *testing.T) { - require.Equal(t, !ok, shouldRecord(trans)) + require.Equal(t, !ok, ShouldRecord(trans)) }) } } diff --git a/pkg/services/ngalert/state/historian/loki.go b/pkg/services/ngalert/state/historian/loki.go index 76e3e9025bd..8e98d411c1e 100644 --- a/pkg/services/ngalert/state/historian/loki.go +++ b/pkg/services/ngalert/state/historian/loki.go @@ -41,6 +41,69 @@ const ( dfLabels = "labels" ) +// QueryResultBuilder is a builder for a data frame that represents query results from Loki. +// It contains three fields: time (timestamp), line (JSON data), and labels (JSON labels). +type QueryResultBuilder struct { + frame *data.Frame +} + +// NewQueryResultBuilder creates a new QueryResultBuilder with the specified capacity. +// The capacity is used to pre-allocate the underlying slices for better performance. +func NewQueryResultBuilder(capacity int) *QueryResultBuilder { + frame := data.NewFrame("states") + lbls := data.Labels(map[string]string{}) + + // We represent state history as a single merged history, that roughly corresponds to what you get in the Grafana Explore tab when querying Loki directly. + // The format is composed of the following vectors: + // 1. `time` - timestamp - when the transition happened + // 2. `line` - JSON - the full data of the transition + // 3. `labels` - JSON - the labels associated with that state transition + times := make([]time.Time, 0, capacity) + lines := make([]json.RawMessage, 0, capacity) + labels := make([]json.RawMessage, 0, capacity) + + frame.Fields = append(frame.Fields, data.NewField(dfTime, lbls, times)) + frame.Fields = append(frame.Fields, data.NewField(dfLine, lbls, lines)) + frame.Fields = append(frame.Fields, data.NewField(dfLabels, lbls, labels)) + + return &QueryResultBuilder{frame: frame} +} + +func (qr QueryResultBuilder) AddRowRaw(timestamp time.Time, line json.RawMessage, labels json.RawMessage) { + frame := qr.frame + frame.Fields[0].Append(timestamp) + frame.Fields[1].Append(line) + frame.Fields[2].Append(labels) +} + +func (qr QueryResultBuilder) AddRow(timestamp time.Time, line LokiEntry, labels json.RawMessage) error { + lineBytes, err := json.Marshal(line) + if err != nil { + return err + } + qr.AddRowRaw(timestamp, lineBytes, labels) + return nil +} + +// ToFrame converts the QueryResultBuilder back to a data.Frame. +func (qr QueryResultBuilder) ToFrame() *data.Frame { + return qr.frame +} + +func (qr QueryResultBuilder) AddWarn(s string) { + m := qr.frame.Meta + if m == nil { + m = &data.FrameMeta{} + qr.frame.SetMeta(m) + } + m.Notices = append(m.Notices, data.Notice{ + Severity: data.NoticeSeverityWarning, + Text: s, + Link: "", + Inspect: 0, + }) +} + const ( StateHistoryLabelKey = "from" StateHistoryLabelValue = "state-history" @@ -191,20 +254,7 @@ func (h RemoteLokiBackend) merge(res []lokiclient.Stream, folderUIDToFilter []st totalLen += len(arr.Values) } - // Create a new slice to store the merged elements. - frame := data.NewFrame("states") - - // We merge all series into a single linear history. - lbls := data.Labels(map[string]string{}) - - // We represent state history as a single merged history, that roughly corresponds to what you get in the Grafana Explore tab when querying Loki directly. - // The format is composed of the following vectors: - // 1. `time` - timestamp - when the transition happened - // 2. `line` - JSON - the full data of the transition - // 3. `labels` - JSON - the labels associated with that state transition - times := make([]time.Time, 0, totalLen) - lines := make([]json.RawMessage, 0, totalLen) - labels := make([]json.RawMessage, 0, totalLen) + queryResult := NewQueryResultBuilder(totalLen) // Initialize a slice of pointers to the current position in each array. pointers := make([]int, len(res)) @@ -259,17 +309,10 @@ func (h RemoteLokiBackend) merge(res []lokiclient.Stream, folderUIDToFilter []st pointers[minElStreamIdx]++ continue } - times = append(times, time.Unix(0, tsNano)) - labels = append(labels, lblsJson) - lines = append(lines, json.RawMessage(entryBytes)) + queryResult.AddRowRaw(time.Unix(0, tsNano), entryBytes, lblsJson) pointers[minElStreamIdx]++ } - - frame.Fields = append(frame.Fields, data.NewField(dfTime, lbls, times)) - frame.Fields = append(frame.Fields, data.NewField(dfLine, lbls, lines)) - frame.Fields = append(frame.Fields, data.NewField(dfLabels, lbls, labels)) - - return frame, nil + return queryResult.ToFrame(), nil } func StatesToStream(rule history_model.RuleMeta, states []state.StateTransition, externalLabels map[string]string, logger log.Logger) lokiclient.Stream { @@ -282,28 +325,11 @@ func StatesToStream(rule history_model.RuleMeta, states []state.StateTransition, samples := make([]lokiclient.Sample, 0, len(states)) for _, state := range states { - if !shouldRecord(state) { + if !ShouldRecord(state) { continue } - sanitizedLabels := removePrivateLabels(state.Labels) - entry := LokiEntry{ - SchemaVersion: 1, - Previous: state.PreviousFormatted(), - Current: state.Formatted(), - Values: valuesAsDataBlob(state.State), - Condition: rule.Condition, - DashboardUID: rule.DashboardUID, - PanelID: rule.PanelID, - Fingerprint: labelFingerprint(sanitizedLabels), - RuleTitle: rule.Title, - RuleID: rule.ID, - RuleUID: rule.UID, - InstanceLabels: sanitizedLabels, - } - if state.State.State == eval.Error { - entry.Error = state.Error.Error() - } + entry := StateTransitionToLokiEntry(rule, state) jsn, err := json.Marshal(entry) if err != nil { @@ -324,6 +350,28 @@ func StatesToStream(rule history_model.RuleMeta, states []state.StateTransition, } } +func StateTransitionToLokiEntry(rule history_model.RuleMeta, state state.StateTransition) LokiEntry { + sanitizedLabels := removePrivateLabels(state.Labels) + entry := LokiEntry{ + SchemaVersion: 1, + Previous: state.PreviousFormatted(), + Current: state.Formatted(), + Values: valuesAsDataBlob(state.State), + Condition: rule.Condition, + DashboardUID: rule.DashboardUID, + PanelID: rule.PanelID, + Fingerprint: labelFingerprint(sanitizedLabels), + RuleTitle: rule.Title, + RuleID: rule.ID, + RuleUID: rule.UID, + InstanceLabels: sanitizedLabels, + } + if state.State.State == eval.Error && state.Error != nil { + entry.Error = state.Error.Error() + } + return entry +} + func (h *RemoteLokiBackend) recordStreams(ctx context.Context, stream lokiclient.Stream, logger log.Logger) error { if err := h.client.Push(ctx, []lokiclient.Stream{stream}); err != nil { return err diff --git a/pkg/setting/setting_unified_alerting.go b/pkg/setting/setting_unified_alerting.go index 743f386ff52..6abaef8bc2e 100644 --- a/pkg/setting/setting_unified_alerting.go +++ b/pkg/setting/setting_unified_alerting.go @@ -156,6 +156,8 @@ type UnifiedAlertingSettings struct { // AlertmanagerMaxTemplateOutputSize specifies the maximum allowed size for rendered template output in bytes. AlertmanagerMaxTemplateOutputSize int64 + + BacktestingMaxEvaluations int } type RecordingRuleSettings struct { @@ -594,6 +596,11 @@ func (cfg *Cfg) ReadUnifiedAlertingSettings(iniFile *ini.File) error { return fmt.Errorf("setting 'alertmanager_max_template_output_bytes' is invalid, only 0 or a positive integer are allowed") } + uaCfg.BacktestingMaxEvaluations = ua.Key("backtesting_max_evaluations").MustInt(100) + if uaCfg.BacktestingMaxEvaluations < 0 { + uaCfg.BacktestingMaxEvaluations = 100 + } + cfg.UnifiedAlerting = uaCfg return nil } diff --git a/pkg/tests/api/alerting/api_backtesting_test.go b/pkg/tests/api/alerting/api_backtesting_test.go index f07be49ff01..faf50a60da2 100644 --- a/pkg/tests/api/alerting/api_backtesting_test.go +++ b/pkg/tests/api/alerting/api_backtesting_test.go @@ -68,7 +68,7 @@ func TestBacktesting(t *testing.T) { require.Truef(t, ok, "The data file does not contain a field `data`") status, body := apiCli.SubmitRuleForBacktesting(t, request) - require.Equal(t, http.StatusOK, status) + require.Equalf(t, http.StatusOK, status, "Response: %s", body) var result data.Frame require.NoErrorf(t, json.Unmarshal([]byte(body), &result), "cannot parse response to data frame") }) @@ -107,6 +107,7 @@ func TestBacktesting(t *testing.T) { resourcepermissions.SetResourcePermissionCommand{ Actions: []string{ accesscontrol.ActionAlertingRuleRead, + accesscontrol.ActionAlertingRuleUpdate, }, Resource: "folders", ResourceID: "*", diff --git a/pkg/tests/api/alerting/test-data/api_backtesting_data.json b/pkg/tests/api/alerting/test-data/api_backtesting_data.json index d02b6905f0b..5fe0f621126 100644 --- a/pkg/tests/api/alerting/test-data/api_backtesting_data.json +++ b/pkg/tests/api/alerting/test-data/api_backtesting_data.json @@ -12,6 +12,9 @@ }, "condition": "A", "no_data_state": "Alerting", + "title": "test-rule-backtesting-data", + "rule_group": "test-group", + "namespace_uid": "test-namespace", "data": [ { "refId": "A", @@ -193,6 +196,9 @@ }, "condition": "C", "no_data_state": "Alerting", + "title": "test-rule-backtesting-data", + "rule_group": "test-group", + "namespace_uid": "test-namespace", "data": [ { "refId": "A", diff --git a/public/app/core/utils/object.ts b/public/app/core/utils/object.ts index 7ace78598c4..ba1426b163c 100644 --- a/public/app/core/utils/object.ts +++ b/public/app/core/utils/object.ts @@ -1,23 +1,29 @@ -import { isArray, isPlainObject } from 'lodash'; +import { isArray, isPlainObject, isString } from 'lodash'; /** * @returns A deep clone of the object, but with any null value removed. * @param value - The object to be cloned and cleaned. * @param convertInfinity - If true, -Infinity or Infinity is converted to 0. * This is because Infinity is not a valid JSON value, and sometimes we want to convert it to 0 instead of default null. + * @param stripBOMs - If true, strips Byte Order Mark (BOM) characters from all strings. + * BOMs (U+FEFF) can cause CUE validation errors ("illegal byte order mark"). */ -export function sortedDeepCloneWithoutNulls(value: T, convertInfinity?: boolean): T { +export function sortedDeepCloneWithoutNulls(value: T, convertInfinity?: boolean, stripBOMs?: boolean): T { if (isArray(value)) { - return value.map((item) => sortedDeepCloneWithoutNulls(item, convertInfinity)) as unknown as T; + return value.map((item) => sortedDeepCloneWithoutNulls(item, convertInfinity, stripBOMs)) as unknown as T; } if (isPlainObject(value)) { return Object.keys(value as { [key: string]: any }) .sort() .reduce((acc: any, key) => { - const v = (value as any)[key]; + let v = (value as any)[key]; // Remove null values if (v != null) { - acc[key] = sortedDeepCloneWithoutNulls(v, convertInfinity); + // Strip BOMs from strings + if (stripBOMs && isString(v)) { + v = v.replace(/\ufeff/g, ''); + } + acc[key] = sortedDeepCloneWithoutNulls(v, convertInfinity, stripBOMs); } if (convertInfinity && (v === Infinity || v === -Infinity)) { diff --git a/public/app/features/alerting/unified/api/backtestApi.ts b/public/app/features/alerting/unified/api/backtestApi.ts new file mode 100644 index 00000000000..14a0827cb20 --- /dev/null +++ b/public/app/features/alerting/unified/api/backtestApi.ts @@ -0,0 +1,50 @@ +import { DataFrameJSON } from '@grafana/data'; +import { AlertQuery, GrafanaAlertStateDecision, Labels } from 'app/types/unified-alerting-dto'; + +import { alertingApi } from './alertingApi'; + +/** + * Request body for the backtest API matching the BacktestConfig struct in the backend + */ +export interface BacktestRequest { + // Required time range fields + from: string; // ISO 8601 timestamp + to: string; // ISO 8601 timestamp + interval: string; // e.g., "1m", "5m" + + // Required alert definition fields + condition: string; + data: AlertQuery[]; + title: string; + no_data_state?: GrafanaAlertStateDecision; + exec_err_state?: GrafanaAlertStateDecision; + + // Optional duration fields + for?: string; + keep_firing_for?: string; + + // Optional metadata fields + labels?: Labels; + missing_series_evals_to_resolve?: number; + + // Optional rule identification fields + uid?: string; + rule_group?: string; + namespace_uid?: string; +} + +export const BACKTEST_URL = '/api/v1/rule/backtest'; + +export const backtestApi = alertingApi.injectEndpoints({ + endpoints: (build) => ({ + runBacktest: build.mutation({ + query: (requestBody) => ({ + url: BACKTEST_URL, + method: 'POST', + body: requestBody, + }), + }), + }), +}); + +export const { useRunBacktestMutation } = backtestApi; diff --git a/public/app/features/alerting/unified/components/backtesting/BacktestDropdownButton.tsx b/public/app/features/alerting/unified/components/backtesting/BacktestDropdownButton.tsx new file mode 100644 index 00000000000..7d7c4467780 --- /dev/null +++ b/public/app/features/alerting/unified/components/backtesting/BacktestDropdownButton.tsx @@ -0,0 +1,63 @@ +import { useCallback, useState } from 'react'; + +import { TimeRange, rangeUtil } from '@grafana/data'; +import { Trans, t } from '@grafana/i18n'; +import { Button, Drawer, Dropdown, Menu, MenuItem } from '@grafana/ui'; + +import { RuleFormValues } from '../../types/rule-form'; + +import { BacktestPanel } from './BacktestPanel'; + +interface BacktestDropdownButtonProps { + ruleDefinition: RuleFormValues; +} + +export function BacktestDropdownButton({ ruleDefinition }: BacktestDropdownButtonProps) { + const [isBacktestPanelOpen, setIsBacktestPanelOpen] = useState(false); + const [backtestTimeRange, setBacktestTimeRange] = useState(); + + const handleTimeRangeSelect = useCallback((rawFrom: string) => { + const timeRange = rangeUtil.convertRawToRange({ from: rawFrom, to: 'now' }); + setBacktestTimeRange(timeRange); + setIsBacktestPanelOpen(true); + }, []); + + const handleCustomSelect = useCallback(() => { + setBacktestTimeRange(undefined); + setIsBacktestPanelOpen(true); + }, []); + + return ( + <> + + handleTimeRangeSelect('now-15m')} + /> + handleTimeRangeSelect('now-1h')} + /> + + + } + > + + + + {isBacktestPanelOpen && ( + setIsBacktestPanelOpen(false)} + size="md" + > + + + )} + + ); +} diff --git a/public/app/features/alerting/unified/components/backtesting/BacktestPanel.tsx b/public/app/features/alerting/unified/components/backtesting/BacktestPanel.tsx new file mode 100644 index 00000000000..1a3a4ae7706 --- /dev/null +++ b/public/app/features/alerting/unified/components/backtesting/BacktestPanel.tsx @@ -0,0 +1,200 @@ +import { css } from '@emotion/css'; +import { fromPairs, isEmpty, isEqual } from 'lodash'; +import { useCallback, useEffect, useRef, useState } from 'react'; + +import { AlertLabels } from '@grafana/alerting/unstable'; +import { DataFrameJSON, GrafanaTheme2, TimeRange, rangeUtil } from '@grafana/data'; +import { Trans, t } from '@grafana/i18n'; +import { + Alert, + Icon, + LoadingPlaceholder, + RefreshPicker, + Stack, + Text, + TimeRangePicker, + Tooltip, + useStyles2, +} from '@grafana/ui'; + +import { useRunBacktestMutation } from '../../api/backtestApi'; +import { RuleFormValues } from '../../types/rule-form'; +import { combineMatcherStrings } from '../../utils/alertmanager'; +import { messageFromError } from '../../utils/redux'; +import { formValuesToRulerGrafanaRuleDTO } from '../../utils/rule-form'; +import { LogRecordViewerByTimestamp } from '../rules/state-history/LogRecordViewer'; +import { LogTimelineViewer } from '../rules/state-history/LogTimelineViewer'; +import { useFrameSubset } from '../rules/state-history/LokiStateHistory'; +import { useRuleHistoryRecords } from '../rules/state-history/useRuleHistoryRecords'; + +interface BacktestPanelProps { + ruleDefinition: RuleFormValues; + initialTimeRange?: TimeRange; +} + +export function BacktestPanel({ ruleDefinition, initialTimeRange }: BacktestPanelProps) { + const styles = useStyles2(getStyles); + const [timeRange, setTimeRange] = useState( + initialTimeRange || rangeUtil.convertRawToRange({ from: 'now-15m', to: 'now' }) + ); + const [stateHistory, setStateHistory] = useState(); + const [instancesFilter, setInstancesFilter] = useState(''); + const shouldRunInitialBacktest = useRef(!!initialTimeRange); + + const [runBacktest, { isLoading, error: mutationError }] = useRunBacktestMutation(); + + const handleRunBacktest = useCallback(async () => { + // Convert form values to the proper AlertRule format + const alertRule = formValuesToRulerGrafanaRuleDTO(ruleDefinition); + + // Build requestBody matching BacktestConfig struct + const requestBody = { + // Required time range fields + from: timeRange.from.toISOString(), + to: timeRange.to.toISOString(), + interval: ruleDefinition.evaluateEvery, + + // Required alert definition fields + condition: alertRule.grafana_alert.condition, + data: alertRule.grafana_alert.data, + title: alertRule.grafana_alert.title, + no_data_state: alertRule.grafana_alert.no_data_state, + exec_err_state: alertRule.grafana_alert.exec_err_state, + + // Optional duration fields + for: alertRule.for, + keep_firing_for: alertRule.keep_firing_for, + + // Optional metadata fields + labels: alertRule.labels, + missing_series_evals_to_resolve: alertRule.grafana_alert.missing_series_evals_to_resolve, + + // Optional rule identification fields + uid: alertRule.grafana_alert.uid, + rule_group: ruleDefinition.group, + namespace_uid: ruleDefinition.folder?.uid, + }; + + try { + const result = await runBacktest(requestBody).unwrap(); + setStateHistory(result); + } catch (err) { + // Error is handled by RTK Query and available via mutationError + } + }, [ruleDefinition, timeRange, runBacktest]); + + // Update time range when initialTimeRange prop changes + useEffect(() => { + if (initialTimeRange) { + setTimeRange(initialTimeRange); + } + }, [initialTimeRange]); + + // Run backtest once after initial mount when timeRange is synchronized with initialTimeRange + useEffect(() => { + if (shouldRunInitialBacktest.current && initialTimeRange && isEqual(timeRange, initialTimeRange)) { + shouldRunInitialBacktest.current = false; + handleRunBacktest(); + } + }, [initialTimeRange, timeRange, handleRunBacktest]); + + const { dataFrames, historyRecords, commonLabels } = useRuleHistoryRecords(stateHistory, instancesFilter); + + const { frameSubset, frameTimeRange } = useFrameSubset(dataFrames); + + const onLogRecordLabelClick = useCallback( + (label: string) => { + const matcherString = combineMatcherStrings(instancesFilter, label); + setInstancesFilter(matcherString); + }, + [instancesFilter] + ); + + const hasResults = stateHistory !== undefined; + + const notices = stateHistory?.schema?.meta?.notices || []; + const errorMessage = mutationError ? messageFromError(mutationError) : null; + + return ( +
+ + {}} + onMoveBackward={() => {}} + onMoveForward={() => {}} + onZoom={() => {}} + /> + {}} + isLoading={isLoading} + noIntervalPicker={true} + /> + +
+ {isLoading && } + + {errorMessage && ( + {errorMessage} + )} + + {!isLoading && !mutationError && hasResults && notices.length > 0 && ( + + {notices.map((notice, index) => ( + + {notice.text} + + ))} + + )} + + {!isLoading && !mutationError && hasResults && ( +
+ {!isEmpty(commonLabels) && ( + + + + Common labels + + + + + + + + )} + + +
+ )} +
+
+ ); +} +const getStyles = (theme: GrafanaTheme2) => ({ + scrollableContent: css({ + flex: 1, + display: 'flex', + flexDirection: 'column', + paddingTop: theme.spacing(2), + overflow: 'hidden', + }), + resultsContainer: css({ + display: 'flex', + flexDirection: 'column', + gap: theme.spacing(2), + flex: 1, + overflow: 'hidden', + }), +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx index 85e72cca2d8..ed05cd8823c 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx @@ -60,6 +60,7 @@ import { formValuesToRulerRuleDTO, } from '../../../utils/rule-form'; import { fromRulerRule, fromRulerRuleAndRuleGroupIdentifier } from '../../../utils/rule-id'; +import { BacktestDropdownButton } from '../../backtesting/BacktestDropdownButton'; import { GrafanaRuleExporter } from '../../export/GrafanaRuleExporter'; import { AlertRuleNameAndMetric } from '../AlertRuleNameInput'; import AnnotationsStep from '../AnnotationsStep'; @@ -290,6 +291,8 @@ export const AlertRuleForm = ({ existing, prefill, isManualRestore }: Props) => Edit YAML )} + + {config.featureToggles.alertingBacktesting && } diff --git a/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.test.tsx b/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.test.tsx index a90e9dc52a8..cbc5563538f 100644 --- a/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.test.tsx +++ b/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.test.tsx @@ -60,4 +60,76 @@ describe('LogRecordViewerByTimestamp', () => { expect(within(errorRows[1]).getByText(/Error message:/)).toBeInTheDocument(); expect(within(errorRows[1]).getByText(/explicit message/)).toBeInTheDocument(); }); + + describe('Numeric Value Formatting', () => { + it('should format numeric values correctly in AlertInstanceValues', () => { + const records: LogRecord[] = [ + { + timestamp: 1681739580000, + line: { + current: 'Alerting', + previous: 'Pending', + labels: {}, + values: { + cpu_usage: 42.987654321, + memory_mb: 1234567.89, + disk_io: 0.001234, + request_count: 10000, + }, + }, + }, + ]; + + render(); + + expect(screen.getByText(/cpu_usage/)).toBeInTheDocument(); + expect(screen.getByText(/4\.299e\+1/i)).toBeInTheDocument(); + + expect(screen.getByText(/memory_mb/)).toBeInTheDocument(); + expect(screen.getByText(/1\.235e\+6/i)).toBeInTheDocument(); + + expect(screen.getByText(/disk_io/)).toBeInTheDocument(); + expect(screen.getByText(/1\.234e-3/i)).toBeInTheDocument(); + + expect(screen.getByText(/request_count/)).toBeInTheDocument(); + expect(screen.getByText(/10000/)).toBeInTheDocument(); + }); + + it('should format various numeric ranges correctly', () => { + const records: LogRecord[] = [ + { + timestamp: 1681739580000, + line: { + current: 'Alerting', + previous: 'Pending', + labels: {}, + values: { + small: 0.001, + normal: 42.5, + large: 123456, + boundary_low: 0.01, + boundary_high: 10000, + }, + }, + }, + ]; + + render(); + + expect(screen.getByText(/small/)).toBeInTheDocument(); + expect(screen.getByText(/1\.000e-3/i)).toBeInTheDocument(); + + expect(screen.getByText(/normal/)).toBeInTheDocument(); + expect(screen.getByText(/42\.5/)).toBeInTheDocument(); + + expect(screen.getByText(/large/)).toBeInTheDocument(); + expect(screen.getByText(/1\.235e\+5/i)).toBeInTheDocument(); + + expect(screen.getByText(/boundary_low/)).toBeInTheDocument(); + expect(screen.getByText(/0\.01/)).toBeInTheDocument(); + + expect(screen.getByText(/boundary_high/)).toBeInTheDocument(); + expect(screen.getByText(/10000/)).toBeInTheDocument(); + }); + }); }); diff --git a/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.tsx b/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.tsx index c1d90347c74..06fcde4a1ae 100644 --- a/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.tsx +++ b/public/app/features/alerting/unified/components/rules/state-history/LogRecordViewer.tsx @@ -13,6 +13,7 @@ import { AlertStateTag } from '../AlertStateTag'; import { ErrorMessageRow } from './ErrorMessageRow'; import { LogRecord, omitLabels } from './common'; +import { formatNumericValue } from './numberFormatter'; type LogRecordViewerProps = { records: LogRecord[]; @@ -182,7 +183,7 @@ const AlertInstanceValues = memo(({ record }: { record: Record } return ( <> {values.map(([key, value]) => ( - + ))} ); diff --git a/public/app/features/alerting/unified/components/rules/state-history/numberFormatter.test.ts b/public/app/features/alerting/unified/components/rules/state-history/numberFormatter.test.ts new file mode 100644 index 00000000000..77dfe40df5a --- /dev/null +++ b/public/app/features/alerting/unified/components/rules/state-history/numberFormatter.test.ts @@ -0,0 +1,173 @@ +import { formatNumericValue } from './numberFormatter'; + +describe('formatNumericValue', () => { + describe('Zero and special values', () => { + it('should format zero correctly', () => { + expect(formatNumericValue(0)).toBe('0'); + expect(formatNumericValue(-0)).toBe('0'); + }); + + it('should handle NaN', () => { + expect(formatNumericValue(NaN)).toBe('NaN'); + }); + + it('should handle Infinity', () => { + expect(formatNumericValue(Infinity)).toBe('Infinity'); + expect(formatNumericValue(-Infinity)).toBe('-Infinity'); + }); + }); + + describe('Very small numbers (scientific notation)', () => { + it('should use scientific notation for values less than 1e-2', () => { + const result1 = formatNumericValue(1e-3); + expect(result1).toMatch(/^1\.000e-3$/i); + + const result2 = formatNumericValue(0.001); + expect(result2).toMatch(/^1\.000e-3$/i); + + const result3 = formatNumericValue(0.009); + expect(result3).toMatch(/^9\.000e-3$/i); + }); + + it('should use scientific notation for values just below 1e-2', () => { + const result = formatNumericValue(0.00999); + expect(result).toMatch(/^9\.990e-3$/i); + }); + + it('should format the example from requirements correctly', () => { + // 1.4153928131348452 has > 4 decimal places, so should use scientific notation + const result = formatNumericValue(1.4153928131348452); + expect(result).toMatch(/^1\.415e\+0$/i); + }); + + it('should handle negative very small numbers', () => { + const result = formatNumericValue(-1e-3); + expect(result).toMatch(/^-1\.000e-3$/i); + + const result2 = formatNumericValue(-0.001); + expect(result2).toMatch(/^-1\.000e-3$/i); + }); + }); + + describe('Human-readable range (standard notation)', () => { + it('should use standard notation for boundary value 1e-2', () => { + expect(formatNumericValue(0.01)).toBe('0.01'); + }); + + it('should use standard notation for values in readable range', () => { + expect(formatNumericValue(0.1)).toBe('0.1'); + expect(formatNumericValue(1)).toBe('1'); + expect(formatNumericValue(1.234)).toBe('1.234'); + expect(formatNumericValue(42.5)).toBe('42.5'); + }); + + it('should limit to 4 decimal places without rounding integer parts', () => { + expect(formatNumericValue(123.456)).toBe('123.456'); + expect(formatNumericValue(1234.567)).toBe('1234.567'); + expect(formatNumericValue(9999.9)).toBe('9999.9'); + expect(formatNumericValue(9999.1234)).toBe('9999.1234'); + }); + + it('should use scientific notation for numbers with more than 4 decimal places', () => { + // Numbers with > 4 decimals should use scientific notation even in readable range + const result1 = formatNumericValue(123.456789); + expect(result1).toMatch(/^1\.235e\+2$/i); + + const result2 = formatNumericValue(1.23456789); + expect(result2).toMatch(/^1\.235e\+0$/i); + + const result3 = formatNumericValue(42.987654321); + expect(result3).toMatch(/^4\.299e\+1$/i); + }); + + it('should use standard notation for boundary value 1e4', () => { + expect(formatNumericValue(10000)).toBe('10000'); + }); + + it('should handle negative numbers in readable range', () => { + expect(formatNumericValue(-0.1)).toBe('-0.1'); + expect(formatNumericValue(-123.456)).toBe('-123.456'); + expect(formatNumericValue(-9999.9)).toBe('-9999.9'); + }); + + it('should use scientific notation for negative numbers with excessive precision', () => { + const result = formatNumericValue(-42.987654321); + expect(result).toMatch(/^-4\.299e\+1$/i); + }); + }); + + describe('Very large numbers (scientific notation)', () => { + it('should use scientific notation for values greater than 1e4', () => { + const result1 = formatNumericValue(10001); + expect(result1).toMatch(/^1\.000e\+4$/i); + + const result2 = formatNumericValue(123456); + expect(result2).toMatch(/^1\.235e\+5$/i); + }); + + it('should handle negative very large numbers', () => { + const result = formatNumericValue(-1e5); + expect(result).toMatch(/^-1\.000e\+5$/i); + + const result2 = formatNumericValue(-123456); + expect(result2).toMatch(/^-1\.235e\+5$/i); + }); + }); + + describe('Edge cases', () => { + it('should handle numbers exactly at boundaries', () => { + expect(formatNumericValue(0.01)).toBe('0.01'); + + const justBelow = formatNumericValue(0.009999); + expect(justBelow).toMatch(/^9\.999e-3$/i); + + expect(formatNumericValue(10000)).toBe('10000'); + + const justAbove = formatNumericValue(10001); + expect(justAbove).toMatch(/^1\.000e\+4$/i); + }); + + it('should use scientific notation for very precise decimals with > 4 decimal places', () => { + expect(formatNumericValue(1.23456789)).toMatch(/^1\.235e\+0$/i); + expect(formatNumericValue(123.456789)).toMatch(/^1\.235e\+2$/i); + expect(formatNumericValue(0.123456789)).toMatch(/^1\.235e-1$/i); + }); + + it('should use standard notation for numbers with exactly 4 or fewer decimal places', () => { + expect(formatNumericValue(1.2345)).toBe('1.2345'); + expect(formatNumericValue(0.1234)).toBe('0.1234'); + expect(formatNumericValue(123.4567)).toBe('123.4567'); + }); + }); + + describe('countDecimalPlaces edge cases', () => { + it('should handle numbers that toString() would convert to scientific notation', () => { + const result = formatNumericValue(1e-10); + expect(result).toMatch(/^1\.000e-10$/i); + + const result2 = formatNumericValue(1e10); + expect(result2).toMatch(/^1\.000e\+10$/i); + }); + + it('should correctly count decimals for numbers with trailing zeros', () => { + expect(formatNumericValue(1.234)).toBe('1.234'); + expect(formatNumericValue(1.2)).toBe('1.2'); + expect(formatNumericValue(1.0)).toBe('1'); + }); + + it('should handle boundary values correctly', () => { + expect(formatNumericValue(0.01)).toBe('0.01'); + expect(formatNumericValue(10000)).toBe('10000'); + + expect(formatNumericValue(0.01001)).toMatch(/^1\.001e-2$/i); + expect(formatNumericValue(9999.1234)).toBe('9999.1234'); + expect(formatNumericValue(9999.12345)).toMatch(/^9\.999e\+3$/i); + }); + + it('should handle numbers in readable range that have many decimals', () => { + expect(formatNumericValue(1.4153928131348452)).toMatch(/^1\.415e\+0$/i); + expect(formatNumericValue(42.987654321)).toMatch(/^4\.299e\+1$/i); + expect(formatNumericValue(123.456789)).toMatch(/^1\.235e\+2$/i); + }); + }); +}); diff --git a/public/app/features/alerting/unified/components/rules/state-history/numberFormatter.ts b/public/app/features/alerting/unified/components/rules/state-history/numberFormatter.ts new file mode 100644 index 00000000000..8e518c2c932 --- /dev/null +++ b/public/app/features/alerting/unified/components/rules/state-history/numberFormatter.ts @@ -0,0 +1,75 @@ +const SCIENTIFIC_NOTATION_THRESHOLD_SMALL = 1e-2; +const SCIENTIFIC_NOTATION_THRESHOLD_LARGE = 1e4; +const MAX_DECIMAL_PLACES = 4; +const EXPONENTIAL_DECIMALS = 3; // 4 significant digits = 1 digit + 3 decimals + +const readableRangeFormatter = new Intl.NumberFormat(undefined, { + maximumFractionDigits: MAX_DECIMAL_PLACES, + useGrouping: false, +}); + +/** + * Counts the number of decimal places in a number. + * Only processes numbers in readable range (1e-2 to 1e4) to avoid + * toString() scientific notation issues for very large/small numbers. + * + * Uses toFixed(10) to ensure standard notation representation. + * 10 decimal places is sufficient to detect if a number has > 4 decimal places. + */ +function countDecimalPlaces(value: number): number { + if (Number.isInteger(value)) { + return 0; + } + + const absValue = Math.abs(value); + + // Only count decimals for numbers in readable range + if (absValue < SCIENTIFIC_NOTATION_THRESHOLD_SMALL || absValue > SCIENTIFIC_NOTATION_THRESHOLD_LARGE) { + return 0; + } + + const str = value.toFixed(10); + const decimalIndex = str.indexOf('.'); + + if (decimalIndex === -1) { + return 0; + } + + // Count decimal places, removing trailing zeros + const decimalPart = str.substring(decimalIndex + 1).replace(/0+$/, ''); + return decimalPart.length; +} + +/** + * Formats a numeric value for display in alert rule history. + * - For values in human-readable range (1e-2 to 1e4) with ≤ 4 decimal places: shows up to 4 decimal places + * - For very small values (< 1e-2): uses scientific notation with 4 significant digits + * - For very large values (> 1e4): uses scientific notation with 4 significant digits + * - For numbers with > 4 decimal places: uses scientific notation with 4 significant digits + * + * @param value - The number to format + * @returns A formatted string representation of the number + */ +export function formatNumericValue(value: number): string { + if (!Number.isFinite(value)) { + return String(value); + } + + if (value === 0) { + return '0'; + } + + const absValue = Math.abs(value); + + if (absValue < SCIENTIFIC_NOTATION_THRESHOLD_SMALL || absValue > SCIENTIFIC_NOTATION_THRESHOLD_LARGE) { + return value.toExponential(EXPONENTIAL_DECIMALS); + } + + const decimalPlaces = countDecimalPlaces(value); + + if (decimalPlaces > MAX_DECIMAL_PLACES) { + return value.toExponential(EXPONENTIAL_DECIMALS); + } + + return readableRangeFormatter.format(value); +} diff --git a/public/app/features/dashboard-scene/scene/setDashboardPanelContext.test.ts b/public/app/features/dashboard-scene/scene/setDashboardPanelContext.test.ts index d5669497180..cf1968f45f7 100644 --- a/public/app/features/dashboard-scene/scene/setDashboardPanelContext.test.ts +++ b/public/app/features/dashboard-scene/scene/setDashboardPanelContext.test.ts @@ -1,10 +1,10 @@ import { AdHocVariableModel, EventBusSrv, GroupByVariableModel, VariableModel } from '@grafana/data'; import { BackendSrv, config, setBackendSrv } from '@grafana/runtime'; -import { GroupByVariable, sceneGraph } from '@grafana/scenes'; +import { GroupByVariable, sceneGraph, SceneQueryRunner } from '@grafana/scenes'; import { AdHocFilterItem, PanelContext } from '@grafana/ui'; import { transformSaveModelToScene } from '../serialization/transformSaveModelToScene'; -import { findVizPanelByKey } from '../utils/utils'; +import { findVizPanelByKey, getQueryRunnerFor } from '../utils/utils'; import { getAdHocFilterVariableFor, setDashboardPanelContext } from './setDashboardPanelContext'; @@ -159,6 +159,23 @@ describe('setDashboardPanelContext', () => { // Verify existing filter value updated expect(variable.state.filters[1].operator).toBe('!='); }); + + it('Should use existing adhoc filter when panel has no panel-level datasource because queries have all the same datasources (v2 behavior)', () => { + const { scene, context } = buildTestScene({ existingFilterVariable: true, panelDatasourceUndefined: true }); + + const variable = getAdHocFilterVariableFor(scene, { uid: 'my-ds-uid' }); + variable.setState({ filters: [] }); + + context.onAddAdHocFilter!({ key: 'hello', value: 'world', operator: '=' }); + + // Should use the existing adhoc filter variable, not create a new one + expect(variable.state.filters).toEqual([{ key: 'hello', value: 'world', operator: '=' }]); + + // Verify no new adhoc variables were created + const variables = sceneGraph.getVariables(scene); + const adhocVars = variables.state.variables.filter((v) => v.state.type === 'adhoc'); + expect(adhocVars.length).toBe(1); + }); }); describe('getFiltersBasedOnGrouping', () => { @@ -312,6 +329,7 @@ interface SceneOptions { existingFilterVariable?: boolean; existingGroupByVariable?: boolean; groupByDatasourceUid?: string; + panelDatasourceUndefined?: boolean; } function buildTestScene(options: SceneOptions) { @@ -385,6 +403,19 @@ function buildTestScene(options: SceneOptions) { }); const vizPanel = findVizPanelByKey(scene, 'panel-4')!; + + // Simulate v2 dashboard behavior where non-mixed panels don't have panel-level datasource + // but the queries have their own datasources + if (options.panelDatasourceUndefined) { + const queryRunner = getQueryRunnerFor(vizPanel); + if (queryRunner instanceof SceneQueryRunner) { + queryRunner.setState({ + datasource: undefined, + queries: [{ refId: 'A', datasource: { uid: 'my-ds-uid', type: 'prometheus' } }], + }); + } + } + const context: PanelContext = { eventBus: new EventBusSrv(), eventsScope: 'global', diff --git a/public/app/features/dashboard-scene/scene/setDashboardPanelContext.ts b/public/app/features/dashboard-scene/scene/setDashboardPanelContext.ts index c9b3fdf44bd..a256a3305b1 100644 --- a/public/app/features/dashboard-scene/scene/setDashboardPanelContext.ts +++ b/public/app/features/dashboard-scene/scene/setDashboardPanelContext.ts @@ -6,7 +6,12 @@ import { AdHocFilterItem, PanelContext } from '@grafana/ui'; import { annotationServer } from 'app/features/annotations/api'; import { dashboardSceneGraph } from '../utils/dashboardSceneGraph'; -import { getDashboardSceneFor, getPanelIdForVizPanel, getQueryRunnerFor } from '../utils/utils'; +import { + getDashboardSceneFor, + getDatasourceFromQueryRunner, + getPanelIdForVizPanel, + getQueryRunnerFor, +} from '../utils/utils'; import { DashboardScene } from './DashboardScene'; @@ -121,7 +126,7 @@ export function setDashboardPanelContext(vizPanel: VizPanel, context: PanelConte context.eventBus.publish(new AnnotationChangeEvent({ id })); }; - context.onAddAdHocFilter = (newFilter: AdHocFilterItem) => { + context.onAddAdHocFilter = async (newFilter: AdHocFilterItem) => { const dashboard = getDashboardSceneFor(vizPanel); const queryRunner = getQueryRunnerFor(vizPanel); @@ -129,7 +134,19 @@ export function setDashboardPanelContext(vizPanel: VizPanel, context: PanelConte return; } - const filterVar = getAdHocFilterVariableFor(dashboard, queryRunner.state.datasource); + let datasource = getDatasourceFromQueryRunner(queryRunner); + + // If the datasource is type-only (e.g. it's possible that only group is set in V2 schema queries) + // we need to resolve it to a full datasource + if (datasource && !datasource.uid) { + const datasourceToLoad = await getDataSourceSrv().get(datasource); + datasource = { + uid: datasourceToLoad.uid, + type: datasourceToLoad.type, + }; + } + + const filterVar = getAdHocFilterVariableFor(dashboard, datasource); updateAdHocFilterVariable(filterVar, newFilter); }; @@ -141,7 +158,8 @@ export function setDashboardPanelContext(vizPanel: VizPanel, context: PanelConte return []; } - const groupByVar = getGroupByVariableFor(dashboard, queryRunner.state.datasource); + const datasource = getDatasourceFromQueryRunner(queryRunner); + const groupByVar = getGroupByVariableFor(dashboard, datasource); if (!groupByVar) { return []; @@ -158,7 +176,7 @@ export function setDashboardPanelContext(vizPanel: VizPanel, context: PanelConte .filter((item) => item !== undefined); }; - context.onAddAdHocFilters = (items: AdHocFilterItem[]) => { + context.onAddAdHocFilters = async (items: AdHocFilterItem[]) => { const dashboard = getDashboardSceneFor(vizPanel); const queryRunner = getQueryRunnerFor(vizPanel); @@ -166,7 +184,18 @@ export function setDashboardPanelContext(vizPanel: VizPanel, context: PanelConte return; } - const filterVar = getAdHocFilterVariableFor(dashboard, queryRunner.state.datasource); + let datasource = getDatasourceFromQueryRunner(queryRunner); + + // If the datasource is type-only (e.g. it's possible that only group is set in V2 schema queries) + // we need to resolve it to a full datasource + if (datasource && !datasource.uid) { + const datasourceToLoad = await getDataSourceSrv().get(datasource); + datasource = { + uid: datasourceToLoad.uid, + type: datasourceToLoad.type, + }; + } + const filterVar = getAdHocFilterVariableFor(dashboard, datasource); bulkUpdateAdHocFiltersVariable(filterVar, items); }; diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts index 0ef2a5e5f05..90c7f5e2e61 100644 --- a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts +++ b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts @@ -144,7 +144,8 @@ export function transformSceneToSaveModelSchemaV2(scene: DashboardScene, isSnaps try { // validateDashboardSchemaV2 will throw an error if the dashboard is not valid if (validateDashboardSchemaV2(dashboardSchemaV2)) { - return sortedDeepCloneWithoutNulls(dashboardSchemaV2, true); + // Strip BOMs from all strings to prevent CUE validation errors ("illegal byte order mark") + return sortedDeepCloneWithoutNulls(dashboardSchemaV2, true, true); } // should never reach this point, validation should throw an error throw new Error('Error we could transform the dashboard to schema v2: ' + dashboardSchemaV2); diff --git a/public/app/features/dashboard-scene/utils/drilldownUtils.ts b/public/app/features/dashboard-scene/utils/drilldownUtils.ts index 2ff0ecf7c6e..cf6f1271162 100644 --- a/public/app/features/dashboard-scene/utils/drilldownUtils.ts +++ b/public/app/features/dashboard-scene/utils/drilldownUtils.ts @@ -3,6 +3,8 @@ import { getDataSourceSrv } from '@grafana/runtime'; import { AdHocFiltersVariable, GroupByVariable, sceneGraph, SceneObject, SceneQueryRunner } from '@grafana/scenes'; import { DataSourceRef } from '@grafana/schema'; +import { getDatasourceFromQueryRunner } from './utils'; + export function verifyDrilldownApplicability( sourceObject: SceneObject, queriesDataSource: DataSourceRef | undefined, @@ -26,7 +28,7 @@ export async function getDrilldownApplicability( return; } - const datasource = queryRunner.state.datasource; + const datasource = getDatasourceFromQueryRunner(queryRunner); const queries = queryRunner.state.data?.request?.targets; const ds = await getDataSourceSrv().get(datasource?.uid); diff --git a/public/app/features/dashboard-scene/utils/urlBuilders.ts b/public/app/features/dashboard-scene/utils/urlBuilders.ts index 942d378a38f..11f604d6990 100644 --- a/public/app/features/dashboard-scene/utils/urlBuilders.ts +++ b/public/app/features/dashboard-scene/utils/urlBuilders.ts @@ -4,7 +4,7 @@ import { sceneGraph, VizPanel } from '@grafana/scenes'; import { contextSrv } from 'app/core/services/context_srv'; import { getExploreUrl } from 'app/core/utils/explore'; -import { getQueryRunnerFor } from './utils'; +import { getDatasourceFromQueryRunner, getQueryRunnerFor } from './utils'; export function getViewPanelUrl(vizPanel: VizPanel) { return locationUtil.getUrlForPartial(locationService.getLocation(), { @@ -27,10 +27,11 @@ export function tryGetExploreUrlForPanel(vizPanel: VizPanel): Promise