From 872fbf02085373553c526b6c75267b5239eebe77 Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Fri, 7 Mar 2025 10:23:23 +0100 Subject: [PATCH 01/39] Alerting: Implement arbitrary data source remote writer. (#101735) * Alerting: Implement arbitrary data source remote writer. Recording rules currently remote write to the endpoint speficied in the configuration file, but we want to change this so that it can be specified on a per-rule basis. This change implements the updated RemoteWriter interface, and dispatches writes to the correct data source. Notes about the implementation: - There is a defaulting behaviour, because we will need to handle situations when upgrading from older Grafana versions, where recording rules do not a data source target specified. - In order to avoid re-creating the remote write clients for every request, a cache is used. The entries have a TTL of 30 seconds at which point they are recreated; this is the delay for picking up new data source settings. Known limitations: - There is no per data source configuration for the remote write path suffix. - Not all data source settings will be used, but basic auth and TLS will be. - There is no RBAC. This will be a follow up feature. * Lint --- .../ngalert/writer/datasourcewriter.go | 170 ++++++++++++++++++ .../ngalert/writer/datasourcewriter_test.go | 119 ++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 pkg/services/ngalert/writer/datasourcewriter.go create mode 100644 pkg/services/ngalert/writer/datasourcewriter_test.go diff --git a/pkg/services/ngalert/writer/datasourcewriter.go b/pkg/services/ngalert/writer/datasourcewriter.go new file mode 100644 index 00000000000..a69b157c80b --- /dev/null +++ b/pkg/services/ngalert/writer/datasourcewriter.go @@ -0,0 +1,170 @@ +package writer + +import ( + "context" + "errors" + "fmt" + "net/url" + "time" + + "github.com/benbjohnson/clock" + "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/data" + gocache "github.com/patrickmn/go-cache" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/ngalert/metrics" + "github.com/grafana/grafana/pkg/services/pluginsintegration/adapters" +) + +const ( + // We do not need to aggresively refresh data source settings, because a + // typical recording rule only writes every 60 seconds. + cacheExpiration = 30 * time.Second + + // Time between cleaning expired data sources. + cacheCleanupInterval = 10 * time.Minute +) + +type DatasourceWriterConfig struct { + // Timeout is the maximum time to wait for a remote write to succeed. + Timeout time.Duration + + // DetaultDatasourceUID is the data source used if no data source is specified. + // This exists to cater for upgrading from old versions of Grafana, where rule + // definitions may not have a target data source specified. + DefaultDatasourceUID string + + // RemoteWritePathSuffix is the path suffix for remote write, normally /push. + RemoteWritePathSuffix string +} + +type DatasourceWriter struct { + cfg DatasourceWriterConfig + datasources datasources.DataSourceService + httpClientProvider HttpClientProvider + clock clock.Clock + l log.Logger + metrics *metrics.RemoteWriter + + writers *gocache.Cache +} + +func NewDatasourceWriter( + cfg DatasourceWriterConfig, + datasources datasources.DataSourceService, + httpClientProvider HttpClientProvider, + clock clock.Clock, + l log.Logger, + metrics *metrics.RemoteWriter, +) *DatasourceWriter { + return &DatasourceWriter{ + cfg: cfg, + datasources: datasources, + httpClientProvider: httpClientProvider, + clock: clock, + l: l, + metrics: metrics, + writers: gocache.New(cacheExpiration, cacheCleanupInterval), + } +} + +func (w *DatasourceWriter) decrypt(ds *datasources.DataSource) (map[string]string, error) { + decryptedJsonData, err := w.datasources.DecryptedValues(context.Background(), ds) + if err != nil { + w.l.Error("Failed to decrypt secure json data", "error", err) + } + return decryptedJsonData, err +} + +func (w *DatasourceWriter) makeWriter(ctx context.Context, orgID int64, dsUID string) (*PrometheusWriter, error) { + ds, err := w.datasources.GetDataSource(ctx, &datasources.GetDataSourceQuery{ + UID: dsUID, + OrgID: orgID, + }) + if err != nil { + return nil, err + } + + if ds.Type != datasources.DS_PROMETHEUS { + return nil, errors.New("can only write to data sources of type prometheus") + } + + is, err := adapters.ModelToInstanceSettings(ds, w.decrypt) + if err != nil { + return nil, err + } + + ho, err := is.HTTPClientOptions(ctx) + if err != nil { + return nil, err + } + + u, err := url.Parse(is.URL) + if err != nil { + return nil, err + } + + u = u.JoinPath(w.cfg.RemoteWritePathSuffix) + + cfg := PrometheusWriterConfig{ + URL: u.String(), + HTTPOptions: httpclient.Options{ + Timeouts: ho.Timeouts, + TLS: ho.TLS, + BasicAuth: ho.BasicAuth, + }, + Timeout: w.cfg.Timeout, + } + if err != nil { + return nil, err + } + + return NewPrometheusWriter( + cfg, + w.httpClientProvider, + w.clock, + w.l, + w.metrics) +} + +func uidKey(orgID int64, uid string) string { + return fmt.Sprintf("%d-%s", orgID, uid) +} + +func (w *DatasourceWriter) WriteDatasource(ctx context.Context, dsUID string, name string, t time.Time, frames data.Frames, orgID int64, extraLabels map[string]string) error { + if dsUID == "" { + if w.cfg.DefaultDatasourceUID == "" { + return errors.New("data source uid not specified and no default set") + } + dsUID = w.cfg.DefaultDatasourceUID + w.l.Debug("Using default data source for remote write", + "org_id", orgID, "datasource_uid", dsUID) + } + + key := uidKey(orgID, dsUID) + + var writer *PrometheusWriter + + val, ok := w.writers.Get(key) + if ok { + var ok bool + writer, ok = val.(*PrometheusWriter) + if !ok { + return errors.New("type in cache not a Writer") + } + } else { + var err error + writer, err = w.makeWriter(ctx, orgID, dsUID) + if err != nil { + w.l.Error("Failed to create writer for data source", + "org_id", orgID, "datasource_uid", dsUID) + return err + } + + w.writers.Set(key, writer, 0) + } + + return writer.Write(ctx, name, t, frames, orgID, extraLabels) +} diff --git a/pkg/services/ngalert/writer/datasourcewriter_test.go b/pkg/services/ngalert/writer/datasourcewriter_test.go new file mode 100644 index 00000000000..6727d352997 --- /dev/null +++ b/pkg/services/ngalert/writer/datasourcewriter_test.go @@ -0,0 +1,119 @@ +package writer + +import ( + "context" + "testing" + "time" + + "github.com/benbjohnson/clock" + "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/infra/httpclient" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/datasources" + dsfakes "github.com/grafana/grafana/pkg/services/datasources/fakes" + "github.com/grafana/grafana/pkg/services/ngalert/metrics" +) + +type testDataSources struct { + dsfakes.FakeDataSourceService + + prom1, prom2 *TestRemoteWriteTarget +} + +func (t *testDataSources) Reset() { + t.prom1.Reset() + t.prom2.Reset() +} + +func setupDataSources(t *testing.T) *testDataSources { + res := &testDataSources{ + prom1: NewTestRemoteWriteTarget(t), + prom2: NewTestRemoteWriteTarget(t), + } + + t.Cleanup(func() { + res.prom1.Close() + }) + t.Cleanup(func() { + res.prom2.Close() + }) + + p1, _ := res.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + UID: "prom-1", + Type: datasources.DS_PROMETHEUS, + }) + p1.URL = res.prom1.srv.URL + "/api/v1" + + p2, _ := res.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + UID: "prom-2", + Type: datasources.DS_PROMETHEUS, + }) + p2.URL = res.prom2.srv.URL + "/api/v1" + + // Add a non-Prometheus datasource. + _, _ = res.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + UID: "loki-1", + Type: datasources.DS_LOKI, + }) + + return res +} + +func TestDatasourceWriter(t *testing.T) { + series := []map[string]string{{"foo": "1"}, {"foo": "2"}, {"foo": "3"}, {"foo": "4"}} + frames := frameGenFromLabels(t, data.FrameTypeNumericWide, series) + + datasources := setupDataSources(t) + + cfg := DatasourceWriterConfig{ + Timeout: time.Second * 5, + DefaultDatasourceUID: "prom-2", + RemoteWritePathSuffix: "/write", + } + + met := metrics.NewRemoteWriterMetrics(prometheus.NewRegistry()) + writer := NewDatasourceWriter(cfg, datasources, httpclient.NewProvider(), clock.New(), log.New("test"), met) + + t.Run("when writing a prometheus datasource then the request is made to the expected endpoint", func(t *testing.T) { + datasources.Reset() + + err := writer.WriteDatasource(context.Background(), "prom-1", "metric", time.Now(), frames, 1, map[string]string{}) + require.NoError(t, err) + + assert.Equal(t, 1, datasources.prom1.RequestsCount) + assert.Equal(t, 0, datasources.prom2.RequestsCount) + + err = writer.WriteDatasource(context.Background(), "prom-2", "metric", time.Now(), frames, 1, map[string]string{}) + require.NoError(t, err) + + assert.Equal(t, 1, datasources.prom1.RequestsCount) + assert.Equal(t, 1, datasources.prom2.RequestsCount) + }) + + t.Run("when writing an unknown datasource then an error is returned", func(t *testing.T) { + datasources.Reset() + + err := writer.WriteDatasource(context.Background(), "prom-3", "metric", time.Now(), frames, 1, map[string]string{}) + require.Error(t, err) + require.EqualError(t, err, "data source not found") + }) + + t.Run("when writing a non-prometheus datasource then an error is returned", func(t *testing.T) { + datasources.Reset() + + err := writer.WriteDatasource(context.Background(), "loki-1", "metric", time.Now(), frames, 1, map[string]string{}) + require.Error(t, err) + require.EqualError(t, err, "can only write to data sources of type prometheus") + }) + + t.Run("when writing with an empty datasource uid then the default is written", func(t *testing.T) { + datasources.Reset() + + err := writer.WriteDatasource(context.Background(), "", "metric", time.Now(), frames, 1, map[string]string{}) + require.NoError(t, err) + }) +} From 0d32ba4d432f1f943da50cbf3f96536525ca9bef Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 7 Mar 2025 11:38:20 +0200 Subject: [PATCH 02/39] Update scenes to v6.2.1 (#101722) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package.json | 4 ++-- yarn.lock | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index bde0ffe3dac..77b946e8711 100644 --- a/package.json +++ b/package.json @@ -278,8 +278,8 @@ "@grafana/prometheus": "workspace:*", "@grafana/runtime": "workspace:*", "@grafana/saga-icons": "workspace:*", - "@grafana/scenes": "6.1.4", - "@grafana/scenes-react": "6.1.4", + "@grafana/scenes": "6.2.1", + "@grafana/scenes-react": "6.2.1", "@grafana/schema": "workspace:*", "@grafana/sql": "workspace:*", "@grafana/ui": "workspace:*", diff --git a/yarn.lock b/yarn.lock index b1b682f44ec..369c4f29a57 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3597,11 +3597,11 @@ __metadata: languageName: unknown linkType: soft -"@grafana/scenes-react@npm:6.1.4": - version: 6.1.4 - resolution: "@grafana/scenes-react@npm:6.1.4" +"@grafana/scenes-react@npm:6.2.1": + version: 6.2.1 + resolution: "@grafana/scenes-react@npm:6.2.1" dependencies: - "@grafana/scenes": "npm:6.1.4" + "@grafana/scenes": "npm:6.2.1" lru-cache: "npm:^10.2.2" react-use: "npm:^17.4.0" peerDependencies: @@ -3613,13 +3613,13 @@ __metadata: react: ^18.0.0 react-dom: ^18.0.0 react-router-dom: ^6.28.0 - checksum: 10/69a344f30937a80e25201c8ce1261f85c10663e68d70103c28603abbee2496c7855d999b2a212132f48af597f3f528de65bade66c05b02c719ca25f8f8c682bd + checksum: 10/b8f44f087999fd6074233090a005de03e9d0ed1ba965a05659773cf48afb3c11ed508371fcf0c4cdc3c9a360b3f33e5126ed9082e2f4ee306459d768bd0a5a34 languageName: node linkType: hard -"@grafana/scenes@npm:6.1.4": - version: 6.1.4 - resolution: "@grafana/scenes@npm:6.1.4" +"@grafana/scenes@npm:6.2.1": + version: 6.2.1 + resolution: "@grafana/scenes@npm:6.2.1" dependencies: "@floating-ui/react": "npm:^0.26.16" "@leeoniya/ufuzzy": "npm:^1.0.16" @@ -3637,7 +3637,7 @@ __metadata: react: ^18.0.0 react-dom: ^18.0.0 react-router-dom: ^6.28.0 - checksum: 10/708652236c3b4a5bb0e1cd84739bef530b06244e2798e754347eabd8ab040b24c7f39376fd7b93d1049da6c0ee40785294882cba0125ce8171d5627a71ae63db + checksum: 10/44e9a0386dd09a1a7a45bcdcbf285a00996e885cf56640179cbc35deb1f26af37ce34744321193f98aa078a7e02fc8a06b8f07c03b392229cc42ed56733bab05 languageName: node linkType: hard @@ -18071,8 +18071,8 @@ __metadata: "@grafana/prometheus": "workspace:*" "@grafana/runtime": "workspace:*" "@grafana/saga-icons": "workspace:*" - "@grafana/scenes": "npm:6.1.4" - "@grafana/scenes-react": "npm:6.1.4" + "@grafana/scenes": "npm:6.2.1" + "@grafana/scenes-react": "npm:6.2.1" "@grafana/schema": "workspace:*" "@grafana/sql": "workspace:*" "@grafana/tsconfig": "npm:^2.0.0" From cf95a42507cb5b3fc056126c97df59aa11eb4c5f Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Fri, 7 Mar 2025 10:40:40 +0100 Subject: [PATCH 03/39] ScopesContext: Move export from unstable to regular entrypoint (#101765) Revert unstable export --- .../src/{unstable => services}/ScopesContext.ts | 0 packages/grafana-runtime/src/services/index.ts | 1 + packages/grafana-runtime/src/unstable.ts | 8 ++------ 3 files changed, 3 insertions(+), 6 deletions(-) rename packages/grafana-runtime/src/{unstable => services}/ScopesContext.ts (100%) diff --git a/packages/grafana-runtime/src/unstable/ScopesContext.ts b/packages/grafana-runtime/src/services/ScopesContext.ts similarity index 100% rename from packages/grafana-runtime/src/unstable/ScopesContext.ts rename to packages/grafana-runtime/src/services/ScopesContext.ts diff --git a/packages/grafana-runtime/src/services/index.ts b/packages/grafana-runtime/src/services/index.ts index 496246bd6c4..3ceef02923a 100644 --- a/packages/grafana-runtime/src/services/index.ts +++ b/packages/grafana-runtime/src/services/index.ts @@ -56,3 +56,4 @@ export { export { isPluginExtensionLink, isPluginExtensionComponent } from './pluginExtensions/utils'; export { setCurrentUser } from './user'; export { RuntimeDataSource } from './RuntimeDataSource'; +export { ScopesContext, type ScopesContextValueState, type ScopesContextValue, useScopes } from './ScopesContext'; diff --git a/packages/grafana-runtime/src/unstable.ts b/packages/grafana-runtime/src/unstable.ts index 472372a9a8e..5cf9253a7c4 100644 --- a/packages/grafana-runtime/src/unstable.ts +++ b/packages/grafana-runtime/src/unstable.ts @@ -9,9 +9,5 @@ * and be subject to the standard policies */ -export { - ScopesContext, - type ScopesContextValueState, - type ScopesContextValue, - useScopes, -} from './unstable/ScopesContext'; +// Dummy export to make it a valid module. Remove when we have real exports. +export const unstable = {}; From 5ef3400115dc7494df2773b74693d79811ba4e78 Mon Sep 17 00:00:00 2001 From: Mariell Hoversholm Date: Fri, 7 Mar 2025 11:07:56 +0100 Subject: [PATCH 04/39] CI: Report to bench (#101686) * feat: report to bench * fix: output json * chore: own the code * temp: limit files to test for faster iteration * temp: force bench to run for testing purposes * fix: bench has been merged to main * fix: correct file name * temp: remove temp * feat: pin the tparse binary Co-authored-by: Matheus Macabu --------- Co-authored-by: Matheus Macabu --- .github/CODEOWNERS | 1 + .../actions/setup-grafana-bench/action.yml | 44 +++++++++++++++++++ .github/workflows/pr-backend-unit-tests.yml | 19 +++++++- Makefile | 5 ++- 4 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 .github/actions/setup-grafana-bench/action.yml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 1d20761dd76..65f706f256e 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -756,6 +756,7 @@ embed.go @grafana/grafana-as-code /.github/pr-commands.json @tolzhabayev /.github/renovate.json5 @grafana/frontend-ops /.github/actions/test-coverage-processor/action.yml @grafana/grafana-backend-group +/.github/actions/setup-grafana-bench/ @Proximyst /.github/workflows/add-to-whats-new.yml @grafana/docs-tooling /.github/workflows/auto-triager/ @grafana/plugins-platform-frontend /.github/workflows/alerting-swagger-gen.yml @grafana/alerting-backend diff --git a/.github/actions/setup-grafana-bench/action.yml b/.github/actions/setup-grafana-bench/action.yml new file mode 100644 index 00000000000..624ad243b7b --- /dev/null +++ b/.github/actions/setup-grafana-bench/action.yml @@ -0,0 +1,44 @@ +name: 'Setup Grafana Bench' +description: 'Sets up and installs Grafana Bench' + +inputs: + github-app-name: + description: 'Name of the GitHub App in Vault' + required: false + default: 'grafana-ci-bot' + branch: + description: 'The branch to install from' + required: false + default: 'main' + +runs: + using: "composite" + steps: + - name: Retrieve GitHub App secrets + id: get-secrets + uses: grafana/shared-workflows/actions/get-vault-secrets@get-vault-secrets-v1.0.1 + with: + repo_secrets: | + APP_ID=${{ inputs.github-app-name }}:app-id + APP_INSTALLATION_ID=${{ inputs.github-app-name }}:app-installation-id + PRIVATE_KEY=${{ inputs.github-app-name }}:private-key + + - name: Generate GitHub App token + id: generate_token + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ env.APP_ID }} + private-key: ${{ env.PRIVATE_KEY }} + repositories: "grafana-bench" + owner: "grafana" + + - name: Setup Bench + shell: bash + env: + GH_TOKEN: ${{ steps.generate_token.outputs.token }} + run: | + git clone https://x-access-token:${GH_TOKEN}@github.com/grafana/grafana-bench.git ../grafana-bench + + cd ../grafana-bench + git switch ${{ inputs.branch }} + go install . diff --git a/.github/workflows/pr-backend-unit-tests.yml b/.github/workflows/pr-backend-unit-tests.yml index 4b264547957..1bbb0d7aa7a 100644 --- a/.github/workflows/pr-backend-unit-tests.yml +++ b/.github/workflows/pr-backend-unit-tests.yml @@ -39,6 +39,7 @@ jobs: run: | sudo apt-get update sudo apt-get install -y build-essential shared-mime-info + go install github.com/mfridman/tparse@c1754a1f484ac5cd422697b0fec635177ddc8507 # v0.17.0 - name: Verify code generation run: | @@ -49,7 +50,7 @@ jobs: run: make gen-go - name: Run unit tests - run: COVER_OPTS="-coverprofile=be-unit.cov -coverpkg=github.com/grafana/grafana/..." make test-go-unit + run: COVER_OPTS="-coverprofile=be-unit.cov -coverpkg=github.com/grafana/grafana/..." GO_TEST_OUTPUT="/tmp/unit.log" make test-go-unit - name: Process and upload coverage uses: ./.github/actions/test-coverage-processor @@ -61,6 +62,22 @@ jobs: codecov-flag: 'be-unit' codecov-name: 'be-unit' + - name: Install Grafana Bench + # We can't allow forks here, as we need secret access. + if: ${{ github.event_name != 'pull_request' }} + uses: ./.github/actions/setup-grafana-bench + + - name: Process output for Bench + if: ${{ github.event_name != 'pull_request' }} + run: | + grafana-bench report \ + --trigger pr-backend-unit-tests-oss \ + --report-input go \ + --report-output log \ + --grafana-version "$(git rev-parse HEAD)" \ + --suite-name grafana-oss-unit-tests \ + /tmp/unit.log + concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true diff --git a/Makefile b/Makefile index f2f37e34812..842ce5b65e9 100644 --- a/Makefile +++ b/Makefile @@ -17,6 +17,7 @@ GO_RACE_FLAG := $(if $(GO_RACE),-race) GO_BUILD_FLAGS += $(if $(GO_BUILD_DEV),-dev) GO_BUILD_FLAGS += $(if $(GO_BUILD_TAGS),-build-tags=$(GO_BUILD_TAGS)) GO_BUILD_FLAGS += $(GO_RACE_FLAG) +GO_TEST_OUTPUT := $(shell [ -n "$(GO_TEST_OUTPUT)" ] && echo '-json | tee $(GO_TEST_OUTPUT) | tparse -all') GIT_BASE = remotes/origin/main # GNU xargs has flag -r, and BSD xargs (e.g. MacOS) has that behaviour by default @@ -248,7 +249,7 @@ test-go: test-go-unit test-go-integration .PHONY: test-go-unit test-go-unit: ## Run unit tests for backend with flags. @echo "test backend unit tests" - $(GO) test $(GO_RACE_FLAG) -short -covermode=atomic -coverprofile=unit.cov -timeout=30m $(GO_TEST_FILES) + $(GO) test $(GO_RACE_FLAG) -short -covermode=atomic -coverprofile=unit.cov -timeout=30m $(GO_TEST_FILES) $(GO_TEST_OUTPUT) .PHONY: test-go-unit-pretty test-go-unit-pretty: check-tparse @@ -261,7 +262,7 @@ test-go-unit-pretty: check-tparse .PHONY: test-go-integration test-go-integration: ## Run integration tests for backend with flags. @echo "test backend integration tests" - $(GO) test $(GO_RACE_FLAG) -count=1 -run "^TestIntegration" -covermode=atomic -coverprofile=integration.cov -timeout=5m $(GO_INTEGRATION_TESTS) + $(GO) test $(GO_RACE_FLAG) -count=1 -run "^TestIntegration" -covermode=atomic -coverprofile=integration.cov -timeout=5m $(GO_INTEGRATION_TESTS) $(GO_TEST_OUTPUT) .PHONY: test-go-integration-alertmanager test-go-integration-alertmanager: ## Run integration tests for the remote alertmanager (config taken from the mimir_backend block). From 56b030492fa733c36ea6c2ed4d13bd088740307a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 7 Mar 2025 10:38:11 +0000 Subject: [PATCH 05/39] Update `make docs` procedure (#101497) Co-authored-by: grafanabot Co-authored-by: Jack Baldry --- docs/docs.mk | 2 +- docs/make-docs | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/docs.mk b/docs/docs.mk index dfb6e9e501c..09308dc8604 100644 --- a/docs/docs.mk +++ b/docs/docs.mk @@ -125,7 +125,7 @@ update: ## Fetch the latest version of this Makefile and the `make-docs` script TOPIC_TYPES := concept multiple-tasks reference section task tutorial visualization .PHONY: $(patsubst %,topic/%,$(TOPIC_TYPES)) topic/%: ## Create a topic from the Writers' Toolkit template. Specify the topic type as the target, for example, `make topic/task TOPIC_PATH=sources/my-new-topic.md`. -topic/%: +$(patsubst %,topic/%,$(TOPIC_TYPES)): $(if $(TOPIC_PATH),,$(error "You must set the TOPIC_PATH variable to the path where the $(@F) topic will be created. For example: make $(@) TOPIC_PATH=sources/my-new-topic.md")) mkdir -p $(dir $(TOPIC_PATH)) curl -s -o $(TOPIC_PATH) https://raw.githubusercontent.com/grafana/writers-toolkit/refs/heads/main/docs/static/templates/$(@F)-template.md diff --git a/docs/make-docs b/docs/make-docs index 80c89aea7ce..f6d72f905dc 100755 --- a/docs/make-docs +++ b/docs/make-docs @@ -8,6 +8,12 @@ # [Semantic versioning](https://semver.org/) is used to help the reader identify the significance of changes. # Changes are relevant to this script and the support docs.mk GNU Make interface. # +# ## 8.5.2 (2025-02-28) +# +# ### Fixed +# +# - topic/ targets are no longer no-ops as a result of 8.5.1. +# # ## 8.5.1 (2025-02-18) # # ### Fixed From d3b3186713cc332cf904a3c99084f41a57239c40 Mon Sep 17 00:00:00 2001 From: Misi Date: Fri, 7 Mar 2025 12:01:18 +0100 Subject: [PATCH 06/39] Auth: Make ssoSettingsSAML GA and enabled by default (#101766) Make ssoSettingsSAML GA and enabled by default --- .../configure-grafana/feature-toggles/index.md | 2 +- pkg/services/featuremgmt/registry.go | 3 ++- pkg/services/featuremgmt/toggles_gen.csv | 2 +- pkg/services/featuremgmt/toggles_gen.json | 12 ++++++++---- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index a4a4163bb49..fbc2e986f46 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -64,6 +64,7 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `groupToNestedTableTransformation` | Enables the group to nested table transformation | Yes | | `newPDFRendering` | New implementation for the dashboard-to-PDF rendering | Yes | | `tlsMemcached` | Use TLS-enabled memcached in the enterprise caching feature | Yes | +| `ssoSettingsSAML` | Use the new SSO Settings API to configure the SAML connector | Yes | | `cloudWatchNewLabelParsing` | Updates CloudWatch label parsing to be more accurate | Yes | | `accessActionSets` | Introduces action sets for resource permissions. Also ensures that all folder editors and admins can create subfolders without needing any additional permissions. | Yes | | `newDashboardSharingComponent` | Enables the new sharing drawer design | Yes | @@ -111,7 +112,6 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `regressionTransformation` | Enables regression analysis transformation | | `onPremToCloudMigrations` | Enable the Grafana Migration Assistant, which helps you easily migrate on-prem resources, such as dashboards, folders, and data source configurations, to your Grafana Cloud stack. | | `alertingSaveStateCompressed` | Enables the compressed protobuf-based alert state storage | -| `ssoSettingsSAML` | Use the new SSO Settings API to configure the SAML connector | | `azureMonitorPrometheusExemplars` | Allows configuration of Azure Monitor as a data source that can provide Prometheus exemplars | | `ssoSettingsLDAP` | Use the new SSO Settings API to configure LDAP | | `improvedExternalSessionHandling` | Enables improved support for OAuth external sessions. After enabling this feature, users might need to re-authenticate themselves. | diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 89efcfe082e..cd247b94ffb 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1070,8 +1070,9 @@ var ( { Name: "ssoSettingsSAML", Description: "Use the new SSO Settings API to configure the SAML connector", - Stage: FeatureStagePublicPreview, + Stage: FeatureStageGeneralAvailability, Owner: identityAccessTeam, + Expression: "true", AllowSelfServe: true, }, { diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 5c19a1a1211..06439b40a21 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -140,7 +140,7 @@ kubernetesAggregator,experimental,@grafana/grafana-app-platform-squad,false,true expressionParser,experimental,@grafana/grafana-app-platform-squad,false,true,false groupByVariable,experimental,@grafana/dashboards-squad,false,false,false scopeFilters,experimental,@grafana/dashboards-squad,false,false,false -ssoSettingsSAML,preview,@grafana/identity-access-team,false,false,false +ssoSettingsSAML,GA,@grafana/identity-access-team,false,false,false oauthRequireSubClaim,experimental,@grafana/identity-access-team,false,false,false newDashboardWithFiltersAndGroupBy,experimental,@grafana/dashboards-squad,false,false,false cloudWatchNewLabelParsing,GA,@grafana/aws-datasources,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 9cb6e99b789..98851496d97 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -3848,14 +3848,18 @@ { "metadata": { "name": "ssoSettingsSAML", - "resourceVersion": "1718727528075", - "creationTimestamp": "2024-03-14T11:04:45Z" + "resourceVersion": "1741342789814", + "creationTimestamp": "2024-03-14T11:04:45Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-03-07 10:19:49.814084 +0000 UTC" + } }, "spec": { "description": "Use the new SSO Settings API to configure the SAML connector", - "stage": "preview", + "stage": "GA", "codeowner": "@grafana/identity-access-team", - "allowSelfServe": true + "allowSelfServe": true, + "expression": "true" } }, { From 0fa2dbdcdcff70eef9768139ad71ede16beb5e1d Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Fri, 7 Mar 2025 12:09:39 +0100 Subject: [PATCH 07/39] Alerting: Generate deterministic UID for folders created for Prometheus rule conversion (#101760) --- pkg/services/ngalert/store/namespace.go | 59 ++++++++- pkg/services/ngalert/store/namespace_test.go | 123 +++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-) diff --git a/pkg/services/ngalert/store/namespace.go b/pkg/services/ngalert/store/namespace.go index 2e00ed40371..63100cb7392 100644 --- a/pkg/services/ngalert/store/namespace.go +++ b/pkg/services/ngalert/store/namespace.go @@ -3,7 +3,10 @@ package store import ( "context" "errors" + "fmt" + "hash/fnv" "sort" + "strconv" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/dashboards" @@ -89,7 +92,14 @@ func (st DBstore) GetNamespaceByTitle(ctx context.Context, title string, orgID i } // GetOrCreateNamespaceByTitle gets or creates a namespace by title in the specified folder. +// +// To avoid race conditions when two concurrent requests try to create the same folder, +// we create folders with a deterministic UID based on the parent UID, title, and organization ID. func (st DBstore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.Folder, error) { + if len(title) == 0 { + return nil, fmt.Errorf("title is empty") + } + var f *folder.Folder var err error @@ -99,7 +109,14 @@ func (st DBstore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, } if f == nil { + // Generate a deterministic UID with an alerting prefix + uid, err := generateAlertingFolderUID(title, parentUID, orgID) + if err != nil { + return nil, fmt.Errorf("error creating a new folder: %w", err) + } + cmd := &folder.CreateFolderCommand{ + UID: uid, OrgID: orgID, Title: title, SignedInUser: user, @@ -107,9 +124,49 @@ func (st DBstore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, } f, err = st.FolderService.Create(ctx, cmd) if err != nil { - return nil, err + // Handle potential race condition where another request might have created + // the folder between our check and creation attempt + existingFolder, lookupErr := st.GetNamespaceByTitle(ctx, title, orgID, user, parentUID) + if lookupErr == nil { + return existingFolder, nil + } + + // If we couldn't find it, return errors + return nil, fmt.Errorf("failed to get or create folder: %w", errors.Join( + fmt.Errorf("create folder: %w", err), + fmt.Errorf("lookup folder: %w", lookupErr), + )) } } return f, nil } + +// generateAlertingFolderUID creates a deterministic UID for folders +// based on the title and parent UID to avoid race conditions when multiple +// identical folders are created concurrently +func generateAlertingFolderUID(title string, parentUID string, orgID int64) (string, error) { + h := fnv.New64a() + + hashData := [][]byte{ + []byte(parentUID), + {0}, // separator + []byte(title), + {0}, + []byte(strconv.FormatInt(orgID, 10)), + } + + // Add hashData strings to the hash with a separator between them + for _, data := range hashData { + _, err := h.Write(data) + if err != nil { + return "", err + } + } + + // Create a deterministic string with alerting prefix + base36 := strconv.FormatUint(h.Sum64(), 36) + uid := fmt.Sprintf("alerting-%s", base36) + + return uid, nil +} diff --git a/pkg/services/ngalert/store/namespace_test.go b/pkg/services/ngalert/store/namespace_test.go index 7693122cee6..6cc1c610dba 100644 --- a/pkg/services/ngalert/store/namespace_test.go +++ b/pkg/services/ngalert/store/namespace_test.go @@ -2,6 +2,7 @@ package store import ( "context" + "strings" "testing" "github.com/google/uuid" @@ -206,6 +207,13 @@ func TestIntegration_GetOrCreateNamespaceByTitle(t *testing.T) { return store } + t.Run("should return error when title is empty", func(t *testing.T) { + store := setupStore(t) + _, err := store.GetOrCreateNamespaceByTitle(context.Background(), "", 1, u, folder.RootFolderUID) + require.Error(t, err) + require.Contains(t, err.Error(), "title is empty") + }) + t.Run("should create folder when it does not exist", func(t *testing.T) { store := setupStore(t) @@ -317,6 +325,65 @@ func TestIntegration_GetOrCreateNamespaceByTitle(t *testing.T) { require.Equal(t, folder3.UID, gotFolder3.UID) require.Equal(t, folder.RootFolderUID, gotFolder3.ParentUID) }) + + t.Run("should create folder with deterministic UID and handle race conditions", func(t *testing.T) { + store := setupStore(t) + + folderTitle := "race condition test folder" + parentUID := folder.RootFolderUID + + // Calculate the expected UID that would be generated + expectedUID, err := generateAlertingFolderUID(folderTitle, parentUID, 1) + require.NoError(t, err) + + // Create a folder first, simulating another concurrent call that succeeded first + createFolder(t, store, expectedUID, folderTitle, 1, parentUID) + + // Now try to create a folder with the same title and parent + // This should not create a duplicate folder but return the existing one + f, err := store.GetOrCreateNamespaceByTitle(context.Background(), folderTitle, 1, u, parentUID) + require.NoError(t, err) + require.Equal(t, expectedUID, f.UID, "Should return folder with same UID as would be deterministically generated") + require.Equal(t, folderTitle, f.Title) + require.Equal(t, parentUID, f.ParentUID) + + // Verify only one folder was created + folders, err := store.FolderService.GetFolders( + context.Background(), + folder.GetFoldersQuery{ + OrgID: 1, + WithFullpath: true, + SignedInUser: u, + }, + ) + require.NoError(t, err) + require.Len(t, folders, 1, "Only one folder should exist") + }) + + t.Run("should handle special characters in folder titles", func(t *testing.T) { + store := setupStore(t) + specialTitles := []string{ + "folder/with/slashes", + "folder with spaces", + "folder-with-dashes", + "folder_with_underscores", + "folder.with.dots", + "!@#$%^&*()", + } + + for _, title := range specialTitles { + t.Run(title, func(t *testing.T) { + f, err := store.GetOrCreateNamespaceByTitle(context.Background(), title, 1, u, folder.RootFolderUID) + require.NoError(t, err) + require.Equal(t, title, f.Title) + + // Verify retrieval works + retrieved, err := store.GetNamespaceByTitle(context.Background(), title, 1, u, folder.RootFolderUID) + require.NoError(t, err) + require.Equal(t, f.UID, retrieved.UID) + }) + } + }) } func TestIntegration_GetNamespaceChildren(t *testing.T) { @@ -417,3 +484,59 @@ func TestIntegration_GetNamespaceChildren(t *testing.T) { require.ElementsMatch(t, []string{rootFolder1, rootFolder2}, []string{children[0].UID, children[1].UID}) }) } + +func TestGenerateAlertingFolderUID(t *testing.T) { + const orgID int64 = 1 + + t.Run("should generate deterministic UIDs for same inputs", func(t *testing.T) { + title := "Test Folder" + parentUID := "parent123" + + uid1, err := generateAlertingFolderUID(title, parentUID, orgID) + require.NoError(t, err) + require.True(t, util.IsValidShortUID(uid1), "Generated UID should be valid") + + uid2, err := generateAlertingFolderUID(title, parentUID, orgID) + require.NoError(t, err) + require.True(t, util.IsValidShortUID(uid2), "Generated UID should be valid") + + require.Equal(t, uid1, uid2, "UIDs should be identical for same inputs") + require.True(t, strings.HasPrefix(uid1, "alerting-"), "UID should have alerting prefix") + }) + + t.Run("should generate different UIDs for different inputs", func(t *testing.T) { + uid1, err := generateAlertingFolderUID("Folder1", "parent1", orgID) + require.NoError(t, err) + require.True(t, util.IsValidShortUID(uid1), "Generated UID should be valid") + + uid2, err := generateAlertingFolderUID("Folder2", "parent1", orgID) + require.NoError(t, err) + require.True(t, util.IsValidShortUID(uid2), "Generated UID should be valid") + + uid3, err := generateAlertingFolderUID("Folder1", "parent2", orgID) + require.NoError(t, err) + require.True(t, util.IsValidShortUID(uid3), "Generated UID should be valid") + + require.NotEqual(t, uid1, uid2, "UIDs should differ for different titles") + require.NotEqual(t, uid1, uid3, "UIDs should differ for different parent UIDs") + }) + + t.Run("should handle special characters in folder titles", func(t *testing.T) { + specialTitles := []string{ + "folder/with/slashes", + "folder with spaces", + "folder-with-dashes", + "folder_with_underscores", + "folder.with.dots", + "!@#$%^&*()", + } + + for _, title := range specialTitles { + t.Run(title, func(t *testing.T) { + uid, err := generateAlertingFolderUID(title, "parent123", orgID) + require.NoError(t, err) + require.True(t, util.IsValidShortUID(uid), "Generated UID should be valid") + }) + } + }) +} From f0d260ba5bfb288fa1b921b7657f67f91c2729f7 Mon Sep 17 00:00:00 2001 From: Ieva Date: Fri, 7 Mar 2025 11:38:15 +0000 Subject: [PATCH 08/39] Service Accounts: Don't show error pop-ups for Service Account and Renderer UI flows (#101679) don't show error pop-ups for SAs and renderer --- pkg/api/user.go | 18 ++++++++++-------- public/app/core/services/backend_srv.ts | 9 +++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/pkg/api/user.go b/pkg/api/user.go index 1fab0727d7d..22e92e59e7a 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -149,7 +149,7 @@ func (hs *HTTPServer) UpdateSignedInUser(c *contextmodel.ReqContext) response.Re cmd.Email = strings.TrimSpace(cmd.Email) cmd.Login = strings.TrimSpace(cmd.Login) - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -349,7 +349,7 @@ func (hs *HTTPServer) UpdateUserEmail(c *contextmodel.ReqContext) response.Respo // 403: forbiddenError // 500: internalServerError func (hs *HTTPServer) GetSignedInUserOrgList(c *contextmodel.ReqContext) response.Response { - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -369,7 +369,7 @@ func (hs *HTTPServer) GetSignedInUserOrgList(c *contextmodel.ReqContext) respons // 403: forbiddenError // 500: internalServerError func (hs *HTTPServer) GetSignedInUserTeamList(c *contextmodel.ReqContext) response.Response { - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -479,7 +479,7 @@ func (hs *HTTPServer) UserSetUsingOrg(c *contextmodel.ReqContext) response.Respo return response.Error(http.StatusBadRequest, "id is invalid", err) } - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -504,6 +504,7 @@ func (hs *HTTPServer) ChangeActiveOrgAndRedirectToHome(c *contextmodel.ReqContex } if !c.SignedInUser.IsIdentityType(claims.TypeUser) { + hs.log.Debug("Requested endpoint only available to users") c.JsonApiErr(http.StatusForbidden, "Endpoint only available for users", nil) return } @@ -548,7 +549,7 @@ func (hs *HTTPServer) ChangeUserPassword(c *contextmodel.ReqContext) response.Re return response.Error(http.StatusBadRequest, "bad request data", err) } - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -584,7 +585,7 @@ func (hs *HTTPServer) SetHelpFlag(c *contextmodel.ReqContext) response.Response return response.Error(http.StatusBadRequest, "id is invalid", err) } - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -614,7 +615,7 @@ func (hs *HTTPServer) SetHelpFlag(c *contextmodel.ReqContext) response.Response // 403: forbiddenError // 500: internalServerError func (hs *HTTPServer) ClearHelpFlags(c *contextmodel.ReqContext) response.Response { - userID, errResponse := getUserID(c) + userID, errResponse := hs.getUserID(c) if errResponse != nil { return errResponse } @@ -627,8 +628,9 @@ func (hs *HTTPServer) ClearHelpFlags(c *contextmodel.ReqContext) response.Respon return response.JSON(http.StatusOK, &util.DynMap{"message": "Help flag set", "helpFlags1": flags}) } -func getUserID(c *contextmodel.ReqContext) (int64, *response.NormalResponse) { +func (hs *HTTPServer) getUserID(c *contextmodel.ReqContext) (int64, *response.NormalResponse) { if !c.SignedInUser.IsIdentityType(claims.TypeUser) { + hs.log.Debug("Requested endpoint only available to users") return 0, response.Error(http.StatusForbidden, "Endpoint only available for users", nil) } diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index cc667ac6c56..3f2c645a300 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -347,6 +347,15 @@ export class BackendSrv implements BackendService { } showErrorAlert(config: BackendSrvRequest, err: FetchError) { + // do not show error alerts for api keys or render tokens, they are used for kiosk mode and reporting and can't react to error pop-ups + if ( + this.dependencies.contextSrv.isSignedIn && + (this.dependencies.contextSrv.user.authenticatedBy === 'apikey' || + this.dependencies.contextSrv.user.authenticatedBy === 'render') + ) { + return; + } + if (config.showErrorAlert === false) { return; } From 654467401159ec91bad7efa9b1a939354e69a93d Mon Sep 17 00:00:00 2001 From: Will Assis <35489495+gassiss@users.noreply.github.com> Date: Fri, 7 Mar 2025 09:04:59 -0300 Subject: [PATCH 09/39] fix(unified-storage): Fix dualwriter DELETE mode3 not returning error from legacy (#101728) * Fix dualwrite package not returning error when getting a non-not found error from legacy storage in mode --- pkg/storage/legacysql/dualwrite/dualwriter.go | 7 ++++--- .../legacysql/dualwrite/dualwriter_mode3_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pkg/storage/legacysql/dualwrite/dualwriter.go b/pkg/storage/legacysql/dualwrite/dualwriter.go index 242b401b856..7523d549e59 100644 --- a/pkg/storage/legacysql/dualwrite/dualwriter.go +++ b/pkg/storage/legacysql/dualwrite/dualwriter.go @@ -95,7 +95,7 @@ func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValida storageObj, errObjectSt := d.unified.Create(ctx, createdCopy, createValidation, options) if errObjectSt != nil { - log.Error("unable to create object in unified storage", "err", err) + log.Error("unable to create object in unified storage", "err", errObjectSt) if d.errorIsOK { return createdFromLegacy, nil } @@ -105,12 +105,13 @@ func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValida if err != nil { log.Error("unable to cleanup object in legacy storage", "err", err) } + return storageObj, errObjectSt } if d.readUnified { return storageObj, errObjectSt } - return createdFromLegacy, errObjectSt + return createdFromLegacy, err } func (d *dualWriter) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { @@ -121,7 +122,7 @@ func (d *dualWriter) Delete(ctx context.Context, name string, deleteValidation r // but legacy failed, the user would get a failure, but not be able to retry the delete // as they would not be able to see the object in unistore anymore. objFromLegacy, asyncLegacy, err := d.legacy.Delete(ctx, name, deleteValidation, options) - if err != nil && !d.readUnified { + if err != nil && (!d.readUnified || !d.errorIsOK && !apierrors.IsNotFound(err)) { return objFromLegacy, asyncLegacy, err } diff --git a/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go b/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go index c53be55b64c..bd55753c949 100644 --- a/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go +++ b/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go @@ -256,6 +256,16 @@ func TestMode3_Delete(t *testing.T) { }, wantErr: true, }, + { + name: "should return an error when deleting an object in the LegacyStorage fails", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Delete", mock.Anything, input, mock.Anything, mock.Anything).Return(nil, false, apierrors.NewInternalError(errors.New("error"))) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Delete", mock.Anything, input, mock.Anything, mock.Anything).Panic("i should not be called") + }, + wantErr: true, + }, } name := "foo" From cc80681beb03d218b483750a0850abd519e1aaf3 Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Fri, 7 Mar 2025 13:51:50 +0100 Subject: [PATCH 10/39] Alerting: Extend recording rules test to exercise writing with data sources. (#101775) The change to use WriteDatasource was done in a previous commit, this adds a test case using DatasourceWriter, in addition to the one using PrometheusWriter. --- .../ngalert/schedule/recording_rule_test.go | 48 +++++++++++++++++-- pkg/services/ngalert/schedule/schedule.go | 1 - pkg/services/ngalert/writer/fake.go | 8 ---- pkg/services/ngalert/writer/testing.go | 9 +++- 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/pkg/services/ngalert/schedule/recording_rule_test.go b/pkg/services/ngalert/schedule/recording_rule_test.go index 8e04ff21886..6979d8469b7 100644 --- a/pkg/services/ngalert/schedule/recording_rule_test.go +++ b/pkg/services/ngalert/schedule/recording_rule_test.go @@ -20,6 +20,8 @@ import ( "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/datasources" + dsfakes "github.com/grafana/grafana/pkg/services/datasources/fakes" "github.com/grafana/grafana/pkg/services/ngalert/metrics" models "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/writer" @@ -173,18 +175,33 @@ func blankRecordingRuleForTests(ctx context.Context) *recordingRule { } func TestRecordingRule_Integration(t *testing.T) { + t.Run("with prometheus writer", func(t *testing.T) { + writeTarget := writer.NewTestRemoteWriteTarget(t) + defer writeTarget.Close() + writerReg := prometheus.NewPedanticRegistry() + writer := setupPrometheusWriter(t, writeTarget, writerReg) + testRecordingRule_Integration(t, writeTarget, writer, writerReg, "") + }) + t.Run("with datasource writer", func(t *testing.T) { + writeTarget := writer.NewTestRemoteWriteTarget(t) + defer writeTarget.Close() + writerReg := prometheus.NewPedanticRegistry() + writer := setupDatasourceWriter(t, writeTarget, writerReg, "ds-uid") + testRecordingRule_Integration(t, writeTarget, writer, writerReg, "ds-uid") + }) +} + +func testRecordingRule_Integration(t *testing.T, writeTarget *writer.TestRemoteWriteTarget, writer RecordingWriter, writerReg *prometheus.Registry, dsUID string) { gen := models.RuleGen.With(models.RuleGen.WithAllRecordingRules(), models.RuleGen.WithOrgID(123)) ruleStore := newFakeRulesStore() reg := prometheus.NewPedanticRegistry() sch := setupScheduler(t, ruleStore, nil, reg, nil, nil, nil) - writeTarget := writer.NewTestRemoteWriteTarget(t) - defer writeTarget.Close() - writerReg := prometheus.NewPedanticRegistry() - sch.recordingWriter = setupWriter(t, writeTarget, writerReg) + sch.recordingWriter = writer t.Run("rule that succeeds", func(t *testing.T) { writeTarget.Reset() rule := gen.With(withQueryForHealth("ok")).GenerateRef() + rule.Record.TargetDatasourceUID = dsUID ruleStore.PutRule(context.Background(), rule) folderTitle := ruleStore.getNamespaceTitle(rule.NamespaceUID) ruleFactory := ruleFactoryFromScheduler(sch) @@ -543,7 +560,7 @@ func withQueryForHealth(health string) models.AlertRuleMutator { } } -func setupWriter(t *testing.T, target *writer.TestRemoteWriteTarget, reg prometheus.Registerer) *writer.PrometheusWriter { +func setupPrometheusWriter(t *testing.T, target *writer.TestRemoteWriteTarget, reg prometheus.Registerer) *writer.PrometheusWriter { provider := testClientProvider{} m := metrics.NewNGAlert(reg) wr, err := writer.NewPrometheusWriterWithSettings(target.ClientSettings(), provider, clock.NewMock(), log.NewNopLogger(), m.GetRemoteWriterMetrics()) @@ -551,6 +568,27 @@ func setupWriter(t *testing.T, target *writer.TestRemoteWriteTarget, reg prometh return wr } +func setupDatasourceWriter(t *testing.T, target *writer.TestRemoteWriteTarget, reg prometheus.Registerer, dsUID string) *writer.DatasourceWriter { + provider := testClientProvider{} + m := metrics.NewNGAlert(reg) + + dss := &dsfakes.FakeDataSourceService{} + p1, _ := dss.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + UID: dsUID, + Type: datasources.DS_PROMETHEUS, + }) + p1.URL = target.DatasourceURL() + + cfg := writer.DatasourceWriterConfig{ + Timeout: time.Second * 5, + DefaultDatasourceUID: "", + RemoteWritePathSuffix: writer.RemoteWriteSuffix, + } + + return writer.NewDatasourceWriter(cfg, dss, provider, clock.NewMock(), + log.New("test"), m.GetRemoteWriterMetrics()) +} + type testClientProvider struct{} func (t testClientProvider) New(options ...httpclient.Options) (*http.Client, error) { diff --git a/pkg/services/ngalert/schedule/schedule.go b/pkg/services/ngalert/schedule/schedule.go index e3515ef14b4..3a77593061c 100644 --- a/pkg/services/ngalert/schedule/schedule.go +++ b/pkg/services/ngalert/schedule/schedule.go @@ -49,7 +49,6 @@ type RulesStore interface { } type RecordingWriter interface { - Write(ctx context.Context, name string, t time.Time, frames data.Frames, orgID int64, extraLabels map[string]string) error WriteDatasource(ctx context.Context, dsUID string, name string, t time.Time, frames data.Frames, orgID int64, extraLabels map[string]string) error } diff --git a/pkg/services/ngalert/writer/fake.go b/pkg/services/ngalert/writer/fake.go index 17a597bd43a..6ba176c6006 100644 --- a/pkg/services/ngalert/writer/fake.go +++ b/pkg/services/ngalert/writer/fake.go @@ -12,14 +12,6 @@ type FakeWriter struct { WriteFunc func(ctx context.Context, name string, t time.Time, frames data.Frames, orgID int64, extraLabels map[string]string) error } -func (w FakeWriter) Write(ctx context.Context, name string, t time.Time, frames data.Frames, orgID int64, extraLabels map[string]string) error { - if w.WriteFunc == nil { - return nil - } - - return w.WriteFunc(ctx, name, t, frames, orgID, extraLabels) -} - func (w FakeWriter) WriteDatasource(ctx context.Context, dsUID string, name string, t time.Time, frames data.Frames, orgID int64, extraLabels map[string]string) error { if w.WriteFunc == nil { return nil diff --git a/pkg/services/ngalert/writer/testing.go b/pkg/services/ngalert/writer/testing.go index 79bed8c1dbe..1996e712247 100644 --- a/pkg/services/ngalert/writer/testing.go +++ b/pkg/services/ngalert/writer/testing.go @@ -12,7 +12,10 @@ import ( "github.com/stretchr/testify/require" ) -const RemoteWriteEndpoint = "/api/v1/write" +const RemoteWritePrefix = "/api/v1" +const RemoteWriteSuffix = "/write" + +const RemoteWriteEndpoint = RemoteWritePrefix + RemoteWriteSuffix type TestRemoteWriteTarget struct { srv *httptest.Server @@ -59,6 +62,10 @@ func (s *TestRemoteWriteTarget) Close() { s.srv.Close() } +func (s *TestRemoteWriteTarget) DatasourceURL() string { + return s.srv.URL + RemoteWritePrefix +} + func (s *TestRemoteWriteTarget) ClientSettings() setting.RecordingRuleSettings { return setting.RecordingRuleSettings{ URL: s.srv.URL + RemoteWriteEndpoint, From 5917ed822726fb9f726d54b96524fd1a183e9104 Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Fri, 7 Mar 2025 14:06:47 +0100 Subject: [PATCH 11/39] Hackaton: Add more unit tests, take 4 (#101704) * cloudmigration/cloudmigrationimpl: run integration tests in parallel * tsdb/mysql: run tests with actual service and instance manager * pluginsintegration/angulardetectorsprovider: reduce job interval in test * util: extract test that should only be ran with -race enabled and unskip it --- .../cloudmigrationimpl/cloudmigration_test.go | 49 +++- .../snapshot_mgmt_alerts_test.go | 28 ++ .../cloudmigrationimpl/xorm_store_test.go | 22 +- .../angulardetectorsprovider/dynamic_test.go | 4 +- pkg/tsdb/mysql/mysql_test.go | 269 ++++++++++-------- pkg/util/shortid_generator_race_test.go | 25 ++ pkg/util/shortid_generator_test.go | 23 -- 7 files changed, 266 insertions(+), 154 deletions(-) create mode 100644 pkg/util/shortid_generator_race_test.go diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go index 348bf8b1e98..246e74c6da7 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go @@ -20,7 +20,6 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/tracing" @@ -47,6 +46,7 @@ import ( "github.com/grafana/grafana/pkg/services/quota/quotatest" secretsfakes "github.com/grafana/grafana/pkg/services/secrets/fakes" secretskv "github.com/grafana/grafana/pkg/services/secrets/kvstore" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" @@ -60,6 +60,8 @@ func Test_NoopServiceDoesNothing(t *testing.T) { } func Test_CreateGetAndDeleteToken(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false) createResp, err := s.CreateToken(context.Background()) @@ -82,6 +84,8 @@ func Test_CreateGetAndDeleteToken(t *testing.T) { } func Test_GetSnapshotStatusFromGMS(t *testing.T) { + t.Parallel() + setupTest := func(ctx context.Context) (service *Service, snapshotUID string, sessionUID string) { s := setUpServiceTest(t, false).(*Service) @@ -150,6 +154,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { } t.Run("test case: gms snapshot initialized", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -172,6 +178,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { }) t.Run("test case: gms snapshot processing", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -194,6 +202,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { }) t.Run("test case: gms snapshot finished", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -216,6 +226,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { }) t.Run("test case: gms snapshot canceled", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -238,6 +250,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { }) t.Run("test case: gms snapshot error", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -260,6 +274,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { }) t.Run("test case: gms snapshot unknown", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -292,6 +308,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { }) t.Run("GMS results applied to local snapshot", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -335,6 +353,8 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { } func Test_OnlyQueriesStatusFromGMSWhenRequired(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) gmsClientMock := &gmsClientMock{ @@ -407,6 +427,8 @@ func Test_OnlyQueriesStatusFromGMSWhenRequired(t *testing.T) { } func Test_DeletedDashboardsNotMigrated(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) // modify what the mock returns for just this test case @@ -454,6 +476,8 @@ func Test_SortFolders(t *testing.T) { } func TestDeleteSession(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) user := &user.SignedInUser{UserUID: "user123"} @@ -496,7 +520,11 @@ func TestDeleteSession(t *testing.T) { } func TestReportEvent(t *testing.T) { + t.Parallel() + t.Run("when the session is nil, it does not report the event", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -513,6 +541,8 @@ func TestReportEvent(t *testing.T) { }) t.Run("when the session is not nil, it reports the event", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -530,6 +560,8 @@ func TestReportEvent(t *testing.T) { } func TestGetFolderNamesForFolderUIDs(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -594,6 +626,8 @@ func TestGetFolderNamesForFolderUIDs(t *testing.T) { } func TestGetParentNames(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -654,6 +688,8 @@ func TestGetParentNames(t *testing.T) { } func TestGetLibraryElementsCommands(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) ctx, cancel := context.WithCancel(context.Background()) @@ -718,6 +754,8 @@ func TestIsPublicSignatureType(t *testing.T) { } func TestGetPlugins(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) ctx, cancel := context.WithCancel(context.Background()) @@ -817,7 +855,6 @@ func TestGetPlugins(t *testing.T) { type configOverrides func(c *setting.Cfg) func setUpServiceTest(t *testing.T, withDashboardMock bool, cfgOverrides ...configOverrides) cloudmigration.Service { - sqlStore := db.InitTestDB(t) secretsService := secretsfakes.NewFakeSecretsService() rr := routing.NewRouteRegister() tracer := tracing.InitializeTracerForTest() @@ -862,6 +899,14 @@ func setUpServiceTest(t *testing.T, withDashboardMock bool, cfgOverrides ...conf featuremgmt.FlagDashboardRestore, // needed for skipping creating soft-deleted dashboards in the snapshot. ) + sqlStore := sqlstore.NewTestStore(t, + sqlstore.WithCfg(cfg), + sqlstore.WithFeatureFlags( + featuremgmt.FlagOnPremToCloudMigrations, + featuremgmt.FlagDashboardRestore, // needed for skipping creating soft-deleted dashboards in the snapshot. + ), + ) + kvStore := kvstore.ProvideService(sqlStore) bus := bus.ProvideBus(tracer) diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go index 91d7ecc5cd7..c7a42105dd8 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go @@ -22,10 +22,14 @@ import ( ) func TestGetAlertMuteTimings(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) t.Run("it returns the mute timings", func(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) s.features = featuremgmt.WithFeatures(featuremgmt.FlagOnPremToCloudMigrations) @@ -42,10 +46,14 @@ func TestGetAlertMuteTimings(t *testing.T) { } func TestGetNotificationTemplates(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) t.Run("it returns the notification templates", func(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -61,10 +69,14 @@ func TestGetNotificationTemplates(t *testing.T) { } func TestGetContactPoints(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) t.Run("it returns the contact points", func(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) user := &user.SignedInUser{ @@ -89,10 +101,14 @@ func TestGetContactPoints(t *testing.T) { } func TestGetNotificationPolicies(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) t.Run("it returns the contact points", func(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -113,10 +129,14 @@ func TestGetNotificationPolicies(t *testing.T) { } func TestGetAlertRules(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) t.Run("it returns the alert rules", func(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -130,6 +150,8 @@ func TestGetAlertRules(t *testing.T) { }) t.Run("when the alert_rules_state config is `paused`, then the alert rules are all returned in `paused` state", func(t *testing.T) { + t.Parallel() + alertRulesState := func(c *setting.Cfg) { c.CloudMigration.AlertRulesState = setting.GMSAlertRulesPaused } @@ -153,10 +175,14 @@ func TestGetAlertRules(t *testing.T) { } func TestGetAlertRuleGroups(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) t.Run("it returns the alert rule groups", func(t *testing.T) { + t.Parallel() + s := setUpServiceTest(t, false).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -190,6 +216,8 @@ func TestGetAlertRuleGroups(t *testing.T) { }) t.Run("with the alert rules state set to paused, it returns the alert rule groups with alert rules paused", func(t *testing.T) { + t.Parallel() + alertRulesState := func(c *setting.Cfg) { c.CloudMigration.AlertRulesState = setting.GMSAlertRulesPaused } diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go index 03ab9065c57..9b86735c5d6 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go @@ -6,21 +6,17 @@ import ( "strconv" "testing" - "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/cloudmigration" fakeSecrets "github.com/grafana/grafana/pkg/services/secrets/fakes" secretskv "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/services/sqlstore" - "github.com/grafana/grafana/pkg/tests/testsuite" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestMain(m *testing.M) { - testsuite.Run(m) -} - func Test_GetAllCloudMigrationSessions(t *testing.T) { + t.Parallel() + _, s := setUpTest(t) ctx := context.Background() @@ -47,6 +43,8 @@ func Test_GetAllCloudMigrationSessions(t *testing.T) { } func Test_CreateMigrationSession(t *testing.T) { + t.Parallel() + _, s := setUpTest(t) ctx := context.Background() @@ -77,8 +75,11 @@ func Test_CreateMigrationSession(t *testing.T) { } func Test_GetMigrationSessionByUID(t *testing.T) { + t.Parallel() + _, s := setUpTest(t) ctx := context.Background() + t.Run("find session by uid", func(t *testing.T) { uid := "qwerty" orgId := int64(1) @@ -114,6 +115,8 @@ func Test_DeleteMigrationSession(t *testing.T) { */ func Test_SnapshotManagement(t *testing.T) { + t.Parallel() + _, s := setUpTest(t) ctx := context.Background() @@ -166,6 +169,8 @@ func Test_SnapshotManagement(t *testing.T) { } func Test_SnapshotResources(t *testing.T) { + t.Parallel() + _, s := setUpTest(t) ctx := context.Background() @@ -242,6 +247,8 @@ func Test_SnapshotResources(t *testing.T) { } func TestGetSnapshotList(t *testing.T) { + t.Parallel() + _, s := setUpTest(t) // Taken from setUpTest sessionUID := "qwerty" @@ -367,7 +374,8 @@ func TestDecryptToken(t *testing.T) { } func setUpTest(t *testing.T) (*sqlstore.SQLStore, *sqlStore) { - testDB := db.InitTestDB(t) + testDB := sqlstore.NewTestStore(t) + s := &sqlStore{ db: testDB, secretsService: fakeSecrets.FakeSecretsService{}, diff --git a/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go index b5c01e5ec23..f2211e14f57 100644 --- a/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go +++ b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go @@ -337,7 +337,7 @@ func TestDynamicAngularDetectorsProviderBackgroundService(t *testing.T) { t.Run("background service", func(t *testing.T) { oldBackgroundJobInterval := backgroundJobIntervalOnPrem - backgroundJobIntervalOnPrem = time.Millisecond * 500 + backgroundJobIntervalOnPrem = 10 * time.Millisecond t.Cleanup(func() { backgroundJobIntervalOnPrem = oldBackgroundJobInterval }) @@ -387,7 +387,7 @@ func TestDynamicAngularDetectorsProviderBackgroundService(t *testing.T) { lastJobTime := time.Now() var jobCalls counter - const jobInterval = time.Millisecond * 500 + const jobInterval = time.Millisecond * 20 done := make(chan struct{}) gcom := newDefaultGCOMScenario(func(_ http.ResponseWriter, _ *http.Request) { now := time.Now() diff --git a/pkg/tsdb/mysql/mysql_test.go b/pkg/tsdb/mysql/mysql_test.go index 014c56cadf5..3325ca7c071 100644 --- a/pkg/tsdb/mysql/mysql_test.go +++ b/pkg/tsdb/mysql/mysql_test.go @@ -1,8 +1,10 @@ package mysql import ( + "cmp" "context" "database/sql" + "encoding/json" "fmt" "math/rand" "os" @@ -10,6 +12,7 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/datasource" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/stretchr/testify/require" @@ -30,7 +33,7 @@ func TestIntegrationMySQL(t *testing.T) { } // change to true to run the MySQL tests runMySQLTests := false - // runMySqlTests := true + // runMySQLTests := true if !(isTestDbMySQL() || runMySQLTests) { t.Skip() @@ -45,33 +48,59 @@ func TestIntegrationMySQL(t *testing.T) { return sql } - dsInfo := sqleng.DataSourceInfo{ - JsonData: sqleng.JsonData{ - MaxOpenConns: 0, - MaxIdleConns: 2, - ConnMaxLifetime: 14400, + logger := backend.NewLoggerWith("logger", "mysql.test") + + jsonData := sqleng.JsonData{ + MaxOpenConns: 0, + MaxIdleConns: 2, + ConnMaxLifetime: 14400, + } + + rawJsonData, err := json.Marshal(&jsonData) + require.NoError(t, err) + + pluginCtx := backend.PluginContext{ + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ + ID: 0, + UID: "mysql-test", + URL: fmt.Sprintf("%s:%s", cmp.Or(os.Getenv("MYSQL_HOST"), "localhost"), cmp.Or(os.Getenv("MYSQL_PORT"), "3306")), + Type: "mysql", + Name: "mysql-test", + User: "grafana", + Database: "grafana_ds_tests", + JSONData: rawJsonData, + DecryptedSecureJSONData: map[string]string{ + "password": "password", + }, }, } - config := sqleng.DataPluginConfiguration{ - DSInfo: dsInfo, - TimeColumnNames: []string{"time", "time_sec"}, - MetricColumnTypes: []string{"CHAR", "VARCHAR", "TINYTEXT", "TEXT", "MEDIUMTEXT", "LONGTEXT"}, - RowLimit: 1000000, + cfg := backend.NewGrafanaCfg(map[string]string{ + backend.SQLMaxOpenConnsDefault: "0", + backend.SQLMaxIdleConnsDefault: "2", + backend.SQLMaxConnLifetimeSecondsDefault: "14400", + backend.SQLRowLimit: "1000000", + backend.UserFacingDefaultError: "", + }) + + ctx := backend.WithGrafanaConfig(context.Background(), cfg) + + exe := &Service{ + im: datasource.NewInstanceManager(NewInstanceSettings(logger)), + logger: logger, } - rowTransformer := mysqlQueryResultTransformer{} - - logger := backend.NewLoggerWith("logger", "mysql.test") - - db := InitMySQLTestDB(t, config.DSInfo.JsonData) - - exe, err := sqleng.NewQueryDataHandler("", db, config, &rowTransformer, newMysqlMacroEngine(logger, ""), logger) - - require.NoError(t, err) + db := InitMySQLTestDB(t, jsonData) fromStart := time.Date(2018, 3, 15, 13, 0, 0, 0, time.UTC) + queryWithPluginCtx := func(q backend.QueryDataRequest) *backend.QueryDataRequest { + return &backend.QueryDataRequest{ + PluginContext: pluginCtx, + Queries: q.Queries, + } + } + t.Run("Given a table with different native data types", func(t *testing.T) { _, err = db.Exec("DROP TABLE IF EXISTS mysql_types") require.NoError(t, err) @@ -125,7 +154,7 @@ func TestIntegrationMySQL(t *testing.T) { require.NoError(t, err) t.Run("Query with Table format should map MySQL column types to Go types", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -135,9 +164,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -217,7 +246,7 @@ func TestIntegrationMySQL(t *testing.T) { } t.Run("When doing a metric query using timeGroup", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -227,9 +256,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -261,7 +290,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using timeGroup with NULL fill enabled", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -275,9 +304,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -322,7 +351,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("Should replace $__interval", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -337,9 +366,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -350,7 +379,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using timeGroup with value fill enabled", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -364,9 +393,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -379,7 +408,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using timeGroup with previous fill enabled", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -393,9 +422,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -518,7 +547,7 @@ func TestIntegrationMySQL(t *testing.T) { } t.Run("When doing a metric query using time as time column should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -528,9 +557,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -543,7 +572,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using tinyint as value column should return metric with value in *float64", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -553,9 +582,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -567,7 +596,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using smallint as value column should return metric with value in *float64", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -577,9 +606,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -591,7 +620,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using time (nullable) as time column should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -601,9 +630,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -614,7 +643,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (int64) as time column and value column (int64) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -624,9 +653,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -637,7 +666,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (int64 nullable) as time column and value column (int64 nullable) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -647,9 +676,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -660,7 +689,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (float64) as time column and value column (float64) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -670,9 +699,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -683,7 +712,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (float64 nullable) as time column and value column (float64 nullable) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -693,9 +722,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -706,7 +735,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (int32) as time column and value column (int32) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -716,9 +745,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -729,7 +758,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (int32 nullable) as time column and value column (int32 nullable) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -739,9 +768,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -752,7 +781,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (float32) as time column and value column (float32) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -762,9 +791,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -776,7 +805,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query using epoch (float32 nullable) as time column and value column (float32 nullable) should return metric with time in time.Time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -786,9 +815,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -800,7 +829,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query grouping by time and select metric column should return correct series", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -810,9 +839,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -825,7 +854,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query with metric column and multiple value columns", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -835,9 +864,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -857,7 +886,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a metric query grouping by time should return correct series", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -867,9 +896,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -884,7 +913,7 @@ func TestIntegrationMySQL(t *testing.T) { t.Run("When doing a query with timeFrom,timeTo,unixEpochFrom,unixEpochTo macros", func(t *testing.T) { sqleng.Interpolate = origInterpolate - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -895,9 +924,9 @@ func TestIntegrationMySQL(t *testing.T) { TimeRange: backend.TimeRange{From: fromStart.Add(-5 * time.Minute), To: fromStart}, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -939,7 +968,7 @@ func TestIntegrationMySQL(t *testing.T) { } t.Run("When doing an annotation query of deploy events should return expected result", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -953,9 +982,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["Deploys"] @@ -966,7 +995,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing an annotation query of ticket events should return expected result", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -980,9 +1009,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["Tickets"] frames := queryResult.Frames @@ -995,16 +1024,16 @@ func TestIntegrationMySQL(t *testing.T) { dt := time.Date(2018, 3, 14, 21, 20, 6, 0, time.UTC) dtFormat := "2006-01-02 15:04:05.999999999" queryJson := fmt.Sprintf("{\"rawSql\": \"SELECT CAST('%s' as datetime) as time_sec, 'message' as text, 'tag1,tag2' as tags\", \"format\": \"table\"}", dt.Format(dtFormat)) - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(queryJson), RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1019,16 +1048,16 @@ func TestIntegrationMySQL(t *testing.T) { t.Run("When doing an annotation query with a time column in epoch second format should return ms", func(t *testing.T) { dt := time.Date(2018, 3, 14, 21, 20, 6, 527e6, time.UTC) queryJson := fmt.Sprintf("{\"rawSql\": \"SELECT %d as time_sec, 'message' as text, 'tag1,tag2' as tags\", \"format\": \"table\"}", dt.Unix()) - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(queryJson), RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1043,16 +1072,16 @@ func TestIntegrationMySQL(t *testing.T) { t.Run("When doing an annotation query with a time column in epoch second format (signed integer) should return ms", func(t *testing.T) { dt := time.Date(2018, 3, 14, 21, 20, 6, 0, time.Local) queryJson := fmt.Sprintf("{\"rawSql\": \"SELECT CAST('%d' as signed integer) as time_sec, 'message' as text, 'tag1,tag2' as tags\", \"format\": \"table\"}", dt.Unix()) - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(queryJson), RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1068,16 +1097,16 @@ func TestIntegrationMySQL(t *testing.T) { dt := time.Date(2018, 3, 14, 21, 20, 6, 527e6, time.UTC) queryJson := fmt.Sprintf("{\"rawSql\": \"SELECT %d as time_sec, 'message' as text, 'tag1,tag2' as tags\", \"format\": \"table\"}", dt.Unix()*1000) - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(queryJson), RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1090,7 +1119,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing an annotation query with a time column holding a unsigned integer null value should return nil", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -1100,9 +1129,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1116,7 +1145,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing an annotation query with a time column holding a DATETIME null value should return nil", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -1126,9 +1155,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1142,7 +1171,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing an annotation query with a time and timeend column should return two fields of type time", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -1152,9 +1181,9 @@ func TestIntegrationMySQL(t *testing.T) { RefID: "A", }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] require.NoError(t, queryResult.Error) @@ -1182,7 +1211,7 @@ func TestIntegrationMySQL(t *testing.T) { require.NoError(t, err) t.Run("When doing a table query that returns 2 rows should limit the result to 1 row", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -1196,7 +1225,7 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) resp, err := handler.QueryData(context.Background(), query) require.NoError(t, err) @@ -1212,7 +1241,7 @@ func TestIntegrationMySQL(t *testing.T) { }) t.Run("When doing a time series that returns 2 rows should limit the result to 1 row", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -1226,7 +1255,7 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) resp, err := handler.QueryData(context.Background(), query) require.NoError(t, err) @@ -1251,7 +1280,7 @@ func TestIntegrationMySQL(t *testing.T) { require.NoError(t, err) t.Run("When no rows are returned, should return an empty frame", func(t *testing.T) { - query := &backend.QueryDataRequest{ + query := queryWithPluginCtx(backend.QueryDataRequest{ Queries: []backend.DataQuery{ { JSON: []byte(`{ @@ -1265,9 +1294,9 @@ func TestIntegrationMySQL(t *testing.T) { }, }, }, - } + }) - resp, err := exe.QueryData(context.Background(), query) + resp, err := exe.QueryData(ctx, query) require.NoError(t, err) queryResult := resp.Responses["A"] diff --git a/pkg/util/shortid_generator_race_test.go b/pkg/util/shortid_generator_race_test.go new file mode 100644 index 00000000000..5ff85f93f87 --- /dev/null +++ b/pkg/util/shortid_generator_race_test.go @@ -0,0 +1,25 @@ +//go:build race +// +build race + +package util + +import ( + "sync" + "testing" +) + +// Run with "go test -race -run ^TestThreadSafe$ github.com/grafana/grafana/pkg/util" +func TestThreadSafe(t *testing.T) { + // Use 1000 go routines to create 100 UIDs each at roughly the same time. + var wg sync.WaitGroup + for i := 0; i < 1000; i++ { + go func() { + for ii := 0; ii < 100; ii++ { + _ = GenerateShortUID() + } + wg.Done() + }() + wg.Add(1) + } + wg.Wait() +} diff --git a/pkg/util/shortid_generator_test.go b/pkg/util/shortid_generator_test.go index 6bc4bb05b5c..b00e89d628c 100644 --- a/pkg/util/shortid_generator_test.go +++ b/pkg/util/shortid_generator_test.go @@ -2,7 +2,6 @@ package util import ( "fmt" - "sync" "testing" "cuelang.org/go/pkg/strings" @@ -19,25 +18,6 @@ func TestAllowedCharMatchesUidPattern(t *testing.T) { } } -// Run with "go test -race -run ^TestThreadSafe$ github.com/grafana/grafana/pkg/util" -func TestThreadSafe(t *testing.T) { - // This test was used to showcase the bug, unfortunately there is - // no way to enable the -race flag programmatically. - t.Skip() - // Use 1000 go routines to create 100 UIDs each at roughly the same time. - var wg sync.WaitGroup - for i := 0; i < 1000; i++ { - go func() { - for ii := 0; ii < 100; ii++ { - _ = GenerateShortUID() - } - wg.Done() - }() - wg.Add(1) - } - wg.Wait() -} - func TestRandomUIDs(t *testing.T) { for i := 0; i < 100; i++ { v := GenerateShortUID() @@ -48,10 +28,7 @@ func TestRandomUIDs(t *testing.T) { if validation != nil { t.Fatalf("created invalid name: %v", validation) } - - // fmt.Println(v) } - // t.FailNow() } func TestCaseInsensitiveCollisionsUIDs(t *testing.T) { From 14ebec527ccf22c15321ac8394a4f9c3909dd4e1 Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Fri, 7 Mar 2025 14:30:40 +0100 Subject: [PATCH 12/39] Alerting: Allow selection of recording rule write target on per-rule basis. (#101778) * Alerting: Allow selection of recording rule write target on per-rule basis. Introduces a new feature flag (`grafanaManagedRecordingRulesDatasources`), disabled by default, to enable the ability to write recording rules data using data source settings, and selecting the data source to use on a per-rule basis. To cope with the scenario of users upgrading, a configuration file option allows setting the default data source to use, if none is specified in the rule, emulating the behaviour of recording rules without the flag enabled. * Lint * Update conf/sample.ini Co-authored-by: Alexander Akhmetov --------- Co-authored-by: Alexander Akhmetov --- conf/defaults.ini | 7 +++++ conf/sample.ini | 7 +++++ .../src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 9 +++++++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 +++ pkg/services/featuremgmt/toggles_gen.json | 14 ++++++++++ pkg/services/ngalert/ngalert.go | 21 ++++++++++++--- pkg/setting/setting_unified_alerting.go | 26 +++++++++++-------- 9 files changed, 76 insertions(+), 14 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 97ad0aead32..a216bb0792a 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -1523,6 +1523,13 @@ basic_auth_password = # Request timeout for recording rule writes. timeout = 10s +# Default data source UID to write to if not specified in the rule definition. +# Only has effect if the grafanaManagedRecordRulesDatasources feature toggle is enabled. +default_datasource_uid = + +# Suffix to apply to the data source URL for remote write requests. +remote_write_path_suffix = /push + # Optional custom headers to include in recording rule write requests. [recording_rules.custom_headers] # exampleHeader = exampleValue diff --git a/conf/sample.ini b/conf/sample.ini index d70f031d6ab..21ef0ad88c6 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -1505,6 +1505,13 @@ basic_auth_password = # Request timeout for recording rule writes. timeout = 30s +# Default data source UID to write to if not specified in the rule definition. +# Only has effect if the grafanaManagedRecordRulesDatasources feature toggle is enabled. +default_datasource_uid = + +# Suffix to apply to the data source URL for remote write requests. +remote_write_path_suffix = /push + # Optional custom headers to include in recording rule write requests. [recording_rules.custom_headers] # exampleHeader = exampleValue diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index adc5f4641e8..f262e6aaf2f 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -256,4 +256,5 @@ export interface FeatureToggles { rendererDisableAppPluginsPreload?: boolean; assetSriChecks?: boolean; alertRuleRestore?: boolean; + grafanaManagedRecordingRulesDatasources?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index cd247b94ffb..f3834bb5757 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1790,6 +1790,15 @@ var ( Owner: grafanaAlertingSquad, Expression: "true", // enabled by default }, + { + Name: "grafanaManagedRecordingRulesDatasources", + Description: "Enables writing to data sources for Grafana-managed recording rules.", + Stage: FeatureStageExperimental, + Owner: grafanaAlertingSquad, + AllowSelfServe: false, + HideFromAdminPage: true, + HideFromDocs: true, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 06439b40a21..ff29806e399 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -237,3 +237,4 @@ newShareReportDrawer,experimental,@grafana/sharing-squad,false,false,false rendererDisableAppPluginsPreload,experimental,@grafana/sharing-squad,false,false,true assetSriChecks,experimental,@grafana/frontend-ops,false,false,true alertRuleRestore,preview,@grafana/alerting-squad,false,false,false +grafanaManagedRecordingRulesDatasources,experimental,@grafana/alerting-squad,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index fde91f9bc4e..957b81e842c 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -958,4 +958,8 @@ const ( // FlagAlertRuleRestore // Enables the alert rule restore feature FlagAlertRuleRestore = "alertRuleRestore" + + // FlagGrafanaManagedRecordingRulesDatasources + // Enables writing to data sources for Grafana-managed recording rules. + FlagGrafanaManagedRecordingRulesDatasources = "grafanaManagedRecordingRulesDatasources" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 98851496d97..0b40ecb9551 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -1964,6 +1964,20 @@ "hideFromDocs": true } }, + { + "metadata": { + "name": "grafanaManagedRecordingRulesDatasources", + "resourceVersion": "1741291902441", + "creationTimestamp": "2025-03-06T20:11:42Z" + }, + "spec": { + "description": "Enables writing to data sources for Grafana-managed recording rules.", + "stage": "experimental", + "codeowner": "@grafana/alerting-squad", + "hideFromAdminPage": true, + "hideFromDocs": true + } + }, { "metadata": { "name": "grafanaconThemes", diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index f09a20609d2..9f4a62656f6 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -374,7 +374,7 @@ func (ng *AlertNG) init() error { // Force-disable the feature if the feature toggle is not on - sets us up for feature toggle removal. ng.Cfg.UnifiedAlerting.RecordingRules.Enabled = false } - recordingWriter, err := createRecordingWriter(ng.FeatureToggles, ng.Cfg.UnifiedAlerting.RecordingRules, ng.httpClientProvider, clk, ng.Metrics.GetRemoteWriterMetrics()) + recordingWriter, err := createRecordingWriter(ng.FeatureToggles, ng.Cfg.UnifiedAlerting.RecordingRules, ng.httpClientProvider, ng.DataSourceService, clk, ng.Metrics.GetRemoteWriterMetrics()) if err != nil { return fmt.Errorf("failed to initialize recording writer: %w", err) } @@ -754,11 +754,26 @@ func createRemoteAlertmanager(cfg remote.AlertmanagerConfig, kvstore kvstore.KVS return remote.NewAlertmanager(cfg, notifier.NewFileStore(cfg.OrgID, kvstore), decryptFn, autogenFn, m, tracer) } -func createRecordingWriter(featureToggles featuremgmt.FeatureToggles, settings setting.RecordingRuleSettings, httpClientProvider httpclient.Provider, clock clock.Clock, m *metrics.RemoteWriter) (schedule.RecordingWriter, error) { +func createRecordingWriter(featureToggles featuremgmt.FeatureToggles, settings setting.RecordingRuleSettings, httpClientProvider httpclient.Provider, datasourceService datasources.DataSourceService, clock clock.Clock, m *metrics.RemoteWriter) (schedule.RecordingWriter, error) { logger := log.New("ngalert.writer") if settings.Enabled { - return writer.NewPrometheusWriterWithSettings(settings, httpClientProvider, clock, logger, m) + if featureToggles.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRulesDatasources) { + cfg := writer.DatasourceWriterConfig{ + Timeout: settings.Timeout, + DefaultDatasourceUID: settings.DefaultDatasourceUID, + RemoteWritePathSuffix: settings.RemoteWritePathSuffix, + } + + logger.Info("Setting up remote write using data sources", + "timeout", cfg.Timeout, "default_datasource_uid", cfg.DefaultDatasourceUID, + "remote_write_path_suffix", cfg.RemoteWritePathSuffix) + + return writer.NewDatasourceWriter(cfg, datasourceService, httpClientProvider, clock, logger, m), nil + } else { + logger.Info("Setting up remote write using static configuration") + return writer.NewPrometheusWriterWithSettings(settings, httpClientProvider, clock, logger, m) + } } return writer.NoopWriter{}, nil diff --git a/pkg/setting/setting_unified_alerting.go b/pkg/setting/setting_unified_alerting.go index 03bea25e0aa..bf359c34855 100644 --- a/pkg/setting/setting_unified_alerting.go +++ b/pkg/setting/setting_unified_alerting.go @@ -132,12 +132,14 @@ type UnifiedAlertingSettings struct { } type RecordingRuleSettings struct { - Enabled bool - URL string - BasicAuthUsername string - BasicAuthPassword string - CustomHeaders map[string]string - Timeout time.Duration + Enabled bool + URL string + BasicAuthUsername string + BasicAuthPassword string + CustomHeaders map[string]string + Timeout time.Duration + DefaultDatasourceUID string + RemoteWritePathSuffix string } // RemoteAlertmanagerSettings contains the configuration needed @@ -435,11 +437,13 @@ func (cfg *Cfg) ReadUnifiedAlertingSettings(iniFile *ini.File) error { rr := iniFile.Section("recording_rules") uaCfgRecordingRules := RecordingRuleSettings{ - Enabled: rr.Key("enabled").MustBool(false), - URL: rr.Key("url").MustString(""), - BasicAuthUsername: rr.Key("basic_auth_username").MustString(""), - BasicAuthPassword: rr.Key("basic_auth_password").MustString(""), - Timeout: rr.Key("timeout").MustDuration(defaultRecordingRequestTimeout), + Enabled: rr.Key("enabled").MustBool(false), + URL: rr.Key("url").MustString(""), + BasicAuthUsername: rr.Key("basic_auth_username").MustString(""), + BasicAuthPassword: rr.Key("basic_auth_password").MustString(""), + Timeout: rr.Key("timeout").MustDuration(defaultRecordingRequestTimeout), + DefaultDatasourceUID: rr.Key("default_datasource_uid").MustString(""), + RemoteWritePathSuffix: rr.Key("remote_write_path_suffix").MustString("/push"), } rrHeaders := iniFile.Section("recording_rules.custom_headers") From 392124de0059f92cbf41c6db84034a84134fa599 Mon Sep 17 00:00:00 2001 From: Ieva Date: Fri, 7 Mar 2025 13:37:26 +0000 Subject: [PATCH 13/39] Service Accounts: Don't show error pop-ups for Service Account and Renderer UI flows (#101776) * don't show error pop-ups for SAs and renderer * only hide non 4xx error pop'ups * linting --- pkg/api/user.go | 4 ++-- public/app/core/services/backend_srv.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/api/user.go b/pkg/api/user.go index 22e92e59e7a..8bc33207ac1 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -505,7 +505,7 @@ func (hs *HTTPServer) ChangeActiveOrgAndRedirectToHome(c *contextmodel.ReqContex if !c.SignedInUser.IsIdentityType(claims.TypeUser) { hs.log.Debug("Requested endpoint only available to users") - c.JsonApiErr(http.StatusForbidden, "Endpoint only available for users", nil) + c.JsonApiErr(http.StatusNotModified, "Endpoint only available for users", nil) return } @@ -631,7 +631,7 @@ func (hs *HTTPServer) ClearHelpFlags(c *contextmodel.ReqContext) response.Respon func (hs *HTTPServer) getUserID(c *contextmodel.ReqContext) (int64, *response.NormalResponse) { if !c.SignedInUser.IsIdentityType(claims.TypeUser) { hs.log.Debug("Requested endpoint only available to users") - return 0, response.Error(http.StatusForbidden, "Endpoint only available for users", nil) + return 0, response.Error(http.StatusNotModified, "Endpoint only available for users", nil) } userID, err := c.SignedInUser.GetInternalID() diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 3f2c645a300..f0f42225547 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -347,8 +347,9 @@ export class BackendSrv implements BackendService { } showErrorAlert(config: BackendSrvRequest, err: FetchError) { - // do not show error alerts for api keys or render tokens, they are used for kiosk mode and reporting and can't react to error pop-ups + // do not show non-user error alerts for api keys or render tokens, they are used for kiosk mode and reporting and can't react to error pop-ups if ( + (err.status < 400 || err.status >= 500) && this.dependencies.contextSrv.isSignedIn && (this.dependencies.contextSrv.user.authenticatedBy === 'apikey' || this.dependencies.contextSrv.user.authenticatedBy === 'render') From 6a1e5dd128f7ff509981d921d70e6b7d66c5742a Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Fri, 7 Mar 2025 15:01:39 +0100 Subject: [PATCH 14/39] AuthZ: Test List (#101721) * Authz: Test List * Anonymous case * Cover rendering --- pkg/services/authz/rbac/service_test.go | 293 ++++++++++++++++++++++++ 1 file changed, 293 insertions(+) diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go index 144b102d7eb..dce4b79da76 100644 --- a/pkg/services/authz/rbac/service_test.go +++ b/pkg/services/authz/rbac/service_test.go @@ -897,6 +897,299 @@ func TestService_Check(t *testing.T) { }) } +func TestService_List(t *testing.T) { + callingService := authn.NewAccessTokenAuthInfo(authn.Claims[authn.AccessTokenClaims]{ + Claims: jwt.Claims{ + Subject: types.NewTypeID(types.TypeAccessPolicy, "some-service"), + Audience: []string{"authzservice"}, + }, + Rest: authn.AccessTokenClaims{Namespace: "org-12"}, + }) + + type testCase struct { + name string + req *authzv1.ListRequest + permissions []accesscontrol.Permission + expected *authzv1.ListResponse + expectErr bool + } + + t.Run("Require auth info", func(t *testing.T) { + s := setupService() + ctx := context.Background() + _, err := s.List(ctx, &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "user:test-uid", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "could not get auth info") + }) + + testCases := []testCase{ + { + name: "should error if no namespace is provided", + req: &authzv1.ListRequest{ + Namespace: "", + Subject: "user:test-uid", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expectErr: true, + }, + { + name: "should error if caller namespace does not match request namespace", + req: &authzv1.ListRequest{ + Namespace: "org-13", + Subject: "user:test-uid", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expectErr: true, + }, + { + name: "should error if no subject is provided", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expectErr: true, + }, + { + name: "should error if an unsupported subject type is provided", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "api-key:12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expectErr: true, + }, + { + name: "should error if an invalid subject is provided", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "invalid:12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expectErr: true, + }, + { + name: "should error if an unknown group is provided", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "user:test-uid", + Group: "unknown.grafana.app", + Resource: "unknown", + Verb: "get", + }, + expectErr: true, + }, + { + name: "should error if an unknown verb is provided", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "user:test-uid", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "unknown", + }, + expectErr: true, + }, + } + t.Run("Request validation", func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + userID := &store.UserIdentifiers{UID: "test-uid", ID: 1} + store := &fakeStore{ + userID: userID, + userPermissions: tc.permissions, + } + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{teams: []int64{1, 2}} + + _, err := s.List(ctx, tc.req) + require.Error(t, err) + }) + } + }) + + testCases = []testCase{ + { + name: "should list permissions for user with permission", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "user:test-uid", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + permissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "dashboards:uid:dash1"}, + {Action: "dashboards:read", Scope: "dashboards:uid:dash2"}, + {Action: "dashboards:read", Scope: "folders:uid:fold1"}, + }, + expected: &authzv1.ListResponse{ + Items: []string{"dash1", "dash2"}, + Folders: []string{"fold1"}, + }, + }, + { + name: "should return empty list for user without permission", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "user:test-uid", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + permissions: []accesscontrol.Permission{}, + expected: &authzv1.ListResponse{}, + }, + } + t.Run("User permission list", func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + userID := &store.UserIdentifiers{UID: "test-uid", ID: 1} + store := &fakeStore{ + userID: userID, + userPermissions: tc.permissions, + } + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{teams: []int64{1, 2}} + + resp, err := s.List(ctx, tc.req) + require.NoError(t, err) + require.ElementsMatch(t, resp.Items, tc.expected.Items) + require.ElementsMatch(t, resp.Folders, tc.expected.Folders) + + // Check cache + id, ok := s.idCache.Get(ctx, userIdentifierCacheKey("org-12", "test-uid")) + require.True(t, ok) + require.Equal(t, id.UID, "test-uid") + perms, ok := s.permCache.Get(ctx, userPermCacheKey("org-12", "test-uid", "dashboards:read")) + require.True(t, ok) + require.Len(t, perms, len(tc.expected.Items)+len(tc.expected.Folders)) + }) + } + }) + + testCases = []testCase{ + { + name: "should list permissions for anonymous with permission", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "anonymous:0", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + permissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "dashboards:uid:dash1"}, + {Action: "dashboards:read", Scope: "dashboards:uid:dash2"}, + {Action: "dashboards:read", Scope: "folders:uid:fold1"}, + }, + expected: &authzv1.ListResponse{ + Items: []string{"dash1", "dash2"}, + Folders: []string{"fold1"}, + }, + }, + { + name: "should return empty list for anonymous without permission", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "anonymous:0", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + permissions: []accesscontrol.Permission{}, + expected: &authzv1.ListResponse{}, + }, + } + t.Run("Anonymous permission list", func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + store := &fakeStore{userPermissions: tc.permissions} + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{teams: []int64{1, 2}} + + resp, err := s.List(ctx, tc.req) + require.NoError(t, err) + require.ElementsMatch(t, resp.Items, tc.expected.Items) + require.ElementsMatch(t, resp.Folders, tc.expected.Folders) + + // Check cache + perms, ok := s.permCache.Get(ctx, anonymousPermCacheKey("org-12", "dashboards:read")) + require.True(t, ok) + require.Len(t, perms, len(tc.expected.Items)+len(tc.expected.Folders)) + }) + } + }) + + testCases = []testCase{ + { + name: "should list permissions for rendering", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "render:0", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expected: &authzv1.ListResponse{ + All: true, + }, + }, + { + name: "should deny rendering access to another app resources", + req: &authzv1.ListRequest{ + Namespace: "org-12", + Subject: "render:0", + Group: "another.grafana.app", + Resource: "dashboards", + Verb: "get", + }, + expectErr: true, + }, + } + t.Run("Rendering permission list", func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + + resp, err := s.List(ctx, tc.req) + if tc.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.expected.All, resp.All) + }) + } + }) +} + func setupService() *Service { cache := cache.NewLocalCache(cache.Config{Expiry: 5 * time.Minute, CleanupInterval: 5 * time.Minute}) logger := log.New("authz-rbac-service") From 2e0d2f3c7687d9fc4adda54f94c3ba4df6d5837e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?= Date: Fri, 7 Mar 2025 15:27:31 +0100 Subject: [PATCH 15/39] fix(unified-storage): break dependency from dualwriter in resource module (#101788) --- pkg/registry/apis/dashboard/search.go | 2 +- pkg/services/apiserver/client/client.go | 2 +- pkg/storage/legacysql/dualwrite/types.go | 14 ++++++++++++++ pkg/storage/unified/resource/search_client.go | 17 ++++++++++------- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/registry/apis/dashboard/search.go b/pkg/registry/apis/dashboard/search.go index 85f5ab2e654..7ade0969ead 100644 --- a/pkg/registry/apis/dashboard/search.go +++ b/pkg/registry/apis/dashboard/search.go @@ -43,7 +43,7 @@ type SearchHandler struct { } func NewSearchHandler(tracer trace.Tracer, dual dualwrite.Service, legacyDashboardSearcher resource.ResourceIndexClient, resourceClient resource.ResourceClient, features featuremgmt.FeatureToggles) *SearchHandler { - searchClient := resource.NewSearchClient(dual, dashboardv0alpha1.DashboardResourceInfo.GroupResource(), resourceClient, legacyDashboardSearcher) + searchClient := resource.NewSearchClient(dualwrite.NewSearchAdapter(dual), dashboardv0alpha1.DashboardResourceInfo.GroupResource(), resourceClient, legacyDashboardSearcher) return &SearchHandler{ client: searchClient, log: log.New("grafana-apiserver.dashboards.search"), diff --git a/pkg/services/apiserver/client/client.go b/pkg/services/apiserver/client/client.go index 021dc5d53eb..2b312f309bd 100644 --- a/pkg/services/apiserver/client/client.go +++ b/pkg/services/apiserver/client/client.go @@ -53,7 +53,7 @@ type k8sHandler struct { func NewK8sHandler(dual dualwrite.Service, namespacer request.NamespaceMapper, gvr schema.GroupVersionResource, restConfig func(context.Context) (*rest.Config, error), dashStore dashboards.Store, userSvc user.Service, resourceClient resource.ResourceClient, sorter sort.Service) K8sHandler { legacySearcher := legacysearcher.NewDashboardSearchClient(dashStore, sorter) - searchClient := resource.NewSearchClient(dual, gvr.GroupResource(), resourceClient, legacySearcher) + searchClient := resource.NewSearchClient(dualwrite.NewSearchAdapter(dual), gvr.GroupResource(), resourceClient, legacySearcher) return &k8sHandler{ namespacer: namespacer, diff --git a/pkg/storage/legacysql/dualwrite/types.go b/pkg/storage/legacysql/dualwrite/types.go index 0478aa5438e..a983432659c 100644 --- a/pkg/storage/legacysql/dualwrite/types.go +++ b/pkg/storage/legacysql/dualwrite/types.go @@ -50,3 +50,17 @@ type Service interface { // change the status (finish migration etc) Update(ctx context.Context, status StorageStatus) (StorageStatus, error) } + +type SearchAdapter struct { + Service +} + +func NewSearchAdapter(s Service) *SearchAdapter { + return &SearchAdapter{Service: s} +} + +func (d *SearchAdapter) IsEnabled(gr schema.GroupResource) bool { + //nolint:errcheck + status, _ := d.Status(context.Background(), gr) + return status.Runtime && d.Service.ShouldManage(gr) +} diff --git a/pkg/storage/unified/resource/search_client.go b/pkg/storage/unified/resource/search_client.go index c3ebeeea185..cf94ed29fb5 100644 --- a/pkg/storage/unified/resource/search_client.go +++ b/pkg/storage/unified/resource/search_client.go @@ -5,13 +5,15 @@ import ( "google.golang.org/grpc" "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" ) -func NewSearchClient(dual dualwrite.Service, gr schema.GroupResource, unifiedClient ResourceIndexClient, legacyClient ResourceIndexClient) ResourceIndexClient { - status, _ := dual.Status(context.Background(), gr) - if status.Runtime && dual.ShouldManage(gr) { +type DualWriter interface { + IsEnabled(schema.GroupResource) bool + ReadFromUnified(context.Context, schema.GroupResource) (bool, error) +} + +func NewSearchClient(dual DualWriter, gr schema.GroupResource, unifiedClient ResourceIndexClient, legacyClient ResourceIndexClient) ResourceIndexClient { + if dual.IsEnabled(gr) { return &searchWrapper{ dual: dual, groupResource: gr, @@ -19,14 +21,15 @@ func NewSearchClient(dual dualwrite.Service, gr schema.GroupResource, unifiedCli legacyClient: legacyClient, } } - if status.ReadUnified { + //nolint:errcheck + if ok, _ := dual.ReadFromUnified(context.Background(), gr); ok { return unifiedClient } return legacyClient } type searchWrapper struct { - dual dualwrite.Service + dual DualWriter groupResource schema.GroupResource unifiedClient ResourceIndexClient From 7ee3b9f04162ea4a8cd5afb07453c142cc51f7df Mon Sep 17 00:00:00 2001 From: Bogdan Matei Date: Fri, 7 Mar 2025 16:38:08 +0200 Subject: [PATCH 16/39] Dynamic Dashboards: Workaround for making responsive items selectable (#101794) --- .../scene/layout-responsive-grid/ResponsiveGridLayoutManager.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridLayoutManager.tsx b/public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridLayoutManager.tsx index 3861971b535..c5f2d0dd09a 100644 --- a/public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridLayoutManager.tsx +++ b/public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridLayoutManager.tsx @@ -48,6 +48,7 @@ export class ResponsiveGridLayoutManager // @ts-ignore this.state.layout.getDragClassCancel = () => 'drag-cancel'; + this.state.layout.isDraggable = () => true; } public addPanel(vizPanel: VizPanel) { From 04cc096b238423f8e02522781c7a9acf39e001ca Mon Sep 17 00:00:00 2001 From: Will Browne Date: Fri, 7 Mar 2025 14:48:55 +0000 Subject: [PATCH 17/39] Docs: Fix plugin installation title by removing non-breaking space (#101798) remove nbsp --- docs/sources/administration/plugin-management/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/administration/plugin-management/index.md b/docs/sources/administration/plugin-management/index.md index 5d89d1204c5..1f43832b69b 100644 --- a/docs/sources/administration/plugin-management/index.md +++ b/docs/sources/administration/plugin-management/index.md @@ -128,7 +128,7 @@ unzip my-plugin-0.2.0.zip -d YOUR_PLUGIN_DIR/my-plugin The path to the plugin directory is defined in the configuration file. For more information, refer to [Configuration](../../setup-grafana/configure-grafana/#plugins). -#### Install a plugin using Grafana configuration +#### Install a plugin using Grafana configuration {{% admonition type="note" %}} This feature requires Grafana 11.5.0 or later. From 3a3f781dcf5e858edee2f6daa2354edfb40afdee Mon Sep 17 00:00:00 2001 From: Isabel Matwawana <76437239+imatwawana@users.noreply.github.com> Date: Fri, 7 Mar 2025 09:55:39 -0500 Subject: [PATCH 18/39] Docs: Add dashboard variables content (#101733) * Added variables section * Updated transformations file and Makefile * Finished Dashboard variables in transformations section --- docs/Makefile | 4 ++-- .../transform-data/index.md | 10 ++++++++ scripts/docs/generate-transformations.ts | 24 ++++++++++++------- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/docs/Makefile b/docs/Makefile index 36b7e4f41be..7bdbe026293 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -9,6 +9,6 @@ include docs.mk .PHONY: sources/panels-visualizations/query-transform-data/transform-data/index.md sources/panels-visualizations/query-transform-data/transform-data/index.md: ## Generate the Transform Data page source. - cd $(CURDIR)/.. && npx tsc ./scripts/docs/generate-transformations.ts && \ - node -e "require('./scripts/docs/generate-transformations').buildMarkdownContent()" && \ + cd $(CURDIR)/.. && \ + npx tsx ./scripts/docs/generate-transformations.ts && \ npx prettier -w $(CURDIR)/$@ diff --git a/docs/sources/panels-visualizations/query-transform-data/transform-data/index.md b/docs/sources/panels-visualizations/query-transform-data/transform-data/index.md index 9fef087e422..cfc5a5e29e5 100644 --- a/docs/sources/panels-visualizations/query-transform-data/transform-data/index.md +++ b/docs/sources/panels-visualizations/query-transform-data/transform-data/index.md @@ -104,6 +104,16 @@ When there are multiple transformations, Grafana applies them in the order they The order in which Grafana applies transformations directly impacts the results. For example, if you use a Reduce transformation to condense all the results of one column into a single value, then you can only apply transformations to that single value. +## Dashboard variables in transformations + +All text input fields in transformations accept [variable syntax](ref:dashboard-variable): + +{{< figure src="/media/docs/grafana/panels-visualizations/screenshot-transformation-variables-v11.6.png" alt="Transformation with a mock variable in a text field" >}} + +When you use dashboard variables in transformations, the variables are automatically interpolated before the transformations are applied to the data. + +For an example, refer to [Use a dashboard variable](#use-a-dashboard-variable) in the **Filter fields by name** transformation. + ## Add a transformation function to data The following steps guide you in adding a transformation to data. This documentation does not include steps for each type of transformation. For a complete list of transformations, refer to [Transformation functions](#transformation-functions). diff --git a/scripts/docs/generate-transformations.ts b/scripts/docs/generate-transformations.ts index 23b6142c20e..a8e52c07fc9 100644 --- a/scripts/docs/generate-transformations.ts +++ b/scripts/docs/generate-transformations.ts @@ -118,6 +118,16 @@ When there are multiple transformations, Grafana applies them in the order they The order in which Grafana applies transformations directly impacts the results. For example, if you use a Reduce transformation to condense all the results of one column into a single value, then you can only apply transformations to that single value. +## Dashboard variables in transformations + +All text input fields in transformations accept [variable syntax](ref:dashboard-variable): + +{{< figure src="/media/docs/grafana/panels-visualizations/screenshot-transformation-variables-v11.6.png" alt="Transformation with a mock variable in a text field" >}} + +When you use dashboard variables in transformations, the variables are automatically interpolated before the transformations are applied to the data. + +For an example, refer to [Use a dashboard variable](#use-a-dashboard-variable) in the **Filter fields by name** transformation. + ## Add a transformation function to data The following steps guide you in adding a transformation to data. This documentation does not include steps for each type of transformation. For a complete list of transformations, refer to [Transformation functions](#transformation-functions). @@ -204,14 +214,6 @@ ${transformationDocsContent[transformationName].getHelperDocs(ImageRenderType.Sh return content; } -export function buildMarkdownContent(): void { - // Build the path to the Markdown file. - const indexPath = resolve(__dirname, '../../' + WRITE_PATH); - - // Write content to the Markdown file. - writeFileSync(indexPath, completeTemplate, 'utf-8'); -} - export function getMarkdownContent(): string { const rootDir = resolve(__dirname, '../../'); const pathToMarkdown = resolve(rootDir, WRITE_PATH); @@ -221,3 +223,9 @@ export function getMarkdownContent(): string { export function getJavaScriptContent(): string { return completeTemplate; } + +// Build the path to the Markdown file. +const indexPath = resolve(__dirname, '../../' + WRITE_PATH); + +// Write content to the Markdown file. +writeFileSync(indexPath, completeTemplate, 'utf-8'); From da8f26a07c819a4863ca4ac23ade0f5bbe7c3311 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Fri, 7 Mar 2025 16:15:55 +0100 Subject: [PATCH 19/39] Alerting: Refactor and clean up rule type guard functions (#101639) --- .../alerting/unified/api/alertRuleApi.ts | 4 +- .../unified/components/MenuItemPauseRule.tsx | 4 +- .../components/export/GrafanaModifyExport.tsx | 6 +- .../rule-editor/GrafanaEvaluationBehavior.tsx | 6 +- .../components/rule-editor/RuleInspector.tsx | 6 +- .../alert-rule-form/AlertRuleForm.tsx | 12 +- .../alert-rule-form/ModifyExportRuleForm.tsx | 4 +- .../components/rule-viewer/AlertRuleMenu.tsx | 6 +- .../rule-viewer/RuleViewer.test.tsx | 17 ++- .../components/rule-viewer/RuleViewer.tsx | 30 ++--- .../components/rule-viewer/tabs/Details.tsx | 42 +++---- .../components/rule-viewer/tabs/Query.tsx | 8 +- .../components/rules/EditRuleGroupModal.tsx | 6 +- .../rules/ReorderRuleGroupModal.tsx | 26 ++-- .../components/rules/RuleActionsButtons.tsx | 6 +- .../components/rules/RuleConfigStatus.tsx | 4 +- .../unified/components/rules/RuleDetails.tsx | 21 ++-- .../components/rules/RuleDetailsButtons.tsx | 6 +- .../rules/RuleDetailsDataSources.tsx | 4 +- .../rules/RuleDetailsMatchingInstances.tsx | 8 +- .../components/rules/RuleListStateView.tsx | 8 +- .../unified/components/rules/RuleState.tsx | 11 +- .../unified/components/rules/RulesGroup.tsx | 7 +- .../unified/components/rules/RulesTable.tsx | 8 +- .../ruleGroup/useUpsertRuleFromRuleGroup.ts | 4 +- .../alerting/unified/hooks/useAbilities.ts | 17 +-- .../hooks/useCombinedRuleNamespaces.ts | 50 ++++---- .../unified/hooks/useFilteredRules.ts | 16 +-- .../unified/hooks/useIsRuleEditable.ts | 6 +- .../unified/reducers/ruler/ruleGroups.ts | 13 +- .../unified/rule-editor/CloneRuleEditor.tsx | 10 +- .../rule-list/DataSourceRuleLoader.tsx | 6 +- .../alerting/unified/rule-list/StateView.tsx | 10 +- .../components/RuleActionsButtons.V2.tsx | 6 +- .../rule-list/components/RuleGroup.tsx | 5 +- .../hooks/useFilteredRulesIterator.ts | 4 +- .../alerting/unified/utils/groupIdentifier.ts | 4 +- .../features/alerting/unified/utils/query.ts | 4 +- .../alerting/unified/utils/rule-form.ts | 17 +-- .../alerting/unified/utils/rule-id.ts | 24 ++-- .../features/alerting/unified/utils/rules.ts | 119 ++++++++++-------- .../scene/AlertStatesDataLayer.ts | 4 +- .../UnifiedAlertStatesWorker.ts | 4 +- .../unified-alerting/UngroupedView.tsx | 6 +- public/app/types/unified-alerting-dto.ts | 6 + 45 files changed, 303 insertions(+), 292 deletions(-) diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts index 9c792b5cbc9..080ea10fe2f 100644 --- a/public/app/features/alerting/unified/api/alertRuleApi.ts +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -25,7 +25,7 @@ import { ExportFormats } from '../components/export/providers'; import { Folder } from '../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME, getDatasourceAPIUid, isGrafanaRulesSource } from '../utils/datasource'; import { arrayKeyValuesToObject } from '../utils/labels'; -import { isCloudRuleIdentifier, isGrafanaRulerRule, isPrometheusRuleIdentifier } from '../utils/rules'; +import { isCloudRuleIdentifier, isPrometheusRuleIdentifier, rulerRuleType } from '../utils/rules'; import { WithNotificationOptions, alertingApi } from './alertingApi'; import { GRAFANA_RULER_CONFIG } from './featureDiscoveryApi'; @@ -330,7 +330,7 @@ export const alertRuleApi = alertingApi.injectEndpoints({ }; }, invalidatesTags: (result, _error, { namespace, payload, rulerConfig }) => { - const grafanaRulerRules = payload.rules.filter((rule) => isGrafanaRulerRule(rule)); + const grafanaRulerRules = payload.rules.filter(rulerRuleType.grafana.rule); return [ { type: 'RuleNamespace', id: `${rulerConfig.dataSourceUid}/${namespace}` }, diff --git a/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx b/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx index c2c800c08e2..0424566f79d 100644 --- a/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx +++ b/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx @@ -6,7 +6,7 @@ import { RulerGrafanaRuleDTO } from 'app/types/unified-alerting-dto'; import { usePauseRuleInGroup } from '../hooks/ruleGroup/usePauseAlertRule'; import { isLoading } from '../hooks/useAsync'; import { stringifyErrorLike } from '../utils/misc'; -import { isGrafanaRulerRulePaused } from '../utils/rules'; +import { isPausedRule } from '../utils/rules'; interface Props { rule: RulerGrafanaRuleDTO; @@ -25,7 +25,7 @@ const MenuItemPauseRule = ({ rule, groupIdentifier, onPauseChange }: Props) => { const notifyApp = useAppNotification(); const [pauseRule, updateState] = usePauseRuleInGroup(); - const [icon, title] = isGrafanaRulerRulePaused(rule) + const [icon, title] = isPausedRule(rule) ? ['play' as const, 'Resume evaluation'] : ['pause' as const, 'Pause evaluation']; diff --git a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx index 1c4241fe519..d9840cd37ec 100644 --- a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx +++ b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx @@ -9,7 +9,7 @@ import { useRuleWithLocation } from '../../hooks/useCombinedRule'; import { formValuesFromExistingRule } from '../../rule-editor/formDefaults'; import { stringifyErrorLike } from '../../utils/misc'; import * as ruleId from '../../utils/rule-id'; -import { isGrafanaRulerRule } from '../../utils/rules'; +import { rulerRuleType } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; import { withPageErrorBoundary } from '../../withPageErrorBoundary'; import { AlertingPageWrapper } from '../AlertingPageWrapper'; @@ -58,7 +58,7 @@ function RuleModifyExport({ ruleIdentifier }: { ruleIdentifier: RuleIdentifier } ); } - if (rulerRule && !isGrafanaRulerRule(rulerRule.rule)) { + if (rulerRule && !rulerRuleType.grafana.rule(rulerRule.rule)) { // alert rule exists but is not a grafana-managed rule return ( { - return group.rules.some((rule) => isGrafanaRulerRule(rule) && Boolean(rule.grafana_alert.provenance) === true); + return group.rules.some( + (rule) => rulerRuleType.grafana.rule(rule) && Boolean(rule.grafana_alert.provenance) === true + ); }; const sortByLabel = (a: SelectableValue, b: SelectableValue) => { diff --git a/public/app/features/alerting/unified/components/rule-editor/RuleInspector.tsx b/public/app/features/alerting/unified/components/rule-editor/RuleInspector.tsx index d862994827b..900346c070f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/RuleInspector.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/RuleInspector.tsx @@ -14,7 +14,7 @@ import { formValuesToRulerRuleDTO, recordingRulerRuleToRuleForm, } from '../../utils/rule-form'; -import { isAlertingRulerRule, isRecordingRulerRule } from '../../utils/rules'; +import { rulerRuleType } from '../../utils/rules'; interface Props { onClose: () => void; @@ -145,9 +145,9 @@ function YamlContentInfo() { } function rulerRuleToRuleFormValues(rulerRule: RulerRuleDTO): Partial { - if (isAlertingRulerRule(rulerRule)) { + if (rulerRuleType.dataSource.alertingRule(rulerRule)) { return alertingRulerRuleToRuleForm(rulerRule); - } else if (isRecordingRulerRule(rulerRule)) { + } else if (rulerRuleType.dataSource.recordingRule(rulerRule)) { return recordingRulerRuleToRuleForm(rulerRule); } 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 e5d28d4d478..b99fcac732a 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 @@ -16,11 +16,10 @@ import { getRuleGroupLocationFromRuleWithLocation, isCloudAlertingRuleByType, isCloudRecordingRuleByType, - isCloudRulerRule, isGrafanaManagedRuleByType, - isGrafanaRulerRule, - isGrafanaRulerRulePaused, + isPausedRule, isRecordingRuleByType, + rulerRuleType, } from 'app/features/alerting/unified/utils/rules'; import { isExpressionQuery } from 'app/features/expressions/guards'; import { RuleGroupIdentifier, RuleIdentifier, RuleWithLocation } from 'app/types/unified-alerting'; @@ -209,7 +208,7 @@ export const AlertRuleForm = ({ existing, prefill, isManualRestore }: Props) => // Cloud Ruler rules identifier changes on update due to containing rule name and hash components // After successful update we need to update the URL to avoid displaying 404 errors - if (isCloudRulerRule(ruleDefinition)) { + if (rulerRuleType.dataSource.rule(ruleDefinition)) { const updatedRuleIdentifier = fromRulerRule(dataSourceName, namespaceName, groupName, ruleDefinition); locationService.replace(`/alerting/${encodeURIComponent(stringifyIdentifier(updatedRuleIdentifier))}/edit`); } @@ -295,7 +294,8 @@ export const AlertRuleForm = ({ existing, prefill, isManualRestore }: Props) => ); - const isPaused = existing && isGrafanaRulerRule(existing.rule) && isGrafanaRulerRulePaused(existing.rule); + const isPaused = rulerRuleType.grafana.alertingRule(existing?.rule) && isPausedRule(existing?.rule); + if (!type) { return null; } @@ -369,7 +369,7 @@ export const AlertRuleForm = ({ existing, prefill, isManualRestore }: Props) => function getReturnToUrl(groupId: RuleGroupIdentifier, rule: RulerRuleDTO | PostableRuleGrafanaRuleDTO) { const { dataSourceName, namespaceName, groupName } = groupId; - if (prometheusRulesPrimary && isCloudRulerRule(rule)) { + if (prometheusRulesPrimary && rulerRuleType.dataSource.rule(rule)) { const ruleIdentifier = fromRulerRule(dataSourceName, namespaceName, groupName, rule); return createViewLinkFromIdentifier(ruleIdentifier); } diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx index 71c049e661b..f6d18278089 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx @@ -20,7 +20,7 @@ import { DEFAULT_GROUP_EVALUATION_INTERVAL, getDefaultFormValues } from '../../. import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource'; import { formValuesToRulerGrafanaRuleDTO, getDefaultQueries } from '../../../utils/rule-form'; -import { isGrafanaRulerRule } from '../../../utils/rules'; +import { rulerRuleType } from '../../../utils/rules'; import { FileExportPreview } from '../../export/FileExportPreview'; import { GrafanaExportDrawer } from '../../export/GrafanaExportDrawer'; import { ExportFormats, HclExportProvider, allGrafanaExportProviders } from '../../export/providers'; @@ -150,7 +150,7 @@ export const getPayloadToExport = ( // we have to update the rule in the group in the same position if it exists, otherwise we have to add it at the end let alreadyExistsInGroup = false; const updatedRules = existingGroup.rules.map((rule: RulerRuleDTO) => { - if (isGrafanaRulerRule(rule) && rule.grafana_alert.uid === ruleUid) { + if (rulerRuleType.grafana.rule(rule) && rule.grafana_alert.uid === ruleUid) { alreadyExistsInGroup = true; return updatedRule; } else { diff --git a/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx b/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx index d8d7641b2ce..f7a1237ae9f 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx @@ -10,7 +10,7 @@ import { PromAlertingRuleState, RulerRuleDTO } from 'app/types/unified-alerting- import { AlertRuleAction, useRulerRuleAbility } from '../../hooks/useAbilities'; import { createShareLink, isLocalDevEnv, isOpenSourceEdition } from '../../utils/misc'; import * as ruleId from '../../utils/rule-id'; -import { isAlertingRule, isGrafanaRulerRule } from '../../utils/rules'; +import { prometheusRuleType, rulerRuleType } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; import { DeclareIncidentMenuItem } from '../bridges/DeclareIncidentButton'; @@ -76,7 +76,7 @@ const AlertRuleMenu = ({ // @TODO Migrate "declare incident button" to plugin links extensions const shouldShowDeclareIncidentButton = (!isOpenSourceEdition() || isLocalDevEnv()) && - isAlertingRule(promRule) && + prometheusRuleType.alertingRule(promRule) && promRule.state === PromAlertingRuleState.Firing; const shareUrl = createShareLink(identifier); @@ -86,7 +86,7 @@ const AlertRuleMenu = ({ const menuItems = ( <> - {canPause && isGrafanaRulerRule(rulerRule) && groupIdentifier.groupOrigin === 'grafana' && ( + {canPause && rulerRuleType.grafana.rule(rulerRule) && groupIdentifier.groupOrigin === 'grafana' && ( )} {canSilence && } diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx index 802be0d3e10..8653c39f707 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx @@ -343,21 +343,30 @@ describe('RuleViewer', () => { const mockRule = getVanillaPromRule({ name: 'prom test alert', namespace: mockCombinedCloudRuleNamespace({ name: 'prometheus' }, prometheus.name), - annotations: { [Annotation.summary]: 'prom summary', [Annotation.runbookURL]: 'https://runbook.example.com' }, + annotations: { + [Annotation.summary]: 'prom summary', + [Annotation.runbookURL]: 'https://runbook.example.com', + }, promRule: { - ...mockPromAlertingRule(), + ...mockPromAlertingRule({ + annotations: { + [Annotation.summary]: 'prom summary', + [Annotation.runbookURL]: 'https://runbook.example.com', + }, + }), duration: 900, // 15 minutes }, }); const mockRuleIdentifier = ruleId.fromCombinedRule(prometheus.name, mockRule); - it('should render pending period for vanilla Prometheus alert rule', async () => { + it('should render metadata for vanilla Prometheus alert rule', async () => { renderRuleViewer(mockRule, mockRuleIdentifier, ActiveTab.Details); expect(screen.getByText('prom test alert')).toBeInTheDocument(); - // One summary is rendered by the Title component, and the other by the DetailsTab component + // Both summary and runbook are rendered by the Title component, and the DetailsTab component + expect(ELEMENTS.metadata.summary(mockRule.annotations[Annotation.runbookURL]).getAll()).toHaveLength(2); expect(ELEMENTS.metadata.summary(mockRule.annotations[Annotation.summary]).getAll()).toHaveLength(2); expect(ELEMENTS.details.pendingPeriod.get()).toHaveTextContent(/15m/i); diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx index e714277c490..d661d942077 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx @@ -23,12 +23,10 @@ import { makeDashboardLink, makePanelLink, stringifyErrorLike } from '../../util import { RulePluginOrigin, getRulePluginOrigin, - isAlertingRule, isFederatedRuleGroup, - isGrafanaRecordingRule, - isGrafanaRulerRule, - isGrafanaRulerRulePaused, - isRecordingRule, + isPausedRule, + prometheusRuleType, + rulerRuleType, } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; import { AlertLabels } from '../AlertLabels'; @@ -71,11 +69,11 @@ const RuleViewer = () => { const { annotations, promRule, rulerRule } = rule; const hasError = isErrorHealth(promRule?.health); - const isAlertType = isAlertingRule(promRule); + const isAlertType = prometheusRuleType.alertingRule(promRule); const isFederatedRule = isFederatedRuleGroup(rule.group); - const isProvisioned = isGrafanaRulerRule(rulerRule) && Boolean(rulerRule.grafana_alert.provenance); - const isPaused = isGrafanaRulerRule(rulerRule) && isGrafanaRulerRulePaused(rulerRule); + const isProvisioned = rulerRuleType.grafana.rule(rulerRule) && Boolean(rulerRule.grafana_alert.provenance); + const isPaused = rulerRuleType.grafana.alertingRule(rulerRule) && isPausedRule(rulerRule); const showError = hasError && !isPaused; const ruleOrigin = rulerRule ? getRulePluginOrigin(rulerRule) : getRulePluginOrigin(promRule); @@ -126,10 +124,12 @@ const RuleViewer = () => { {activeTab === ActiveTab.Query && } {activeTab === ActiveTab.Instances && } - {activeTab === ActiveTab.History && isGrafanaRulerRule(rule.rulerRule) && } + {activeTab === ActiveTab.History && rulerRuleType.grafana.rule(rule.rulerRule) && ( + + )} {activeTab === ActiveTab.Routing && } {activeTab === ActiveTab.Details &&
} - {activeTab === ActiveTab.VersionHistory && isGrafanaRulerRule(rule.rulerRule) && ( + {activeTab === ActiveTab.VersionHistory && rulerRuleType.grafana.rule(rule.rulerRule) && ( )} @@ -206,7 +206,7 @@ const createMetadata = (rule: CombinedRule): PageInfoItem[] => { ), }); } - if (isGrafanaRecordingRule(rule.rulerRule)) { + if (rulerRuleType.grafana.recordingRule(rule.rulerRule)) { const metric = rule.rulerRule?.grafana_alert.record?.metric ?? ''; metadata.push({ label: 'Metric name', @@ -336,15 +336,15 @@ function usePageNav(rule: CombinedRule) { const { annotations, promRule, rulerRule } = rule; const summary = annotations[Annotation.summary]; - const isAlertType = isAlertingRule(promRule); + const isAlertType = prometheusRuleType.alertingRule(promRule); const numberOfInstance = isAlertType ? calculateTotalInstances(rule.instanceTotals) : undefined; const namespaceName = decodeGrafanaNamespace(rule.namespace).name; const groupName = rule.group.name; - const isGrafanaAlertRule = isGrafanaRulerRule(rulerRule) && isAlertType; - const grafanaRecordingRule = isGrafanaRecordingRule(rulerRule); - const isRecordingRuleType = isRecordingRule(promRule); + const isGrafanaAlertRule = rulerRuleType.grafana.rule(rulerRule) && isAlertType; + const grafanaRecordingRule = rulerRuleType.grafana.recordingRule(rulerRule); + const isRecordingRuleType = prometheusRuleType.recordingRule(promRule); const pageNav: NavModelItem = { ...defaultPageNav, diff --git a/public/app/features/alerting/unified/components/rule-viewer/tabs/Details.tsx b/public/app/features/alerting/unified/components/rule-viewer/tabs/Details.tsx index 218e6cf0420..8adb1023776 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/tabs/Details.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/tabs/Details.tsx @@ -7,13 +7,7 @@ import { Trans, t } from 'app/core/internationalization'; import { CombinedRule } from 'app/types/unified-alerting'; import { usePendingPeriod } from '../../../hooks/rules/usePendingPeriod'; -import { - getAnnotations, - isGrafanaAlertingRule, - isGrafanaRecordingRule, - isGrafanaRulerRule, - isRecordingRulerRule, -} from '../../../utils/rules'; +import { getAnnotations, isPausedRule, prometheusRuleType, rulerRuleType } from '../../../utils/rules'; import { isNullDate } from '../../../utils/time'; import { Tokenize } from '../../Tokenize'; import { DetailText } from '../../common/DetailText'; @@ -25,6 +19,7 @@ enum RuleType { GrafanaManagedRecordingRule = 'Grafana-managed recording rule', CloudAlertRule = 'Cloud alert rule', CloudRecordingRule = 'Cloud recording rule', + Unknown = 'Unknown', } const DetailGroup = ({ title, children }: { title: string; children: React.ReactNode }) => { @@ -45,30 +40,29 @@ interface DetailsProps { export const Details = ({ rule }: DetailsProps) => { const styles = useStyles2(getStyles); - let ruleType: RuleType; - const pendingPeriod = usePendingPeriod(rule); - if (isGrafanaRulerRule(rule.rulerRule)) { - ruleType = isGrafanaRecordingRule(rule.rulerRule) - ? RuleType.GrafanaManagedRecordingRule - : RuleType.GrafanaManagedAlertRule; - } else if (isRecordingRulerRule(rule.rulerRule)) { - ruleType = RuleType.CloudRecordingRule; - } else { - // probably not the greatest assumption - ruleType = RuleType.CloudAlertRule; + let determinedRuleType: RuleType = RuleType.Unknown; + if (rulerRuleType.grafana.alertingRule(rule.rulerRule)) { + determinedRuleType = RuleType.GrafanaManagedAlertRule; + } else if (rulerRuleType.grafana.recordingRule(rule.rulerRule)) { + determinedRuleType = RuleType.GrafanaManagedRecordingRule; + } else if (rulerRuleType.dataSource.alertingRule(rule.rulerRule)) { + determinedRuleType = RuleType.CloudAlertRule; + } else if (rulerRuleType.dataSource.recordingRule(rule.rulerRule)) { + determinedRuleType = RuleType.CloudRecordingRule; } const evaluationDuration = rule.promRule?.evaluationTime; const evaluationTimestamp = rule.promRule?.lastEvaluation; - const annotations = getAnnotations(rule); + const annotations = prometheusRuleType.alertingRule(rule.promRule) ? getAnnotations(rule.promRule) : undefined; const hasEvaluationDuration = Number.isFinite(evaluationDuration); - const updated = isGrafanaRulerRule(rule.rulerRule) ? rule.rulerRule.grafana_alert.updated : undefined; - const isPaused = isGrafanaAlertingRule(rule.rulerRule) && rule.rulerRule.grafana_alert.is_paused; + const updated = rulerRuleType.grafana.rule(rule.rulerRule) ? rule.rulerRule.grafana_alert.updated : undefined; + const isPaused = rulerRuleType.grafana.alertingRule(rule.rulerRule) && isPausedRule(rule.rulerRule); + const pausedIcon = ( @@ -82,8 +76,8 @@ export const Details = ({ rule }: DetailsProps) => { return (
- - {isGrafanaRulerRule(rule.rulerRule) && ( + + {rulerRuleType.grafana.rule(rule.rulerRule) && ( <> { )} - {isGrafanaRulerRule(rule.rulerRule) && + {rulerRuleType.grafana.rule(rule.rulerRule) && // grafana recording rules don't have these fields rule.rulerRule.grafana_alert.no_data_state && rule.rulerRule.grafana_alert.exec_err_state && ( diff --git a/public/app/features/alerting/unified/components/rule-viewer/tabs/Query.tsx b/public/app/features/alerting/unified/components/rule-viewer/tabs/Query.tsx index 7d954c53303..388dae34f94 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/tabs/Query.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/tabs/Query.tsx @@ -8,7 +8,7 @@ import { CombinedRule } from 'app/types/unified-alerting'; import { GrafanaRuleQueryViewer, QueryPreview } from '../../../GrafanaRuleQueryViewer'; import { useAlertQueriesStatus } from '../../../hooks/useAlertQueriesStatus'; import { alertRuleToQueries } from '../../../utils/query'; -import { isFederatedRuleGroup, isGrafanaRulerRule } from '../../../utils/rules'; +import { isFederatedRuleGroup, rulerRuleType } from '../../../utils/rules'; import { useAlertQueryRunner } from '../../rule-editor/query-and-alert-condition/useAlertQueryRunner'; interface Props { @@ -25,7 +25,7 @@ const QueryResults = ({ rule }: Props) => { const onRunQueries = useCallback(() => { if (queries.length > 0 && allDataSourcesAvailable) { let condition; - if (rule && isGrafanaRulerRule(rule.rulerRule)) { + if (rule && rulerRuleType.grafana.rule(rule.rulerRule)) { condition = rule.rulerRule.grafana_alert.condition; } runQueries(queries, condition ?? 'A'); @@ -46,7 +46,7 @@ const QueryResults = ({ rule }: Props) => { return ( <> - {isGrafanaRulerRule(rule.rulerRule) && !isFederatedRule && ( + {rulerRuleType.grafana.rule(rule.rulerRule) && !isFederatedRule && ( { /> )} - {!isGrafanaRulerRule(rule.rulerRule) && + {!rulerRuleType.grafana.rule(rule.rulerRule) && !isFederatedRule && queryPreviewData && Object.keys(queryPreviewData).length > 0 && ( diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index 828920a7e52..ae15d299c37 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -35,7 +35,7 @@ import { fetchRulerRulesAction, rulesInSameGroupHaveInvalidFor } from '../../sta import { checkEvaluationIntervalGlobalLimit } from '../../utils/config'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { stringifyErrorLike } from '../../utils/misc'; -import { AlertInfo, getAlertInfo, isGrafanaOrDataSourceRecordingRule } from '../../utils/rules'; +import { AlertInfo, getAlertInfo, rulerRuleType } from '../../utils/rules'; import { formatPrometheusDuration, parsePrometheusDuration, safeParsePrometheusDuration } from '../../utils/time'; import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable'; import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning'; @@ -310,9 +310,7 @@ export function EditRuleGroupModalForm(props: ModalFormProps): React.ReactElemen notifyApp.error('There are errors in the form. Correct the errors and retry.'); }; - const rulesWithoutRecordingRules = compact( - ruleGroup?.rules.filter((rule) => !isGrafanaOrDataSourceRecordingRule(rule)) - ); + const rulesWithoutRecordingRules = compact(ruleGroup?.rules.filter((rule) => !rulerRuleType.any.recordingRule(rule))); const hasSomeNoRecordingRules = rulesWithoutRecordingRules.length > 0; return ( diff --git a/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx index 5318b4a6f37..779c68197e4 100644 --- a/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx @@ -10,7 +10,6 @@ import { import cx from 'classnames'; import { produce } from 'immer'; import { useCallback, useEffect, useState } from 'react'; -import * as React from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { Badge, Button, Icon, Modal, Tooltip, useStyles2 } from '@grafana/ui'; @@ -31,12 +30,7 @@ import { SwapOperation, swapItems } from '../../reducers/ruler/ruleGroups'; import { fetchRulerRulesAction } from '../../state/actions'; import { isCloudRulesSource } from '../../utils/datasource'; import { hashRulerRule } from '../../utils/rule-id'; -import { - isAlertingRulerRule, - isGrafanaRulerRule, - isRecordingRulerRule, - rulesSourceToDataSourceName, -} from '../../utils/rules'; +import { getRuleName, rulerRuleType, rulesSourceToDataSourceName } from '../../utils/rules'; interface ModalProps { namespace: CombinedRuleNamespace; @@ -189,14 +183,16 @@ const ListItem = ({ provided, rule, isClone = false, isDragging = false }: ListI {...provided.draggableProps} {...provided.dragHandleProps} > - {isGrafanaRulerRule(rule) &&
{rule.grafana_alert.title}
} - {isRecordingRulerRule(rule) && ( - <> -
{rule.record}
- - - )} - {isAlertingRulerRule(rule) &&
{rule.alert}
} +
+ {getRuleName(rule)} + {rulerRuleType.any.recordingRule(rule) && ( + <> + {' '} + + + )} +
+ {rulerRuleType.dataSource.alertingRule(rule) &&
{rule.alert}
}
); diff --git a/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx b/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx index 9f7d4710520..8ff803ac421 100644 --- a/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleActionsButtons.tsx @@ -16,7 +16,7 @@ import { GRAFANA_RULES_SOURCE_NAME, getRulesSourceName } from '../../utils/datas import { groupIdentifier } from '../../utils/groupIdentifier'; import { createViewLink } from '../../utils/misc'; import * as ruleId from '../../utils/rule-id'; -import { isGrafanaAlertingRule, isGrafanaRulerRule } from '../../utils/rules'; +import { rulerRuleType } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; import { RedirectToCloneRule } from './CloneRule'; @@ -52,7 +52,7 @@ export const RuleActionsButtons = ({ compact, showViewButton, rule, rulesSource const { namespace, group, rulerRule } = rule; const { hasActiveFilters } = useRulesFilter(); - const isProvisioned = isGrafanaRulerRule(rule.rulerRule) && Boolean(rule.rulerRule.grafana_alert.provenance); + const isProvisioned = rulerRuleType.grafana.rule(rule.rulerRule) && Boolean(rule.rulerRule.grafana_alert.provenance); const [editRuleSupported, editRuleAllowed] = useAlertRuleAbility(rule, AlertRuleAction.Update); @@ -125,7 +125,7 @@ export const RuleActionsButtons = ({ compact, showViewButton, rule, rulesSource buttonSize={buttonSize} /> {deleteModal} - {isGrafanaAlertingRule(rule.rulerRule) && showSilenceDrawer && ( + {rulerRuleType.grafana.alertingRule(rule.rulerRule) && showSilenceDrawer && ( setShowSilenceDrawer(false)} /> )} {redirectToClone?.identifier && ( diff --git a/public/app/features/alerting/unified/components/rules/RuleConfigStatus.tsx b/public/app/features/alerting/unified/components/rules/RuleConfigStatus.tsx index 2309f946e78..c0ed270d66f 100644 --- a/public/app/features/alerting/unified/components/rules/RuleConfigStatus.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleConfigStatus.tsx @@ -7,7 +7,7 @@ import { Icon, Tooltip, useStyles2 } from '@grafana/ui/src'; import { CombinedRule } from '../../../../../types/unified-alerting'; import { checkEvaluationIntervalGlobalLimit } from '../../utils/config'; -import { isGrafanaRulerRule } from '../../utils/rules'; +import { rulerRuleType } from '../../utils/rules'; interface RuleConfigStatusProps { rule: CombinedRule; @@ -15,7 +15,7 @@ interface RuleConfigStatusProps { export function RuleConfigStatus({ rule }: RuleConfigStatusProps) { const styles = useStyles2(getStyles); - const isGrafanaManagedRule = isGrafanaRulerRule(rule.rulerRule); + const isGrafanaManagedRule = rulerRuleType.grafana.rule(rule.rulerRule); const exceedsLimit = useMemo(() => { return isGrafanaManagedRule ? checkEvaluationIntervalGlobalLimit(rule.group.interval).exceedsLimit : false; diff --git a/public/app/features/alerting/unified/components/rules/RuleDetails.tsx b/public/app/features/alerting/unified/components/rules/RuleDetails.tsx index ab593b12b1c..2e5394fc02f 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetails.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetails.tsx @@ -7,7 +7,7 @@ import { CombinedRule } from 'app/types/unified-alerting'; import { usePendingPeriod } from '../../hooks/rules/usePendingPeriod'; import { useCleanAnnotations } from '../../utils/annotations'; -import { isGrafanaRecordingRule, isRecordingRule, isRecordingRulerRule } from '../../utils/rules'; +import { prometheusRuleType, rulerRuleType } from '../../utils/rules'; import { isNullDate } from '../../utils/time'; import { AlertLabels } from '../AlertLabels'; import { DetailsField } from '../DetailsField'; @@ -53,15 +53,12 @@ export const RuleDetails = ({ rule }: Props) => { - {!( - isRecordingRulerRule(rule.rulerRule) || - isRecordingRule(rule.promRule) || - isGrafanaRecordingRule(rule.rulerRule) - ) && ( - - - - )} + {rulerRuleType.any.alertingRule(rule.rulerRule) || + (prometheusRuleType.alertingRule(rule.promRule) && ( + + + + ))} ); }; @@ -74,7 +71,9 @@ const EvaluationBehaviorSummary = ({ rule }: EvaluationBehaviorSummaryProps) => const every = rule.group.interval; const lastEvaluation = rule.promRule?.lastEvaluation; const lastEvaluationDuration = rule.promRule?.evaluationTime; - const metric = isGrafanaRecordingRule(rule.rulerRule) ? rule.rulerRule?.grafana_alert.record?.metric : undefined; + const metric = rulerRuleType.grafana.recordingRule(rule.rulerRule) + ? rule.rulerRule?.grafana_alert.record?.metric + : undefined; const pendingPeriod = usePendingPeriod(rule); diff --git a/public/app/features/alerting/unified/components/rules/RuleDetailsButtons.tsx b/public/app/features/alerting/unified/components/rules/RuleDetailsButtons.tsx index 079a26dcf48..6f19a0ab8a6 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetailsButtons.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetailsButtons.tsx @@ -10,7 +10,7 @@ import { useStateHistoryModal } from '../../hooks/useStateHistoryModal'; import { Annotation } from '../../utils/constants'; import { isCloudRulesSource } from '../../utils/datasource'; import { createExploreLink } from '../../utils/misc'; -import { isFederatedRuleGroup, isGrafanaAlertingRule, isGrafanaRulerRule } from '../../utils/rules'; +import { isFederatedRuleGroup, rulerRuleType } from '../../utils/rules'; interface Props { rule: CombinedRule; @@ -103,13 +103,13 @@ const RuleDetailsButtons = ({ rule, rulesSource }: Props) => { } } - if (isGrafanaAlertingRule(rule.rulerRule)) { + if (rulerRuleType.grafana.alertingRule(rule.rulerRule)) { buttons.push( diff --git a/public/app/features/alerting/unified/components/rules/RuleDetailsDataSources.tsx b/public/app/features/alerting/unified/components/rules/RuleDetailsDataSources.tsx index 9f6ea6c1dcc..9f18a713b18 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetailsDataSources.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetailsDataSources.tsx @@ -8,7 +8,7 @@ import { ExpressionDatasourceUID } from 'app/features/expressions/types'; import { CombinedRule, RulesSource } from 'app/types/unified-alerting'; import { isCloudRulesSource } from '../../utils/datasource'; -import { isGrafanaRulerRule } from '../../utils/rules'; +import { rulerRuleType } from '../../utils/rules'; import { DetailsField } from '../DetailsField'; type Props = { @@ -25,7 +25,7 @@ export function RuleDetailsDataSources(props: Props): JSX.Element | null { return [{ name: rulesSource.name, icon: rulesSource.meta.info.logos.small }]; } - if (isGrafanaRulerRule(rule.rulerRule)) { + if (rulerRuleType.grafana.rule(rule.rulerRule)) { const { data } = rule.rulerRule.grafana_alert; const unique = data.reduce>((dataSources, query) => { const ds = getDataSourceSrv().getInstanceSettings(query.datasourceUid); diff --git a/public/app/features/alerting/unified/components/rules/RuleDetailsMatchingInstances.tsx b/public/app/features/alerting/unified/components/rules/RuleDetailsMatchingInstances.tsx index c9491eda390..c45c3cdee34 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetailsMatchingInstances.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetailsMatchingInstances.tsx @@ -1,7 +1,7 @@ import { css, cx } from '@emotion/css'; import { countBy, sum } from 'lodash'; -import { useMemo, useState } from 'react'; import * as React from 'react'; +import { useMemo, useState } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { LinkButton, useStyles2 } from '@grafana/ui'; @@ -18,7 +18,7 @@ import { mapStateWithReasonToBaseState } from 'app/types/unified-alerting-dto'; import { GRAFANA_RULES_SOURCE_NAME, isGrafanaRulesSource } from '../../utils/datasource'; import { parsePromQLStyleMatcherLooseSafe } from '../../utils/matchers'; -import { isAlertingRule } from '../../utils/rules'; +import { prometheusRuleType } from '../../utils/rules'; import { AlertInstancesTable } from './AlertInstancesTable'; import { getComponentsFromStats } from './RuleStats'; @@ -73,13 +73,13 @@ export function RuleDetailsMatchingInstances(props: Props) { const alerts = useMemo( (): Alert[] => - isAlertingRule(promRule) && promRule.alerts?.length + prometheusRuleType.alertingRule(promRule) && promRule.alerts?.length ? filterAlerts(queryString, alertState, sortAlerts(SortOrder.Importance, promRule.alerts)) : [], [promRule, alertState, queryString] ); - if (!isAlertingRule(promRule)) { + if (!prometheusRuleType.alertingRule(promRule)) { return null; } diff --git a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx index 0bd026d6d43..3295e2950e6 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx @@ -14,7 +14,7 @@ import { GRAFANA_RULES_SOURCE_NAME, getRulesDataSources } from '../../utils/data import { createViewLink } from '../../utils/misc'; import { isAsyncRequestStatePending } from '../../utils/redux'; import { hashRule } from '../../utils/rule-id'; -import { getRulePluginOrigin, isAlertingRule, isProvisionedRule } from '../../utils/rules'; +import { getRulePluginOrigin, isProvisionedRule, prometheusRuleType } from '../../utils/rules'; import { calculateTotalInstances } from '../rule-viewer/RuleViewer'; import { RuleActionsButtons } from './RuleActionsButtons'; @@ -43,7 +43,7 @@ export const RuleListStateView = ({ namespaces }: Props) => { // We might hit edge cases where there type = alerting, but there is no state. // In this case, we shouldn't try to group these alerts in the state view // Even though we handle this at the API layer, this is a last catch point for any edge cases - if (rule.promRule && isAlertingRule(rule.promRule) && rule.promRule.state) { + if (prometheusRuleType.alertingRule(rule.promRule) && rule.promRule.state) { result.get(rule.promRule.state)?.push(rule); } }) @@ -103,7 +103,9 @@ const RulesByState = ({ state, rules }: { state: PromAlertingRuleState; rules: C const { rulerRule, promRule } = rule; const isProvisioned = rulerRule ? isProvisionedRule(rulerRule) : false; - const instancesCount = isAlertingRule(rule.promRule) ? calculateTotalInstances(rule.instanceTotals) : undefined; + const instancesCount = prometheusRuleType.alertingRule(rule.promRule) + ? calculateTotalInstances(rule.instanceTotals) + : undefined; if (!promRule) { return null; diff --git a/public/app/features/alerting/unified/components/rules/RuleState.tsx b/public/app/features/alerting/unified/components/rules/RuleState.tsx index 48912322e54..78637dac2e1 100644 --- a/public/app/features/alerting/unified/components/rules/RuleState.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleState.tsx @@ -7,7 +7,7 @@ import { Trans } from 'app/core/internationalization'; import { CombinedRule } from 'app/types/unified-alerting'; import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; -import { getFirstActiveAt, isAlertingRule, isGrafanaRecordingRule, isRecordingRule } from '../../utils/rules'; +import { getFirstActiveAt, prometheusRuleType, rulerRuleType } from '../../utils/rules'; import { StateTag } from '../StateTag'; import { AlertStateTag } from './AlertStateTag'; @@ -24,7 +24,7 @@ export const RuleState = ({ rule, isDeleting, isCreating, isPaused }: Props) => const { promRule, rulerRule } = rule; // return how long the rule has been in its firing state, if any const RecordingRuleState = () => { - if (isPaused && isGrafanaRecordingRule(rulerRule)) { + if (isPaused && rulerRuleType.grafana.recordingRule(rulerRule)) { return ( @@ -39,8 +39,7 @@ export const RuleState = ({ rule, isDeleting, isCreating, isPaused }: Props) => }; const forTime = useMemo(() => { if ( - promRule && - isAlertingRule(promRule) && + prometheusRuleType.alertingRule(promRule) && promRule.alerts?.length && promRule.state !== PromAlertingRuleState.Inactive ) { @@ -80,14 +79,14 @@ export const RuleState = ({ rule, isDeleting, isCreating, isPaused }: Props) => Creating ); - } else if (promRule && isAlertingRule(promRule)) { + } else if (prometheusRuleType.alertingRule(promRule)) { return ( {!isPaused && forTime} ); - } else if (promRule && isRecordingRule(promRule)) { + } else if (promRule && prometheusRuleType.recordingRule(promRule)) { return ; } return <>n/a; diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx index ccf725b5f99..7cde4ee36c4 100644 --- a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx @@ -15,7 +15,7 @@ import { useHasRuler } from '../../hooks/useHasRuler'; import { useRulesAccess } from '../../utils/accessControlHooks'; import { GRAFANA_RULES_SOURCE_NAME, getRulesSourceName, isCloudRulesSource } from '../../utils/datasource'; import { makeFolderLink, makeFolderSettingsLink } from '../../utils/misc'; -import { isFederatedRuleGroup, isGrafanaRulerRule } from '../../utils/rules'; +import { isFederatedRuleGroup, rulerRuleType } from '../../utils/rules'; import { CollapseToggle } from '../CollapseToggle'; import { RuleLocation } from '../RuleLocation'; import { GrafanaRuleFolderExporter } from '../export/GrafanaRuleFolderExporter'; @@ -63,7 +63,8 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }: const { currentData: dsFeatures } = useDiscoverDsFeaturesQuery({ rulesSourceName }); const rulerRule = group.rules[0]?.rulerRule; - const folderUID = (rulerRule && isGrafanaRulerRule(rulerRule) && rulerRule.grafana_alert.namespace_uid) || undefined; + const folderUID = + (rulerRule && rulerRuleType.grafana.rule(rulerRule) && rulerRule.grafana_alert.namespace_uid) || undefined; const { folder } = useFolder(folderUID); // group "is deleting" if rules source has ruler, but this group has no rules that are in ruler @@ -72,7 +73,7 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }: // check if group has provisioned items const isProvisioned = group.rules.some((rule) => { - return isGrafanaRulerRule(rule.rulerRule) && rule.rulerRule.grafana_alert.provenance; + return rulerRuleType.grafana.rule(rule.rulerRule) && rule.rulerRule.grafana_alert.provenance; }); // check what view mode we are in diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx index 1f7c6c44b11..b5a0aeabc62 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx @@ -19,7 +19,7 @@ import { PluginOriginBadge } from '../../plugins/PluginOriginBadge'; import { calculateNextEvaluationEstimate } from '../../rule-list/components/util'; import { Annotation } from '../../utils/constants'; import { GRAFANA_RULES_SOURCE_NAME, getRulesSourceName } from '../../utils/datasource'; -import { getRulePluginOrigin, isGrafanaRulerRule, isGrafanaRulerRulePaused } from '../../utils/rules'; +import { getRulePluginOrigin, isPausedRule, rulerRuleType } from '../../utils/rules'; import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable'; import { DynamicTableWithGuidelines } from '../DynamicTableWithGuidelines'; import { ProvisioningBadge } from '../Provisioning'; @@ -219,7 +219,7 @@ function useColumns( return ; } - const isGrafanaManagedRule = isGrafanaRulerRule(rulerRule); + const isGrafanaManagedRule = rulerRuleType.grafana.rule(rulerRule); if (!isGrafanaManagedRule) { return null; } @@ -347,13 +347,13 @@ function useRuleStatus(rule: CombinedRule) { // If prometheusRulesPrimary is enabled, we don't fetch rules from the Ruler API (except for Grafana managed rules) // so there is no way to detect statuses - if (prometheusRulesPrimary && !isGrafanaRulerRule(rulerRule)) { + if (prometheusRulesPrimary && !rulerRuleType.grafana.rule(rulerRule)) { return { isDeleting: false, isCreating: false, isPaused: false }; } const isDeleting = Boolean(hasRuler && rulerRulesLoaded && promRule && !rulerRule); const isCreating = Boolean(hasRuler && rulerRulesLoaded && rulerRule && !promRule); - const isPaused = isGrafanaRulerRule(rulerRule) && isGrafanaRulerRulePaused(rulerRule); + const isPaused = rulerRuleType.grafana.alertingRule(rulerRule) && isPausedRule(rulerRule); return { isDeleting, isCreating, isPaused }; } diff --git a/public/app/features/alerting/unified/hooks/ruleGroup/useUpsertRuleFromRuleGroup.ts b/public/app/features/alerting/unified/hooks/ruleGroup/useUpsertRuleFromRuleGroup.ts index 7d739837955..d3a68a4c095 100644 --- a/public/app/features/alerting/unified/hooks/ruleGroup/useUpsertRuleFromRuleGroup.ts +++ b/public/app/features/alerting/unified/hooks/ruleGroup/useUpsertRuleFromRuleGroup.ts @@ -7,7 +7,7 @@ import { PostableRuleDTO } from 'app/types/unified-alerting-dto'; import { alertRuleApi } from '../../api/alertRuleApi'; import { addRuleAction, updateRuleAction } from '../../reducers/ruler/ruleGroups'; -import { isGrafanaRuleIdentifier, isGrafanaRulerRule } from '../../utils/rules'; +import { isGrafanaRuleIdentifier, rulerRuleType } from '../../utils/rules'; import { useAsync } from '../useAsync'; import { useDeleteRuleFromGroup } from './useDeleteRuleFromGroup'; @@ -132,7 +132,7 @@ function copyGrafanaUID(ruleIdentifier: EditableRuleIdentifier, ruleDefinition: // by copying over the rule UID the backend will perform an atomic move operation // so there is no need for us to manually remove it from the previous group return produce(ruleDefinition, (draft) => { - const isGrafanaManagedRuleDefinition = isGrafanaRulerRule(draft); + const isGrafanaManagedRuleDefinition = rulerRuleType.grafana.rule(draft); if (isGrafanaManagedRuleIdentifier && isGrafanaManagedRuleDefinition) { draft.grafana_alert.uid = ruleIdentifier.uid; diff --git a/public/app/features/alerting/unified/hooks/useAbilities.ts b/public/app/features/alerting/unified/hooks/useAbilities.ts index 8bace1f93e9..098913b2375 100644 --- a/public/app/features/alerting/unified/hooks/useAbilities.ts +++ b/public/app/features/alerting/unified/hooks/useAbilities.ts @@ -22,7 +22,7 @@ import { getInstancesPermissions, getNotificationsPermissions, getRulesPermissio import { getRulesSourceName } from '../utils/datasource'; import { getGroupOriginName } from '../utils/groupIdentifier'; import { isAdmin } from '../utils/misc'; -import { isFederatedRuleGroup, isGrafanaRecordingRule, isGrafanaRulerRule, isPluginProvidedRule } from '../utils/rules'; +import { isFederatedRuleGroup, isPluginProvidedRule, rulerRuleType } from '../utils/rules'; import { useIsRuleEditable } from './useIsRuleEditable'; @@ -206,9 +206,10 @@ export function useAllAlertRuleAbilities(rule: CombinedRule): Abilities>(() => { - const isProvisioned = isGrafanaRulerRule(rule.rulerRule) && Boolean(rule.rulerRule.grafana_alert.provenance); + const isProvisioned = + rulerRuleType.grafana.rule(rule.rulerRule) && Boolean(rule.rulerRule.grafana_alert.provenance); const isFederated = isFederatedRuleGroup(rule.group); - const isGrafanaManagedAlertRule = isGrafanaRulerRule(rule.rulerRule); + const isGrafanaManagedAlertRule = rulerRuleType.grafana.rule(rule.rulerRule); const isPluginProvided = isPluginProvidedRule(rule.rulerRule); // if a rule is either provisioned, federated or provided by a plugin rule, we don't allow it to be removed or edited @@ -252,10 +253,10 @@ export function useAllRulerRuleAbilities( const canSilence = useCanSilence(rule); const abilities = useMemo>(() => { - const isProvisioned = isGrafanaRulerRule(rule) && Boolean(rule.grafana_alert.provenance); + const isProvisioned = rulerRuleType.grafana.rule(rule) && Boolean(rule.grafana_alert.provenance); // const isFederated = isFederatedRuleGroup(); const isFederated = false; - const isGrafanaManagedAlertRule = isGrafanaRulerRule(rule); + const isGrafanaManagedAlertRule = rulerRuleType.grafana.rule(rule); const isPluginProvided = isPluginProvidedRule(rule); // if a rule is either provisioned, federated or provided by a plugin rule, we don't allow it to be removed or edited @@ -441,11 +442,11 @@ const { useGetGrafanaAlertingConfigurationStatusQuery } = alertmanagerApi; * 2. the admin has configured to only send instances to external AMs */ function useCanSilence(rule?: RulerRuleDTO): [boolean, boolean] { - const folderUID = isGrafanaRulerRule(rule) ? rule.grafana_alert.namespace_uid : undefined; + const folderUID = rulerRuleType.grafana.rule(rule) ? rule.grafana_alert.namespace_uid : undefined; const { loading: folderIsLoading, folder } = useFolder(folderUID); - const isGrafanaManagedRule = rule && isGrafanaRulerRule(rule); - const isGrafanaRecording = rule && isGrafanaRecordingRule(rule); + const isGrafanaManagedRule = rule && rulerRuleType.grafana.rule(rule); + const isGrafanaRecording = rulerRuleType.grafana.recordingRule(rule); const silenceSupported = useGrafanaRulesSilenceSupport(); const canSilenceInFolder = useCanSilenceInFolder(folderUID); diff --git a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts index f28bc644ecc..6350144a583 100644 --- a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts +++ b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts @@ -32,13 +32,7 @@ import { isGrafanaRulesSource, } from '../utils/datasource'; import { hashQuery } from '../utils/rule-id'; -import { - isAlertingRule, - isAlertingRulerRule, - isGrafanaRulerRule, - isRecordingRule, - isRecordingRulerRule, -} from '../utils/rules'; +import { getAnnotations, isPausedRule, prometheusRuleType, rulerRuleType } from '../utils/rules'; import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector'; @@ -98,7 +92,7 @@ export function useCombinedRuleNamespaces( // We need to set the namespace_uid for grafana rules as it's required to obtain the rule's groups // All rules from all groups have the same namespace_uid so we're taking the first one. - if (isGrafanaRulerRule(groups[0].rules[0])) { + if (rulerRuleType.grafana.rule(groups[0].rules[0])) { namespace.uid = groups[0].rules[0].grafana_alert.namespace_uid; } @@ -247,8 +241,10 @@ export function addRulerGroupsToCombinedNamespace( groups: RulerRuleGroupDTO[] = [] ): void { namespace.groups = groups.map((group) => { - const numRecordingRules = group.rules.filter((rule) => isRecordingRulerRule(rule)).length; - const numPaused = group.rules.filter((rule) => isGrafanaRulerRule(rule) && rule.grafana_alert.is_paused).length; + const numRecordingRules = group.rules.filter((rule) => rulerRuleType.any.recordingRule(rule)).length; + const numPaused = group.rules.filter((rule) => { + return rulerRuleType.grafana.alertingRule(rule) && isPausedRule(rule); + }).length; const combinedGroup: CombinedRuleGroup = { name: group.name, @@ -298,8 +294,10 @@ export function addPromGroupsToCombinedNamespace(namespace: CombinedRuleNamespac const existingRule = getExistingRuleInGroup(rule, combinedRulesByName, namespace.rulesSource); if (existingRule) { existingRule.promRule = rule; - existingRule.instanceTotals = isAlertingRule(rule) ? calculateRuleTotals(rule) : {}; - existingRule.filteredInstanceTotals = isAlertingRule(rule) ? calculateRuleFilteredTotals(rule) : {}; + existingRule.instanceTotals = prometheusRuleType.alertingRule(rule) ? calculateRuleTotals(rule) : {}; + existingRule.filteredInstanceTotals = prometheusRuleType.alertingRule(rule) + ? calculateRuleFilteredTotals(rule) + : {}; } else { combinedGroup!.rules.push(promRuleToCombinedRule(rule, namespace, combinedGroup!)); } @@ -344,9 +342,9 @@ export function calculateGroupTotals(group: Pick) }; } - const countsByState = countBy(group.rules, (rule) => isAlertingRule(rule) && rule.state); + const countsByState = countBy(group.rules, (rule) => prometheusRuleType.alertingRule(rule) && rule.state); const countsByHealth = countBy(group.rules, (rule) => rule.health); - const recordingCount = group.rules.filter((rule) => isRecordingRule(rule)).length; + const recordingCount = group.rules.filter((rule) => prometheusRuleType.recordingRule(rule)).length; return { alerting: countsByState[PromAlertingRuleState.Firing], @@ -382,12 +380,12 @@ function promRuleToCombinedRule(rule: Rule, namespace: CombinedRuleNamespace, gr name: rule.name, query: rule.query, labels: rule.labels || {}, - annotations: isAlertingRule(rule) ? rule.annotations || {} : {}, + annotations: prometheusRuleType.alertingRule(rule) ? getAnnotations(rule) : {}, promRule: rule, namespace: namespace, group, - instanceTotals: isAlertingRule(rule) ? calculateRuleTotals(rule) : {}, - filteredInstanceTotals: isAlertingRule(rule) ? calculateRuleFilteredTotals(rule) : {}, + instanceTotals: prometheusRuleType.alertingRule(rule) ? calculateRuleTotals(rule) : {}, + filteredInstanceTotals: prometheusRuleType.alertingRule(rule) ? calculateRuleFilteredTotals(rule) : {}, }; } @@ -396,7 +394,7 @@ function rulerRuleToCombinedRule( namespace: CombinedRuleNamespace, group: CombinedRuleGroup ): CombinedRule { - return isAlertingRulerRule(rule) + return rulerRuleType.dataSource.alertingRule(rule) ? { name: rule.alert, query: rule.expr, @@ -408,7 +406,7 @@ function rulerRuleToCombinedRule( instanceTotals: {}, filteredInstanceTotals: {}, } - : isRecordingRulerRule(rule) + : rulerRuleType.dataSource.recordingRule(rule) ? { name: rule.record, query: rule.expr, @@ -473,10 +471,18 @@ function getExistingRuleInGroup( } function isCombinedRuleEqualToPromRule(combinedRule: CombinedRule, rule: Rule, checkQuery = true): boolean { + const promRuleAnnotations = prometheusRuleType.alertingRule(rule) ? getAnnotations(rule) : {}; + const promRuleLabels = rule.labels ?? {}; + const promQuery = checkQuery ? hashQuery(rule.query) : ''; + + const combinedRuleAnnotations = combinedRule.annotations; + const combinedRuleLabels = combinedRule.labels; + const combinedRuleQuery = checkQuery ? hashQuery(combinedRule.query) : ''; + if (combinedRule.name === rule.name) { return isEqual( - [checkQuery ? hashQuery(combinedRule.query) : '', combinedRule.labels, combinedRule.annotations], - [checkQuery ? hashQuery(rule.query) : '', rule.labels || {}, isAlertingRule(rule) ? rule.annotations || {} : {}] + [combinedRuleQuery, combinedRuleLabels, combinedRuleAnnotations], + [promQuery, promRuleLabels, promRuleAnnotations] ); } return false; @@ -554,7 +560,7 @@ export function useCombinedRules( // We need to set the namespace_uid for grafana rules as it's required to obtain the rule's groups // All rules from all groups have the same namespace_uid so we're taking the first one. - if (isGrafanaRulerRule(groups[0].rules[0])) { + if (rulerRuleType.grafana.rule(groups[0].rules[0])) { namespace.uid = groups[0].rules[0].grafana_alert.namespace_uid; } diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.ts index e4d720cc7cb..c8123de32c4 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.ts @@ -14,13 +14,7 @@ import { labelsMatchMatchers, matcherToMatcherField } from '../utils/alertmanage import { Annotation } from '../utils/constants'; import { isCloudRulesSource } from '../utils/datasource'; import { parseMatcher, parsePromQLStyleMatcherLoose } from '../utils/matchers'; -import { - getRuleHealth, - isAlertingRule, - isGrafanaRulerRule, - isPluginProvidedRule, - isPromRuleType, -} from '../utils/rules'; +import { getRuleHealth, isPluginProvidedRule, isPromRuleType, prometheusRuleType, rulerRuleType } from '../utils/rules'; import { calculateGroupTotals, calculateRuleFilteredTotals, calculateRuleTotals } from './useCombinedRuleNamespaces'; import { useURLSearchParams } from './useURLSearchParams'; @@ -106,7 +100,7 @@ export const useFilteredRules = (namespaces: CombinedRuleNamespace[], filterStat filteredRules.forEach((namespace) => { namespace.groups.forEach((group) => { group.rules.forEach((rule) => { - if (isAlertingRule(rule.promRule)) { + if (prometheusRuleType.alertingRule(rule.promRule)) { rule.instanceTotals = calculateRuleTotals(rule.promRule); rule.filteredInstanceTotals = calculateRuleFilteredTotals(rule.promRule); } @@ -225,7 +219,7 @@ const reduceGroups = (filterState: RulesFilter) => { if ('contactPoint' in matchesFilterFor) { const contactPoint = filterState.contactPoint; const hasContactPoint = - isGrafanaRulerRule(rule.rulerRule) && + rulerRuleType.grafana.rule(rule.rulerRule) && rule.rulerRule.grafana_alert.notification_settings?.receiver === contactPoint; if (hasContactPoint) { @@ -234,7 +228,7 @@ const reduceGroups = (filterState: RulesFilter) => { } if ('dataSourceNames' in matchesFilterFor) { - if (isGrafanaRulerRule(rule.rulerRule)) { + if (rulerRuleType.grafana.rule(rule.rulerRule)) { const doesNotQueryDs = isQueryingDataSource(rule.rulerRule, filterState); if (doesNotQueryDs) { @@ -274,7 +268,7 @@ const reduceGroups = (filterState: RulesFilter) => { if ('ruleState' in matchesFilterFor) { const promRule = rule.promRule; - const hasPromRuleDefinition = promRule && isAlertingRule(promRule); + const hasPromRuleDefinition = promRule && prometheusRuleType.alertingRule(promRule); const ruleStateMatches = hasPromRuleDefinition && promRule.state === filterState.ruleState; diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts index 017e008c02c..59211520d47 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts @@ -3,7 +3,7 @@ import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { featureDiscoveryApi } from '../api/featureDiscoveryApi'; import { getRulesPermissions } from '../utils/access-control'; -import { isGrafanaRulerRule } from '../utils/rules'; +import { rulerRuleType } from '../utils/rules'; import { useFolder } from './useFolder'; @@ -24,7 +24,7 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): rulesSourceName, }); - const folderUID = rule && isGrafanaRulerRule(rule) ? rule.grafana_alert.namespace_uid : undefined; + const folderUID = rule && rulerRuleType.grafana.rule(rule) ? rule.grafana_alert.namespace_uid : undefined; const rulePermission = getRulesPermissions(rulesSourceName); const { folder, loading } = useFolder(folderUID); @@ -47,7 +47,7 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): // Grafana rules can be edited if user can edit the folder they're in // When RBAC is disabled access to a folder is the only requirement for managing rules // When RBAC is enabled the appropriate alerting permissions need to be met - if (isGrafanaRulerRule(rule)) { + if (rulerRuleType.grafana.rule(rule)) { if (!folderUID) { throw new Error( `Rule ${rule.grafana_alert.title} does not have a folder uid, cannot determine if it is editable.` diff --git a/public/app/features/alerting/unified/reducers/ruler/ruleGroups.ts b/public/app/features/alerting/unified/reducers/ruler/ruleGroups.ts index 41d7c42c889..b8d91042b2c 100644 --- a/public/app/features/alerting/unified/reducers/ruler/ruleGroups.ts +++ b/public/app/features/alerting/unified/reducers/ruler/ruleGroups.ts @@ -6,12 +6,7 @@ import { PostableRuleDTO, PostableRulerRuleGroupDTO } from 'app/types/unified-al import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { hashRulerRule } from '../../utils/rule-id'; -import { - isCloudRuleIdentifier, - isCloudRulerRule, - isGrafanaRuleIdentifier, - isGrafanaRulerRule, -} from '../../utils/rules'; +import { isCloudRuleIdentifier, isGrafanaRuleIdentifier, rulerRuleType } from '../../utils/rules'; // rule-scoped actions export const addRuleAction = createAction<{ rule: PostableRuleDTO; groupName?: string; interval?: string }>( @@ -61,7 +56,7 @@ export const ruleGroupReducer = createReducer(initialState, (builder) => { const index = findRuleIndex(draft.rules, identifier); const matchingRule = draft.rules[index]; - if (isGrafanaRulerRule(matchingRule)) { + if (rulerRuleType.grafana.rule(matchingRule)) { matchingRule.grafana_alert.is_paused = pause; } else { throw new Error('Matching rule is not a Grafana-managed rule'); @@ -98,8 +93,8 @@ const ruleFinder = (identifier: RuleIdentifier) => { const dataSourceManagedIdentifier = isCloudRuleIdentifier(identifier); return (rule: PostableRuleDTO) => { - const isGrafanaManagedRule = isGrafanaRulerRule(rule); - const isDataSourceManagedRule = isCloudRulerRule(rule); + const isGrafanaManagedRule = rulerRuleType.grafana.rule(rule); + const isDataSourceManagedRule = rulerRuleType.dataSource.rule(rule); if (grafanaManagedIdentifier && isGrafanaManagedRule) { return rule.grafana_alert.uid === identifier.uid; diff --git a/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx index e92ad4b46e6..c8da82711dc 100644 --- a/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx +++ b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx @@ -10,7 +10,7 @@ import { useRuleWithLocation } from '../hooks/useCombinedRule'; import { generateCopiedName } from '../utils/duplicate'; import { stringifyErrorLike } from '../utils/misc'; import { rulerRuleToFormValues } from '../utils/rule-form'; -import { getRuleName, isAlertingRulerRule, isGrafanaRulerRule, isRecordingRulerRule } from '../utils/rules'; +import { getRuleName, rulerRuleType } from '../utils/rules'; import { createRelativeUrl } from '../utils/url'; export function CloneRuleEditor({ sourceRuleId }: { sourceRuleId: RuleIdentifier }) { @@ -45,14 +45,14 @@ export function CloneRuleEditor({ sourceRuleId }: { sourceRuleId: RuleIdentifier } function changeRuleName(rule: RulerRuleDTO, newName: string) { - if (isGrafanaRulerRule(rule)) { + if (rulerRuleType.grafana.rule(rule)) { rule.grafana_alert.title = newName; } - if (isAlertingRulerRule(rule)) { + if (rulerRuleType.dataSource.alertingRule(rule)) { rule.alert = newName; } - if (isRecordingRulerRule(rule)) { + if (rulerRuleType.dataSource.recordingRule(rule)) { rule.record = newName; } } @@ -64,7 +64,7 @@ export function cloneRuleDefinition(rule: RuleWithLocation) { generateCopiedName(getRuleName(ruleClone.rule), ruleClone.group.rules.map(getRuleName)) ); - if (isGrafanaRulerRule(ruleClone.rule)) { + if (rulerRuleType.grafana.rule(ruleClone.rule)) { ruleClone.rule.grafana_alert.uid = ''; // Provisioned alert rules have provisioned alert group which cannot be used in UI diff --git a/public/app/features/alerting/unified/rule-list/DataSourceRuleLoader.tsx b/public/app/features/alerting/unified/rule-list/DataSourceRuleLoader.tsx index d041b42566d..95edc6db6f6 100644 --- a/public/app/features/alerting/unified/rule-list/DataSourceRuleLoader.tsx +++ b/public/app/features/alerting/unified/rule-list/DataSourceRuleLoader.tsx @@ -5,7 +5,7 @@ import { DataSourceRuleGroupIdentifier, Rule, RuleIdentifier } from 'app/types/u import { alertRuleApi } from '../api/alertRuleApi'; import { featureDiscoveryApi } from '../api/featureDiscoveryApi'; import { equal, fromRule, fromRulerRule, stringifyIdentifier } from '../utils/rule-id'; -import { getRulePluginOrigin, isAlertingRule, isRecordingRule } from '../utils/rules'; +import { getRulePluginOrigin, prometheusRuleType } from '../utils/rules'; import { createRelativeUrl } from '../utils/url'; import { AlertRuleListItem, RecordingRuleListItem, UnknownRuleListItem } from './components/AlertRuleListItem'; @@ -74,7 +74,7 @@ export const DataSourceRuleLoader = memo(function DataSourceRuleLoader({ return null; }, [groupIdentifier, isLoading, rule, rulerRule]); - if (isAlertingRule(rule)) { + if (prometheusRuleType.alertingRule(rule)) { return ( { // We might hit edge cases where there type = alerting, but there is no state. // In this case, we shouldn't try to group these alerts in the state view // Even though we handle this at the API layer, this is a last catch point for any edge cases - if (rule.promRule && isAlertingRule(rule.promRule) && rule.promRule.state) { + if (prometheusRuleType.alertingRule(rule.promRule) && rule.promRule.state) { result.get(rule.promRule.state)?.push(rule); } }) @@ -97,8 +97,10 @@ const RulesByState = ({ state, rules }: { state: PromAlertingRuleState; rules: C {pageItems.map((rule) => { const { rulerRule, promRule } = rule; - const isProvisioned = isGrafanaRulerRule(rulerRule) && Boolean(rulerRule.grafana_alert.provenance); - const instancesCount = isAlertingRule(rule.promRule) ? calculateTotalInstances(rule.instanceTotals) : undefined; + const isProvisioned = rulerRuleType.grafana.rule(rulerRule) && Boolean(rulerRule.grafana_alert.provenance); + const instancesCount = prometheusRuleType.alertingRule(rule.promRule) + ? calculateTotalInstances(rule.instanceTotals) + : undefined; const groupId = groupIdentifier.fromCombinedRule(rule); if (!promRule) { diff --git a/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx b/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx index 5c27ea48476..21199ce1523 100644 --- a/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx +++ b/public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx @@ -11,7 +11,7 @@ import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { AlertRuleAction, useRulerRuleAbility } from '../../hooks/useAbilities'; import * as ruleId from '../../utils/rule-id'; -import { isGrafanaAlertingRule, isGrafanaRulerRule } from '../../utils/rules'; +import { isProvisionedRule, rulerRuleType } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; interface Props { @@ -37,7 +37,7 @@ export function RuleActionsButtons({ compact, rule, promRule, groupIdentifier }: { identifier: RuleIdentifier; isProvisioned: boolean } | undefined >(undefined); - const isProvisioned = isGrafanaRulerRule(rule) && Boolean(rule.grafana_alert.provenance); + const isProvisioned = isProvisionedRule(rule); const [editRuleSupported, editRuleAllowed] = useRulerRuleAbility(rule, groupIdentifier, AlertRuleAction.Update); @@ -72,7 +72,7 @@ export function RuleActionsButtons({ compact, rule, promRule, groupIdentifier }: handleDuplicateRule={() => setRedirectToClone({ identifier, isProvisioned })} /> {deleteModal} - {isGrafanaAlertingRule(rule) && showSilenceDrawer && ( + {rulerRuleType.grafana.alertingRule(rule) && showSilenceDrawer && ( setShowSilenceDrawer(false)} /> )} {redirectToClone?.identifier && ( diff --git a/public/app/features/alerting/unified/rule-list/components/RuleGroup.tsx b/public/app/features/alerting/unified/rule-list/components/RuleGroup.tsx index 42c12a08c59..4020917fca5 100644 --- a/public/app/features/alerting/unified/rule-list/components/RuleGroup.tsx +++ b/public/app/features/alerting/unified/rule-list/components/RuleGroup.tsx @@ -7,7 +7,8 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { alertRuleApi } from '../../api/alertRuleApi'; import { usePagination } from '../../hooks/usePagination'; -import { isAlertingRule } from '../../utils/rules'; +import { Annotation } from '../../utils/constants'; +import { prometheusRuleType } from '../../utils/rules'; import { AlertRuleListItem } from './AlertRuleListItem'; import { EvaluationGroup } from './EvaluationGroup'; @@ -65,7 +66,7 @@ export const EvaluationGroupLoader = ({ state={PromAlertingRuleState.Inactive} name={rule.name} href={'/'} - summary={isAlertingRule(rule) ? rule.annotations?.summary : undefined} + summary={prometheusRuleType.alertingRule(rule) ? rule.annotations?.[Annotation.summary] : undefined} />; return null; diff --git a/public/app/features/alerting/unified/rule-list/hooks/useFilteredRulesIterator.ts b/public/app/features/alerting/unified/rule-list/hooks/useFilteredRulesIterator.ts index 6512ddcea18..01ec7fe7bcf 100644 --- a/public/app/features/alerting/unified/rule-list/hooks/useFilteredRulesIterator.ts +++ b/public/app/features/alerting/unified/rule-list/hooks/useFilteredRulesIterator.ts @@ -21,7 +21,7 @@ import { labelsMatchMatchers } from '../../utils/alertmanager'; import { Annotation } from '../../utils/constants'; import { getDatasourceAPIUid, getExternalRulesSources } from '../../utils/datasource'; import { parseMatcher } from '../../utils/matchers'; -import { isAlertingRule } from '../../utils/rules'; +import { prometheusRuleType } from '../../utils/rules'; import { useGrafanaGroupsGenerator, usePrometheusGroupsGenerator } from './prometheusGroupsGenerator'; @@ -168,7 +168,7 @@ function ruleFilter(rule: PromRuleDTO, filterState: RulesFilter) { } if (filterState.ruleState) { - if (!isAlertingRule(rule)) { + if (!prometheusRuleType.alertingRule(rule)) { return false; } if (rule.state !== filterState.ruleState) { diff --git a/public/app/features/alerting/unified/utils/groupIdentifier.ts b/public/app/features/alerting/unified/utils/groupIdentifier.ts index 61ae10d49ab..aa0701c7767 100644 --- a/public/app/features/alerting/unified/utils/groupIdentifier.ts +++ b/public/app/features/alerting/unified/utils/groupIdentifier.ts @@ -1,10 +1,10 @@ import { CombinedRule, RuleGroupIdentifier, RuleGroupIdentifierV2 } from 'app/types/unified-alerting'; import { GRAFANA_RULES_SOURCE_NAME, getDatasourceAPIUid, getRulesSourceName, isGrafanaRulesSource } from './datasource'; -import { isGrafanaRulerRule } from './rules'; +import { rulerRuleType } from './rules'; function fromCombinedRule(rule: CombinedRule): RuleGroupIdentifierV2 { - if (isGrafanaRulerRule(rule.rulerRule) && isGrafanaRulesSource(rule.namespace.rulesSource)) { + if (rulerRuleType.grafana.rule(rule.rulerRule) && isGrafanaRulesSource(rule.namespace.rulesSource)) { return { namespace: { uid: rule.rulerRule.grafana_alert.namespace_uid }, groupName: rule.group.name, diff --git a/public/app/features/alerting/unified/utils/query.ts b/public/app/features/alerting/unified/utils/query.ts index cebef12e2e8..dc40ca3534e 100644 --- a/public/app/features/alerting/unified/utils/query.ts +++ b/public/app/features/alerting/unified/utils/query.ts @@ -8,7 +8,7 @@ import { CombinedRule } from 'app/types/unified-alerting'; import { AlertQuery } from 'app/types/unified-alerting-dto'; import { isCloudRulesSource } from './datasource'; -import { isGrafanaRulerRule } from './rules'; +import { rulerRuleType } from './rules'; import { safeParsePrometheusDuration } from './time'; export function alertRuleToQueries(combinedRule: CombinedRule | undefined | null): AlertQuery[] { @@ -18,7 +18,7 @@ export function alertRuleToQueries(combinedRule: CombinedRule | undefined | null const { namespace, rulerRule } = combinedRule; const { rulesSource } = namespace; - if (isGrafanaRulerRule(rulerRule)) { + if (rulerRuleType.grafana.rule(rulerRule)) { const query = rulerRule.grafana_alert.data; return widenRelativeTimeRanges(query, rulerRule.for ?? '', combinedRule.group.interval); } diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index ecea384f9e7..d9e1130736e 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -58,14 +58,7 @@ import { isGrafanaRulesSource, } from './datasource'; import { arrayToRecord, recordToArray } from './misc'; -import { - isAlertingRulerRule, - isGrafanaAlertingRuleByType, - isGrafanaRecordingRule, - isGrafanaRecordingRuleByType, - isGrafanaRulerRule, - isRecordingRulerRule, -} from './rules'; +import { isGrafanaAlertingRuleByType, isGrafanaRecordingRuleByType, rulerRuleType } from './rules'; import { parseInterval } from './time'; export type PromOrLokiQuery = PromQuery | LokiQuery; @@ -275,7 +268,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF const defaultFormValues = getDefaultFormValues(); if (isGrafanaRulesSource(ruleSourceName)) { // GRAFANA-MANAGED RULES - if (isGrafanaRecordingRule(rule)) { + if (rulerRuleType.grafana.recordingRule(rule)) { // grafana recording rule const ga = rule.grafana_alert; return { @@ -292,7 +285,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF isPaused: ga.is_paused, metric: ga.record?.metric, }; - } else if (isGrafanaRulerRule(rule)) { + } else if (rulerRuleType.grafana.rule(rule)) { // grafana alerting rule const ga = rule.grafana_alert; const routingSettings: AlertManagerManualRouting | undefined = getContactPointsFromDTO(ga); @@ -326,7 +319,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF } } else { // DATASOURCE-MANAGED RULES - if (isAlertingRulerRule(rule)) { + if (rulerRuleType.dataSource.alertingRule(rule)) { const datasourceUid = getDataSourceSrv().getInstanceSettings(ruleSourceName)?.uid ?? ''; const defaultQuery = { @@ -354,7 +347,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF namespace, group: group.name, }; - } else if (isRecordingRulerRule(rule)) { + } else if (rulerRuleType.dataSource.recordingRule(rule)) { const recordingRuleValues = recordingRulerRuleToRuleForm(rule); return { diff --git a/public/app/features/alerting/unified/utils/rule-id.ts b/public/app/features/alerting/unified/utils/rule-id.ts index 747cf852db2..cfe1c288543 100644 --- a/public/app/features/alerting/unified/utils/rule-id.ts +++ b/public/app/features/alerting/unified/utils/rule-id.ts @@ -18,14 +18,12 @@ import { shouldUsePrometheusRulesPrimary } from '../featureToggles'; import { GRAFANA_RULES_SOURCE_NAME } from './datasource'; import { - isAlertingRule, - isAlertingRulerRule, + getRuleName, isCloudRuleIdentifier, isGrafanaRuleIdentifier, - isGrafanaRulerRule, isPrometheusRuleIdentifier, - isRecordingRule, - isRecordingRulerRule, + prometheusRuleType, + rulerRuleType, } from './rules'; export function fromRulerRule( @@ -34,14 +32,14 @@ export function fromRulerRule( groupName: string, rule: RulerRuleDTO ): EditableRuleIdentifier { - if (isGrafanaRulerRule(rule)) { + if (rulerRuleType.grafana.rule(rule)) { return { uid: rule.grafana_alert.uid!, ruleSourceName: 'grafana' }; } return { ruleSourceName, namespace, groupName, - ruleName: isAlertingRulerRule(rule) ? rule.alert : rule.record, + ruleName: getRuleName(rule), rulerRuleHash: hashRulerRule(rule), } satisfies CloudRuleIdentifier; } @@ -51,7 +49,7 @@ export function fromRulerRuleAndGroupIdentifierV2( rule: RulerRuleDTO ): EditableRuleIdentifier { if (ruleGroup.groupOrigin === 'grafana') { - if (isGrafanaRulerRule(rule)) { + if (rulerRuleType.grafana.rule(rule)) { return { uid: rule.grafana_alert.uid, ruleSourceName: 'grafana' }; } logError(new Error('Rule is not a Grafana Ruler rule')); @@ -260,7 +258,7 @@ export function hash(value: string): number { // this is used to identify rules, mimir / loki rules do not have a unique identifier export function hashRulerRule(rule: RulerRuleDTO): string { - if (isGrafanaRulerRule(rule)) { + if (rulerRuleType.grafana.rule(rule)) { return rule.grafana_alert.uid; } @@ -276,10 +274,10 @@ function getRulerRuleFingerprint(rule: RulerCloudRuleDTO) { const queryHash = prometheusRulesPrimary ? '' : hashQuery(rule.expr); const labelsHash = hashLabelsOrAnnotations(rule.labels); - if (isRecordingRulerRule(rule)) { + if (rulerRuleType.dataSource.recordingRule(rule)) { return [rule.record, PromRuleType.Recording, queryHash, labelsHash]; } - if (isAlertingRulerRule(rule)) { + if (rulerRuleType.dataSource.alertingRule(rule)) { return [rule.alert, PromRuleType.Alerting, queryHash, hashLabelsOrAnnotations(rule.annotations), labelsHash]; } throw new Error('Only recording and alerting ruler rules can be hashed'); @@ -296,10 +294,10 @@ function getPromRuleFingerprint(rule: Rule) { const queryHash = prometheusRulesPrimary ? '' : hashQuery(rule.query); const labelsHash = hashLabelsOrAnnotations(rule.labels); - if (isRecordingRule(rule)) { + if (prometheusRuleType.recordingRule(rule)) { return [rule.name, PromRuleType.Recording, queryHash, labelsHash]; } - if (isAlertingRule(rule)) { + if (prometheusRuleType.alertingRule(rule)) { return [rule.name, PromRuleType.Alerting, queryHash, hashLabelsOrAnnotations(rule.annotations), labelsHash]; } throw new Error('Only recording and alerting rules can be hashed'); diff --git a/public/app/features/alerting/unified/utils/rules.ts b/public/app/features/alerting/unified/utils/rules.ts index ad6ffc15a3f..001ecf36d71 100644 --- a/public/app/features/alerting/unified/utils/rules.ts +++ b/public/app/features/alerting/unified/utils/rules.ts @@ -25,6 +25,8 @@ import { Annotations, GrafanaAlertState, GrafanaAlertStateWithReason, + GrafanaAlertingRuleDefinition, + GrafanaRecordingRuleDefinition, PostableRuleDTO, PromAlertingRuleState, PromRuleType, @@ -47,49 +49,71 @@ import { GRAFANA_ORIGIN_LABEL } from './labels'; import { AsyncRequestState } from './redux'; import { formatPrometheusDuration, safeParsePrometheusDuration } from './time'; -export function isAlertingRule(rule: Rule | undefined): rule is AlertingRule { - return typeof rule === 'object' && rule.type === PromRuleType.Alerting; -} +/* Grafana managed rules */ -export function isRecordingRule(rule: Rule | undefined): rule is RecordingRule { - return typeof rule === 'object' && rule.type === PromRuleType.Recording; -} - -export function isAlertingRulerRule(rule?: RulerRuleDTO): rule is RulerAlertingRuleDTO { - return typeof rule === 'object' && 'alert' in rule; -} - -export function isRecordingRulerRule(rule?: RulerRuleDTO): rule is RulerRecordingRuleDTO { - return typeof rule === 'object' && 'record' in rule; -} - -export function isGrafanaOrDataSourceRecordingRule(rule?: RulerRuleDTO) { - return ( - (typeof rule === 'object' && isRecordingRulerRule(rule)) || - (isGrafanaRulerRule(rule) && 'record' in rule.grafana_alert) - ); -} - -export function isGrafanaRecordingRule(rule?: RulerRuleDTO): rule is RulerGrafanaRuleDTO { - return typeof rule === 'object' && isGrafanaOrDataSourceRecordingRule(rule) && isGrafanaRulerRule(rule); -} - -export function isGrafanaAlertingRule(rule?: RulerRuleDTO): rule is RulerGrafanaRuleDTO { - return typeof rule === 'object' && isGrafanaRulerRule(rule) && !isGrafanaOrDataSourceRecordingRule(rule); -} - -export function isGrafanaRulerRule(rule?: RulerRuleDTO | PostableRuleDTO): rule is RulerGrafanaRuleDTO { +function isGrafanaRulerRule(rule?: RulerRuleDTO | PostableRuleDTO): rule is RulerGrafanaRuleDTO { return typeof rule === 'object' && 'grafana_alert' in rule; } -export function isCloudRulerRule(rule?: RulerRuleDTO | PostableRuleDTO): rule is RulerCloudRuleDTO { +function isGrafanaAlertingRule(rule?: RulerRuleDTO): rule is RulerGrafanaRuleDTO { + return isGrafanaRulerRule(rule) && !isGrafanaRecordingRule(rule); +} + +function isGrafanaRecordingRule(rule?: RulerRuleDTO): rule is RulerGrafanaRuleDTO { + return isGrafanaRulerRule(rule) && 'record' in rule.grafana_alert; +} + +export function isPausedRule(rule: RulerGrafanaRuleDTO) { + return Boolean(rule.grafana_alert.is_paused); +} + +/* Data source managed rules */ + +function isAlertingRulerRule(rule?: RulerRuleDTO): rule is RulerAlertingRuleDTO { + return typeof rule === 'object' && 'alert' in rule; +} + +function isCloudRulerRule(rule?: RulerRuleDTO | PostableRuleDTO): rule is RulerCloudRuleDTO { return typeof rule === 'object' && !isGrafanaRulerRule(rule); } -export function isGrafanaRulerRulePaused(rule: RulerGrafanaRuleDTO) { - return rule && isGrafanaRulerRule(rule) && Boolean(rule.grafana_alert.is_paused); +function isCloudRecordingRulerRule(rule?: RulerRuleDTO): rule is RulerRecordingRuleDTO { + return typeof rule === 'object' && 'record' in rule; } +/* Prometheus rules */ + +function isAlertingRule(rule?: Rule): rule is AlertingRule { + return typeof rule === 'object' && rule.type === PromRuleType.Alerting; +} + +function isRecordingRule(rule?: Rule): rule is RecordingRule { + return typeof rule === 'object' && rule.type === PromRuleType.Recording; +} + +export const rulerRuleType = { + grafana: { + rule: isGrafanaRulerRule, + alertingRule: isGrafanaAlertingRule, + recordingRule: isGrafanaRecordingRule, + }, + dataSource: { + rule: isCloudRulerRule, + alertingRule: isAlertingRulerRule, + recordingRule: isCloudRecordingRulerRule, + }, + any: { + recordingRule: (rule?: RulerRuleDTO) => isCloudRecordingRulerRule(rule) || isGrafanaRecordingRule(rule), + alertingRule: (rule?: RulerRuleDTO) => isAlertingRulerRule(rule) || isGrafanaAlertingRule(rule), + }, +}; + +export const prometheusRuleType = { + rule: (rule?: Rule) => isAlertingRule(rule) || isRecordingRule(rule), + alertingRule: isAlertingRule, + recordingRule: isRecordingRule, +}; + export function alertInstanceKey(alert: Alert): string { return JSON.stringify(alert.labels); } @@ -139,11 +163,7 @@ export function getRuleHealth(health: string): RuleHealth | undefined { } export function getPendingPeriod(rule: CombinedRule): string | undefined { - if ( - isRecordingRulerRule(rule.rulerRule) || - isRecordingRule(rule.promRule) || - isGrafanaRecordingRule(rule.rulerRule) - ) { + if (rulerRuleType.any.recordingRule(rule.rulerRule)) { return undefined; } @@ -162,16 +182,10 @@ export function getPendingPeriod(rule: CombinedRule): string | undefined { return undefined; } -export function getAnnotations(rule: CombinedRule): Annotations | undefined { - if ( - isRecordingRulerRule(rule.rulerRule) || - isRecordingRule(rule.promRule) || - isGrafanaRecordingRule(rule.rulerRule) - ) { - return undefined; - } - return rule.annotations ?? []; +export function getAnnotations(rule?: AlertingRule): Annotations { + return rule?.annotations ?? {}; } + export interface RulePluginOrigin { pluginId: string; } @@ -300,15 +314,16 @@ export function isFederatedRuleGroup(group: CombinedRuleGroup) { return Array.isArray(group.source_tenants); } -export function getRuleName(rule: RulerRuleDTO) { - if (isGrafanaRulerRule(rule)) { +export function getRuleName(rule: RulerRuleDTO): string { + if (rulerRuleType.grafana.rule(rule)) { return rule.grafana_alert.title; } - if (isAlertingRulerRule(rule)) { + + if (rulerRuleType.dataSource.alertingRule(rule)) { return rule.alert; } - if (isRecordingRulerRule(rule)) { + if (rulerRuleType.dataSource.recordingRule(rule)) { return rule.record; } diff --git a/public/app/features/dashboard-scene/scene/AlertStatesDataLayer.ts b/public/app/features/dashboard-scene/scene/AlertStatesDataLayer.ts index 54f0a7264f8..643ea0ac56f 100644 --- a/public/app/features/dashboard-scene/scene/AlertStatesDataLayer.ts +++ b/public/app/features/dashboard-scene/scene/AlertStatesDataLayer.ts @@ -17,7 +17,7 @@ import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi'; import { ungroupRulesByFileName } from 'app/features/alerting/unified/api/prometheus'; import { Annotation } from 'app/features/alerting/unified/utils/constants'; import { GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource'; -import { isAlertingRule } from 'app/features/alerting/unified/utils/rules'; +import { prometheusRuleType } from 'app/features/alerting/unified/utils/rules'; import { dispatch } from 'app/store/store'; import { AccessControlAction } from 'app/types'; import { RuleNamespace } from 'app/types/unified-alerting'; @@ -96,7 +96,7 @@ export class AlertStatesDataLayer const panelIdToAlertState: Record = {}; groups.forEach((group) => group.rules.forEach((rule) => { - if (isAlertingRule(rule) && rule.annotations && rule.annotations[Annotation.panelID]) { + if (prometheusRuleType.alertingRule(rule) && rule.annotations?.[Annotation.panelID]) { this.hasAlertRules = true; const panelId = Number(rule.annotations[Annotation.panelID]); const state = promAlertStateToAlertState(rule.state); diff --git a/public/app/features/query/state/DashboardQueryRunner/UnifiedAlertStatesWorker.ts b/public/app/features/query/state/DashboardQueryRunner/UnifiedAlertStatesWorker.ts index 8e352896e54..18c001c983c 100644 --- a/public/app/features/query/state/DashboardQueryRunner/UnifiedAlertStatesWorker.ts +++ b/public/app/features/query/state/DashboardQueryRunner/UnifiedAlertStatesWorker.ts @@ -8,7 +8,7 @@ import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi'; import { ungroupRulesByFileName } from 'app/features/alerting/unified/api/prometheus'; import { Annotation } from 'app/features/alerting/unified/utils/constants'; import { GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource'; -import { isAlertingRule } from 'app/features/alerting/unified/utils/rules'; +import { prometheusRuleType } from 'app/features/alerting/unified/utils/rules'; import { promAlertStateToAlertState } from 'app/features/dashboard-scene/scene/AlertStatesDataLayer'; import { dispatch } from 'app/store/store'; import { AccessControlAction } from 'app/types'; @@ -82,7 +82,7 @@ export class UnifiedAlertStatesWorker implements DashboardQueryRunnerWorker { const panelIdToAlertState: Record = {}; groups.forEach((group) => group.rules.forEach((rule) => { - if (isAlertingRule(rule) && rule.annotations && rule.annotations[Annotation.panelID]) { + if (prometheusRuleType.alertingRule(rule) && rule.annotations && rule.annotations[Annotation.panelID]) { this.hasAlertRules[dashboard.uid] = true; const panelId = Number(rule.annotations[Annotation.panelID]); const state = promAlertStateToAlertState(rule.state); diff --git a/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx b/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx index 6b43f91b632..35a58263e22 100644 --- a/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx +++ b/public/app/plugins/panel/alertlist/unified-alerting/UngroupedView.tsx @@ -10,13 +10,13 @@ import { alertStateToReadable, alertStateToState, getFirstActiveAt, - isAlertingRule, + prometheusRuleType, } from 'app/features/alerting/unified/utils/rules'; import { createRelativeUrl } from 'app/features/alerting/unified/utils/url'; import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../../features/alerting/unified/utils/datasource'; -import { AlertInstanceTotalState, AlertingRule, CombinedRuleWithLocation } from '../../../../types/unified-alerting'; +import { AlertInstanceTotalState, CombinedRuleWithLocation } from '../../../../types/unified-alerting'; import { AlertInstances } from '../AlertInstances'; import { getStyles } from '../UnifiedAlertList'; import { UnifiedAlertListOptions } from '../types'; @@ -47,7 +47,7 @@ const UngroupedModeView = ({ rules, options, handleInstancesLimit, limitInstance
    {rulesToDisplay.map((ruleWithLocation, index) => { const { namespaceName, groupName, dataSourceName } = ruleWithLocation; - const alertingRule: AlertingRule | undefined = isAlertingRule(ruleWithLocation.promRule) + const alertingRule = prometheusRuleType.alertingRule(ruleWithLocation.promRule) ? ruleWithLocation.promRule : undefined; const firstActiveAt = getFirstActiveAt(alertingRule); diff --git a/public/app/types/unified-alerting-dto.ts b/public/app/types/unified-alerting-dto.ts index 85f06bf9722..bf112911756 100644 --- a/public/app/types/unified-alerting-dto.ts +++ b/public/app/types/unified-alerting-dto.ts @@ -278,6 +278,12 @@ export interface GrafanaRuleDefinition extends PostableGrafanaRuleDefinition { version?: number; } +// types for Grafana-managed recording and alerting rules +export type GrafanaAlertingRuleDefinition = Omit; +export type GrafanaRecordingRuleDefinition = GrafanaRuleDefinition & { + record: GrafanaRuleDefinition['record']; +}; + export interface RulerGrafanaRuleDTO { grafana_alert: T; for?: string; From 827da46c5116b02ad85cf2aadbf6a2f0ab4ab7aa Mon Sep 17 00:00:00 2001 From: owensmallwood Date: Fri, 7 Mar 2025 09:51:15 -0600 Subject: [PATCH 20/39] Unified Storage: Remove wildcard queries (#101441) * WIP adding custom analyzer so we can do substring search efficiently * Adding unit tests for title search * formatting * adds more title search unit tests * organize helpers * fixes issue caused by having two title mappings * Removes camelcase token filter since it prevents you from searching for a substring of chars and numbers. Adds regression test. * adds back mapping for title_phrase * use simple analyzer for input query string so it doesn't filter out english stop words * ran bleve tests, table snapshots updated * ignore linter for "unused" test functions. They are very helpful for troubleshooting search. Keeping them. * only log total hits and query cost if result not nil * fixes failing test - one more field because there are two title mappings now * fix test * fixes test - only take first item when its the title * Adds separate internal field for title ngram mapping. When searching with a query, results are sorted by score desc. When searching without a query, results are sorted by title desc. Adjusts ngram max to be 10. Text queries are a disjunction of an exact match, phrase match, and a match. Boosted to have priority in that order. Adds more unit tests for searching. * linter * fix test * ran tests - generated new test dash json * sort by title phrase instead of title * fix test - not relying on /apis/dashboard/search to apply title sorting anymore --- pkg/registry/apis/dashboard/search.go | 8 +- pkg/registry/apis/dashboard/search_test.go | 14 +- pkg/storage/unified/resource/document.go | 7 +- pkg/storage/unified/resource/document_test.go | 1 + pkg/storage/unified/search/bleve.go | 48 +- pkg/storage/unified/search/bleve_mappings.go | 28 +- .../unified/search/bleve_mappings_test.go | 7 +- .../unified/search/bleve_search_test.go | 426 ++++++++++++++++++ .../unified/search/custom_analyzers.go | 48 ++ .../testdata/doc/dashboard-aaa-out.json | 1 + .../search/testdata/doc/folder-aaa-out.json | 1 + .../search/testdata/doc/folder-bbb-out.json | 1 + .../search/testdata/doc/playlist-aaa-out.json | 1 + .../search/testdata/doc/report-aaa-out.json | 1 + 14 files changed, 561 insertions(+), 31 deletions(-) create mode 100644 pkg/storage/unified/search/bleve_search_test.go create mode 100644 pkg/storage/unified/search/custom_analyzers.go diff --git a/pkg/registry/apis/dashboard/search.go b/pkg/registry/apis/dashboard/search.go index 7ade0969ead..ffa9de3b04c 100644 --- a/pkg/registry/apis/dashboard/search.go +++ b/pkg/registry/apis/dashboard/search.go @@ -369,6 +369,10 @@ func (s *SearchHandler) DoSearch(w http.ResponseWriter, r *http.Request) { return } + if result != nil { + s.log.Debug("search result hits and cost", "total_hits", result.TotalHits, "query_cost", result.QueryCost) + } + parsedResults, err := dashboardsearch.ParseResults(result, searchRequest.Offset) if err != nil { errhttp.Write(ctx, err, w) @@ -378,8 +382,8 @@ func (s *SearchHandler) DoSearch(w http.ResponseWriter, r *http.Request) { if len(searchRequest.SortBy) == 0 { // default sort by resource descending ( folders then dashboards ) then title sort.Slice(parsedResults.Hits, func(i, j int) bool { - return parsedResults.Hits[i].Resource > parsedResults.Hits[j].Resource || - (parsedResults.Hits[i].Resource == parsedResults.Hits[j].Resource && strings.ToLower(parsedResults.Hits[i].Title) < strings.ToLower(parsedResults.Hits[j].Title)) + // Just sorting by resource for now. The rest should be sorted by search score already + return parsedResults.Hits[i].Resource > parsedResults.Hits[j].Resource }) } diff --git a/pkg/registry/apis/dashboard/search_test.go b/pkg/registry/apis/dashboard/search_test.go index 16460c87a69..70a669b5c55 100644 --- a/pkg/registry/apis/dashboard/search_test.go +++ b/pkg/registry/apis/dashboard/search_test.go @@ -276,7 +276,7 @@ func TestSearchHandler(t *testing.T) { } }) - t.Run("Sort - default sort by resource then title", func(t *testing.T) { + t.Run("Sort - default sort by resource", func(t *testing.T) { rows := make([]*resource.ResourceTableRow, len(mockResults)) for i, r := range mockResults { rows[i] = &resource.ResourceTableRow{ @@ -334,7 +334,7 @@ func TestSearchHandler(t *testing.T) { err := json.NewDecoder(resp.Body).Decode(p) require.NoError(t, err) assert.Equal(t, len(mockResults), len(p.Hits)) - assert.Equal(t, mockResults[3].Value, p.Hits[0].Title) + assert.Equal(t, mockResults[2].Value, p.Hits[0].Title) assert.Equal(t, mockResults[1].Value, p.Hits[3].Title) }) } @@ -646,16 +646,16 @@ var mockResults = []MockResult{ Resource: "dashboard", Value: "Dashboard 2", }, - { - Name: "f2", - Resource: "folder", - Value: "Folder 2", - }, { Name: "f1", Resource: "folder", Value: "Folder 1", }, + { + Name: "f2", + Resource: "folder", + Value: "Folder 2", + }, } func (m *MockClient) Search(ctx context.Context, in *resource.ResourceSearchRequest, opts ...grpc.CallOption) (*resource.ResourceSearchResponse, error) { diff --git a/pkg/storage/unified/resource/document.go b/pkg/storage/unified/resource/document.go index 2229527bb56..c8d3b72072b 100644 --- a/pkg/storage/unified/resource/document.go +++ b/pkg/storage/unified/resource/document.go @@ -62,6 +62,9 @@ type IndexableDocument struct { // The generic display name Title string `json:"title,omitempty"` + // internal field for searching title with ngrams + TitleNgram string `json:"title_ngram,omitempty"` + // internal sort field for title ( don't set this directly ) TitlePhrase string `json:"title_phrase,omitempty"` @@ -169,7 +172,8 @@ func NewIndexableDocument(key *ResourceKey, rv int64, obj utils.GrafanaMetaAcces Key: key, RV: rv, Name: key.Name, - Title: title, // We always want *something* to display + Title: title, // We always want *something* to display + TitleNgram: title, TitlePhrase: strings.ToLower(title), // Lowercase for case-insensitive sorting Labels: obj.GetLabels(), Folder: obj.GetFolder(), @@ -264,6 +268,7 @@ const SEARCH_FIELD_NAMESPACE = "namespace" const SEARCH_FIELD_NAME = "name" const SEARCH_FIELD_RV = "rv" const SEARCH_FIELD_TITLE = "title" +const SEARCH_FIELD_TITLE_NGRAM = "title_ngram" const SEARCH_FIELD_TITLE_PHRASE = "title_phrase" // filtering/sorting on title by full phrase const SEARCH_FIELD_DESCRIPTION = "description" const SEARCH_FIELD_TAGS = "tags" diff --git a/pkg/storage/unified/resource/document_test.go b/pkg/storage/unified/resource/document_test.go index 2135748533d..8c14550578b 100644 --- a/pkg/storage/unified/resource/document_test.go +++ b/pkg/storage/unified/resource/document_test.go @@ -37,6 +37,7 @@ func TestStandardDocumentBuilder(t *testing.T) { "rv": 10, "title": "test playlist unified storage", "title_phrase": "test playlist unified storage", + "title_ngram": "test playlist unified storage", "created": 1717236672000, "createdBy": "user:ABC", "updatedBy": "user:XYZ", diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index 73eee078f69..621e6c3c1b5 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/storage/unified/search/bleve.go @@ -13,6 +13,8 @@ import ( "time" "github.com/blevesearch/bleve/v2" + "github.com/blevesearch/bleve/v2/analysis/analyzer/keyword" + "github.com/blevesearch/bleve/v2/analysis/analyzer/standard" "github.com/blevesearch/bleve/v2/mapping" "github.com/blevesearch/bleve/v2/search" "github.com/blevesearch/bleve/v2/search/query" @@ -118,7 +120,10 @@ func (b *bleveBackend) BuildIndex(ctx context.Context, var index bleve.Index build := true - mapper := getBleveMappings(fields) + mapper, err := getBleveMappings(fields) + if err != nil { + return nil, err + } if size > b.opts.FileThreshold { resourceDir := filepath.Join(b.opts.Root, key.Namespace, @@ -596,12 +601,26 @@ func (b *bleveIndex) toBleveSearchRequest(ctx context.Context, req *resource.Res // Add a text query if req.Query != "" && req.Query != "*" { searchrequest.Fields = append(searchrequest.Fields, resource.SEARCH_FIELD_SCORE) - // mimic the behavior of the sql search - query := strings.ToLower(req.Query) - if !strings.Contains(query, "*") { - query = "*" + query + "*" - } - queries = append(queries, bleve.NewWildcardQuery(query)) + + // There are multiple ways to match the query string to documents. The following queries are ordered by priority: + + // Query 1: Match the exact query string + queryExact := bleve.NewMatchQuery(req.Query) + queryExact.SetBoost(10.0) + queryExact.Analyzer = keyword.Name // don't analyze the query input - treat it as a single token + + // Query 2: Phrase query with standard analyzer + queryPhrase := bleve.NewMatchPhraseQuery(req.Query) + queryExact.SetBoost(5.0) + queryPhrase.Analyzer = standard.Name + + // Query 3: Match query with standard analyzer + queryAnalyzed := bleve.NewMatchQuery(req.Query) + queryAnalyzed.Analyzer = standard.Name + + // At least one of the queries must match + searchQuery := bleve.NewDisjunctionQuery(queryExact, queryAnalyzed, queryPhrase) + queries = append(queries, searchQuery) } switch len(queries) { @@ -664,11 +683,18 @@ func (b *bleveIndex) toBleveSearchRequest(ctx context.Context, req *resource.Res sorting := getSortFields(req) searchrequest.SortBy(sorting) - // Always sort by *something*, otherwise the order is unstable + // When no sort fields are provided, sort by score if there is a query, otherwise sort by title if len(sorting) == 0 { - searchrequest.Sort = append(searchrequest.Sort, &search.SortDocID{ - Desc: false, - }) + if req.Query != "" && req.Query != "*" { + searchrequest.Sort = append(searchrequest.Sort, &search.SortScore{ + Desc: true, + }) + } else { + searchrequest.Sort = append(searchrequest.Sort, &search.SortField{ + Field: resource.SEARCH_FIELD_TITLE_PHRASE, + Desc: false, + }) + } } return searchrequest, nil diff --git a/pkg/storage/unified/search/bleve_mappings.go b/pkg/storage/unified/search/bleve_mappings.go index 7c59bbae559..8ffceeca568 100644 --- a/pkg/storage/unified/search/bleve_mappings.go +++ b/pkg/storage/unified/search/bleve_mappings.go @@ -3,15 +3,22 @@ package search import ( "github.com/blevesearch/bleve/v2" "github.com/blevesearch/bleve/v2/analysis/analyzer/keyword" + "github.com/blevesearch/bleve/v2/analysis/analyzer/standard" "github.com/blevesearch/bleve/v2/mapping" "github.com/grafana/grafana/pkg/storage/unified/resource" ) -func getBleveMappings(fields resource.SearchableDocumentFields) mapping.IndexMapping { +func getBleveMappings(fields resource.SearchableDocumentFields) (mapping.IndexMapping, error) { mapper := bleve.NewIndexMapping() + + err := RegisterCustomAnalyzers(mapper) + if err != nil { + return nil, err + } mapper.DefaultMapping = getBleveDocMappings(fields) - return mapper + + return mapper, nil } func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentMapping { @@ -24,15 +31,22 @@ func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentM } mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_NAME, nameMapping) + // for searching by title - uses an edge ngram token filter + titleSearchMapping := bleve.NewTextFieldMapping() + titleSearchMapping.Analyzer = TITLE_ANALYZER + titleSearchMapping.Store = true + mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE_NGRAM, titleSearchMapping) + + // mapping for title to search on words/tokens larger than the ngram size + titleWordMapping := bleve.NewTextFieldMapping() + titleWordMapping.Analyzer = standard.Name + titleWordMapping.Store = true + mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE, titleWordMapping) + // for filtering/sorting by title full phrase titlePhraseMapping := bleve.NewKeywordFieldMapping() mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE_PHRASE, titlePhraseMapping) - // for searching by title - // TODO: do we still need this since we have SEARCH_FIELD_TITLE_PHRASE? - titleSearchMapping := bleve.NewTextFieldMapping() - mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_TITLE, titleSearchMapping) - descriptionMapping := &mapping.FieldMapping{ Name: resource.SEARCH_FIELD_DESCRIPTION, Type: "text", diff --git a/pkg/storage/unified/search/bleve_mappings_test.go b/pkg/storage/unified/search/bleve_mappings_test.go index 7a563fff827..e4aa1165a8a 100644 --- a/pkg/storage/unified/search/bleve_mappings_test.go +++ b/pkg/storage/unified/search/bleve_mappings_test.go @@ -12,7 +12,8 @@ import ( ) func TestDocumentMapping(t *testing.T) { - mappings := getBleveMappings(nil) + mappings, err := getBleveMappings(nil) + require.NoError(t, err) data := resource.IndexableDocument{ Title: "title", Description: "descr", @@ -37,7 +38,7 @@ func TestDocumentMapping(t *testing.T) { } doc := document.NewDocument("id") - err := mappings.MapDocument(doc, data) + err = mappings.MapDocument(doc, data) require.NoError(t, err) for _, f := range doc.Fields { @@ -46,5 +47,5 @@ func TestDocumentMapping(t *testing.T) { fmt.Printf("DOC: fields %d\n", len(doc.Fields)) fmt.Printf("DOC: size %d\n", doc.Size()) - require.Equal(t, 14, len(doc.Fields)) + require.Equal(t, 15, len(doc.Fields)) } diff --git a/pkg/storage/unified/search/bleve_search_test.go b/pkg/storage/unified/search/bleve_search_test.go new file mode 100644 index 00000000000..ce4e8814916 --- /dev/null +++ b/pkg/storage/unified/search/bleve_search_test.go @@ -0,0 +1,426 @@ +package search + +import ( + "context" + "fmt" + "log" + "os" + "testing" + + "github.com/blevesearch/bleve/v2" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/store/kind/dashboard" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/storage/unified/resource" + "github.com/stretchr/testify/require" +) + +func TestCanSearchByTitle(t *testing.T) { + key := &resource.ResourceKey{ + Namespace: "default", + Group: "dashboard.grafana.app", + Resource: "dashboards", + } + + t.Run("when query is empty, sort documents by title instead of search score", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "name1", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "bbb", + TitleNgram: "bbb", + TitlePhrase: "bbb", + }) + require.NoError(t, err) + err = index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name2", + Key: &resource.ResourceKey{ + Name: "name2", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "aaa", + TitleNgram: "aaa", + TitlePhrase: "aaa", + }) + require.NoError(t, err) + + // search for phrase + query := newQuery("") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(2), res.TotalHits) + require.Equal(t, "name2", res.Results.Rows[0].Key.Name) + }) + + t.Run("will boost phrase match query over match query results", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "name1", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "I want to say a hello", + TitleNgram: "I want to say a hello", + TitlePhrase: "I want to say a hello", + }) + require.NoError(t, err) + err = index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name2", + Key: &resource.ResourceKey{ + Name: "name2", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "we want hello", + TitleNgram: "we want hello", + TitlePhrase: "we want hello", + }) + require.NoError(t, err) + + // search for phrase + query := newQuery("want hello") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(2), res.TotalHits) + require.Equal(t, "name2", res.Results.Rows[0].Key.Name) + }) + + t.Run("will prioritize matches", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "name1", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "Asserts Dashboards", + TitleNgram: "Asserts Dashboards", + TitlePhrase: "Asserts Dashboards", + }) + require.NoError(t, err) + err = index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name2", + Key: &resource.ResourceKey{ + Name: "name2", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "New dashboard 10", + TitleNgram: "New dashboard 10", + TitlePhrase: "New dashboard 10", + }) + require.NoError(t, err) + + query := newQuery("New dash") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(2), res.TotalHits) + require.Equal(t, "name2", res.Results.Rows[0].Key.Name) + }) + + t.Run("will boost exact match query over match phrase query results", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "name1", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "we want hello pls", + TitleNgram: "we want hello pls", + TitlePhrase: "we want hello pls", + }) + require.NoError(t, err) + err = index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name2", + Key: &resource.ResourceKey{ + Name: "name2", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "we want hello", + TitleNgram: "we want hello", + TitlePhrase: "we want hello", + }) + require.NoError(t, err) + + // search for exact match + query := newQuery("we want hello") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(2), res.TotalHits) + require.Equal(t, "name2", res.Results.Rows[0].Key.Name) + }) + + t.Run("title with numbers will match document", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "aaa", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "A123456", + TitleNgram: "A123456", + }) + require.NoError(t, err) + + // search for prefix of title with mix of chars and numbers + query := newQuery("A12") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // search for whole title + query = newQuery("A123456") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + }) + + t.Run("title search will match document", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "aaa", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "I want to say a wonderfully Hello to the WORLD! Hello-world", + TitleNgram: "I want to say a wonderfully Hello to the WORLD! Hello-world", + TitlePhrase: "I want to say a wonderfully Hello to the WORLD! Hello-world", + }) + require.NoError(t, err) + + // search by entire phrase + query := newQuery("I want to say a wonderfully Hello to the WORLD! Hello-world") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // search for word at start + query = newQuery("hello") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // search for word larger than ngram max size + query = newQuery("wonderfully") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // search for word at end + query = newQuery("world") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // can search for word substring anchored at start of word (edge ngram) + query = newQuery("worl") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // can search for multiple, non-consecutive words in title + query = newQuery("hello world") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + + // can search for a term with a hyphen + query = newQuery("hello-world") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(1), res.TotalHits) + }) + + t.Run("title search will NOT match documents", func(t *testing.T) { + index := newTestDashboardsIndex(t) + err := index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name1", + Key: &resource.ResourceKey{ + Name: "name1", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "I want to say a wonderful Hello to the WORLD! Hello-world", + TitleNgram: "I want to say a wonderful Hello to the WORLD! Hello-world", + }) + require.NoError(t, err) + err = index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name2", + Key: &resource.ResourceKey{ + Name: "name2", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "A0456", + TitleNgram: "A0456", + }) + require.NoError(t, err) + err = index.Write(&resource.IndexableDocument{ + RV: 1, + Name: "name3", + Key: &resource.ResourceKey{ + Name: "name3", + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, + Title: "mash-A02382-10", + TitleNgram: "mash-A02382-10", + TitlePhrase: "mash-A02382-10", + }) + require.NoError(t, err) + + // word that doesn't exist + query := newQuery("cats") + res, err := index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(0), res.TotalHits) + + // string shorter than 3 chars (ngam min) + query = newQuery("ma") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(0), res.TotalHits) + + // substring that doesn't exist + query = newQuery("A01") + res, err = index.Search(context.Background(), nil, query, nil) + require.NoError(t, err) + require.Equal(t, int64(0), res.TotalHits) + }) +} + +func newQuery(query string) *resource.ResourceSearchRequest { + return &resource.ResourceSearchRequest{ + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Namespace: "default", + Group: "dashboard.grafana.app", + Resource: "dashboards", + }, + }, + Limit: 100000, + Query: query, + } +} + +func newTestDashboardsIndex(t *testing.T) resource.ResourceIndex { + key := &resource.ResourceKey{ + Namespace: "default", + Group: "dashboard.grafana.app", + Resource: "dashboards", + } + tmpdir, err := os.MkdirTemp("", "grafana-bleve-test") + require.NoError(t, err) + + backend, err := NewBleveBackend(BleveOptions{ + Root: tmpdir, + FileThreshold: 9999, // use in-memory for tests + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering)) + require.NoError(t, err) + + // AVOID NPE in test + resource.NewIndexMetrics(backend.opts.Root, backend) + + rv := int64(10) + ctx := identity.WithRequester(context.Background(), &user.SignedInUser{Namespace: "ns"}) + + info, err := DashboardBuilder(func(ctx context.Context, namespace string, blob resource.BlobSupport) (resource.DocumentBuilder, error) { + return &DashboardDocumentBuilder{ + Namespace: namespace, + Blob: blob, + Stats: make(map[string]map[string]int64), // empty stats + DatasourceLookup: dashboard.CreateDatasourceLookup([]*dashboard.DatasourceQueryResult{{}}), + }, nil + }) + require.NoError(t, err) + + index, err := backend.BuildIndex(ctx, resource.NamespacedResource{ + Namespace: key.Namespace, + Group: key.Group, + Resource: key.Resource, + }, 2, rv, info.Fields, func(index resource.ResourceIndex) (int64, error) { return 0, nil }) + require.NoError(t, err) + + return index +} + +// helper to check which tokens are generated by an analyzer +// nolint:unused +func debugAnalyzer(index bleve.Index, analyzerName string, text string) { + // Get the analyzer (default: "standard") + analyzer := index.Mapping().AnalyzerNamed(analyzerName) + if analyzer == nil { + log.Fatal("Analyzer not found") + } + + // Analyze text to see generated tokens + analysisResult := analyzer.Analyze([]byte(text)) + + // Print tokens + fmt.Println("Generated tokens for analyzer:", analyzerName) + for _, token := range analysisResult { + fmt.Println(string(token.Term)) + } +} + +// helper to check which terms are indexed for a field +// nolint:unused +func debugIndexedTerms(index bleve.Index, field string) { + // Check what terms exist for the title field + fieldTerms, err := index.FieldDict(field) + if err != nil { + log.Fatal(err) + } + + for { + term, err := fieldTerms.Next() + if err != nil { + break + } + if term != nil { + fmt.Println(term.Term) + } + } +} diff --git a/pkg/storage/unified/search/custom_analyzers.go b/pkg/storage/unified/search/custom_analyzers.go new file mode 100644 index 00000000000..51e2176298b --- /dev/null +++ b/pkg/storage/unified/search/custom_analyzers.go @@ -0,0 +1,48 @@ +package search + +import ( + "github.com/blevesearch/bleve/v2/analysis/analyzer/custom" + "github.com/blevesearch/bleve/v2/analysis/token/edgengram" + "github.com/blevesearch/bleve/v2/analysis/token/lowercase" + "github.com/blevesearch/bleve/v2/analysis/token/unique" + "github.com/blevesearch/bleve/v2/analysis/tokenizer/whitespace" + "github.com/blevesearch/bleve/v2/mapping" +) + +const TITLE_ANALYZER = "title_analyzer" + +func RegisterCustomAnalyzers(mapper *mapping.IndexMappingImpl) error { + return registerTitleAnalyzer(mapper) +} + +// The registerTitleAnalyzer function defines a custom analyzer for the title field. +// The edgeNgramTokenFilter will create n-grams anchored to the front of each token. +// For example, the token "hello" will be tokenized into "hel", "hell", "hello". +func registerTitleAnalyzer(mapper *mapping.IndexMappingImpl) error { + // Define an N-Gram tokenizer (for substring search) + edgeNgramTokenFilter := map[string]interface{}{ + "type": edgengram.Name, + "min": 3.0, + "max": 10.0, + "back": edgengram.FRONT, + } + err := mapper.AddCustomTokenFilter("edge_ngram_filter", edgeNgramTokenFilter) + if err != nil { + return err + } + + //Create a custom analyzer using the N-Gram tokenizer + ngramAnalyzer := map[string]interface{}{ + "type": custom.Name, + "tokenizer": whitespace.Name, + "token_filters": []string{"edge_ngram_filter", lowercase.Name, unique.Name}, + //"char_filters": //TODO IF NEEDED + } + + err = mapper.AddCustomAnalyzer(TITLE_ANALYZER, ngramAnalyzer) + if err != nil { + return err + } + + return nil +} diff --git a/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json b/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json index 9fa0a53b2c1..1edd60a2ec9 100644 --- a/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/dashboard-aaa-out.json @@ -8,6 +8,7 @@ "name": "aaa", "rv": 1234, "title": "Test title", + "title_ngram": "Test title", "title_phrase": "test title", "description": "test description", "tags": [ diff --git a/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json b/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json index c2807e7f48a..aff5002f8b6 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json @@ -8,6 +8,7 @@ "name": "aaa", "rv": 1234, "title": "test-aaa", + "title_ngram": "test-aaa", "title_phrase": "test-aaa", "created": 1730490142000, "createdBy": "user:1", diff --git a/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json b/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json index cabacb8b404..5ff5292aa96 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json +++ b/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json @@ -8,6 +8,7 @@ "name": "bbb", "rv": 1234, "title": "test-bbb", + "title_ngram": "test-bbb", "title_phrase": "test-bbb", "created": 1730490142000, "createdBy": "user:1", diff --git a/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json b/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json index 0bc5b824883..cb49b90a95a 100644 --- a/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json @@ -8,6 +8,7 @@ "name": "aaa", "rv": 1234, "title": "Test AAA", + "title_ngram": "Test AAA", "title_phrase": "test aaa", "created": 1731336353000, "createdBy": "user:t000000001" diff --git a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json index 656f57c46f1..0eab64c17e5 100644 --- a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json @@ -8,6 +8,7 @@ "name": "aaa", "rv": 1234, "title": "Test AAA", + "title_ngram": "Test AAA", "title_phrase": "test aaa", "labels": { "grafana.app/deprecatedInternalID": "123" From 48ea9b08a239dc79d73b40d4455bde45e477c8ac Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Fri, 7 Mar 2025 16:56:24 +0100 Subject: [PATCH 21/39] Alerting: Add recording rule target datasource support to Prometheus conversion API (#101799) What is this feature? Adds target datasource UID to the recording rules so that they write to the same datasource used for alerting rule queries after the import. Why do we need this feature? Target datasourse support was added in #101678, and under a feature flag grafanaManagedRecordingRulesDatasources (#101778). This PR makes the importing process: Check if the import contains recording rules Verify both recording rules and the grafanaManagedRecordingRulesDatasources feature flag are enabled If either check fails, return an error If both checks pass, create recording rules with the provided datasource UID set as both the query and target datasource --- pkg/services/ngalert/api/api.go | 9 +- .../ngalert/api/api_convert_prometheus.go | 47 +++++++- .../api/api_convert_prometheus_test.go | 109 ++++++++++++++++-- pkg/services/ngalert/prom/convert.go | 5 +- pkg/services/ngalert/prom/convert_test.go | 1 + .../alerting/api_convert_prometheus_test.go | 18 ++- pkg/tests/testinfra/testinfra.go | 8 ++ 7 files changed, 176 insertions(+), 21 deletions(-) diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index 10d50a6b0b5..03431b0d7ca 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -188,7 +188,14 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) { if api.FeatureManager.IsEnabledGlobally(featuremgmt.FlagAlertingConversionAPI) { api.RegisterConvertPrometheusApiEndpoints(NewConvertPrometheusApi( - NewConvertPrometheusSrv(&api.Cfg.UnifiedAlerting, logger, api.RuleStore, api.DatasourceCache, api.AlertRules), + NewConvertPrometheusSrv( + &api.Cfg.UnifiedAlerting, + logger, + api.RuleStore, + api.DatasourceCache, + api.AlertRules, + api.FeatureManager, + ), ), m) } } diff --git a/pkg/services/ngalert/api/api_convert_prometheus.go b/pkg/services/ngalert/api/api_convert_prometheus.go index 56deec061f5..3c7878bc500 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus.go +++ b/pkg/services/ngalert/api/api_convert_prometheus.go @@ -18,6 +18,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -47,7 +48,17 @@ var ( ).Errorf("missing datasource UID header") errInvalidHeaderValueMsg = "Invalid value for header {{.Public.Header}}: must be 'true' or 'false'" - errInvalidHeaderValueBase = errutil.ValidationFailed("aleting.invalidHeaderValue").MustTemplate(errInvalidHeaderValueMsg, errutil.WithPublic(errInvalidHeaderValueMsg)) + errInvalidHeaderValueBase = errutil.ValidationFailed("alerting.invalidHeaderValue").MustTemplate(errInvalidHeaderValueMsg, errutil.WithPublic(errInvalidHeaderValueMsg)) + + errRecordingRulesNotEnabled = errutil.ValidationFailed( + "alerting.recordingRulesNotEnabled", + errutil.WithPublicMessage("Cannot import recording rules: Feature not enabled."), + ).Errorf("recording rules not enabled") + + errRecordingRulesDatasourcesNotEnabled = errutil.ValidationFailed( + "alerting.recordingRulesDatasourcesNotEnabled", + errutil.WithPublicMessage("Cannot import recording rules: Configuration of target datasources not enabled."), + ).Errorf("recording rules target datasources configuration not enabled") ) func errInvalidHeaderValue(header string) error { @@ -98,15 +109,24 @@ type ConvertPrometheusSrv struct { ruleStore RuleStore datasourceCache datasources.CacheService alertRuleService *provisioning.AlertRuleService + featureToggles featuremgmt.FeatureToggles } -func NewConvertPrometheusSrv(cfg *setting.UnifiedAlertingSettings, logger log.Logger, ruleStore RuleStore, datasourceCache datasources.CacheService, alertRuleService *provisioning.AlertRuleService) *ConvertPrometheusSrv { +func NewConvertPrometheusSrv( + cfg *setting.UnifiedAlertingSettings, + logger log.Logger, + ruleStore RuleStore, + datasourceCache datasources.CacheService, + alertRuleService *provisioning.AlertRuleService, + featureToggles featuremgmt.FeatureToggles, +) *ConvertPrometheusSrv { return &ConvertPrometheusSrv{ cfg: cfg, logger: logger, ruleStore: ruleStore, datasourceCache: datasourceCache, alertRuleService: alertRuleService, + featureToggles: featureToggles, } } @@ -301,6 +321,20 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostRuleGroup(c *contextm workingFolderUID := getWorkingFolderUID(c) logger = logger.New("folder_title", namespaceTitle, "group", promGroup.Name, "working_folder_uid", workingFolderUID) + // If we're importing recording rules, we can only import them if the feature is enabled, + // and the feature flag that enables configuring target datasources per-rule is also enabled. + if promGroupHasRecordingRules(promGroup) { + if !srv.cfg.RecordingRules.Enabled { + logger.Error("Cannot import recording rules", "error", errRecordingRulesNotEnabled) + return errorToResponse(errRecordingRulesNotEnabled) + } + + if !srv.featureToggles.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRulesDatasources) { + logger.Error("Cannot import recording rules", "error", errRecordingRulesDatasourcesNotEnabled) + return errorToResponse(errRecordingRulesDatasourcesNotEnabled) + } + } + logger.Info("Converting Prometheus rule group", "rules", len(promGroup.Rules)) ns, errResp := srv.getOrCreateNamespace(c, namespaceTitle, logger, workingFolderUID) @@ -494,3 +528,12 @@ func namespaceErrorResponse(err error) response.Response { return toNamespaceErrorResponse(err) } + +func promGroupHasRecordingRules(promGroup apimodels.PrometheusRuleGroup) bool { + for _, rule := range promGroup.Rules { + if rule.Record != "" { + return true + } + } + return false +} diff --git a/pkg/services/ngalert/api/api_convert_prometheus_test.go b/pkg/services/ngalert/api/api_convert_prometheus_test.go index 44d1973f519..c6b0a33166a 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus_test.go +++ b/pkg/services/ngalert/api/api_convert_prometheus_test.go @@ -17,6 +17,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" dsfakes "github.com/grafana/grafana/pkg/services/datasources/fakes" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/foldertest" acfakes "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol/fakes" @@ -48,6 +49,13 @@ func TestRouteConvertPrometheusPostRuleGroup(t *testing.T) { "severity": "critical", }, }, + { + Record: "recorded-metric", + Expr: "vector(1)", + Labels: map[string]string{ + "severity": "warning", + }, + }, }, } @@ -104,21 +112,39 @@ func TestRouteConvertPrometheusPostRuleGroup(t *testing.T) { response := srv.RouteConvertPrometheusPostRuleGroup(rc, fldr.Title, simpleGroup) require.Equal(t, http.StatusAccepted, response.Status()) - // Get the updated rule + // Get the rules remaining, err := ruleStore.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{ OrgID: 1, }) require.NoError(t, err) - require.Len(t, remaining, 1) + require.Len(t, remaining, 2) - require.Equal(t, simpleGroup.Name, remaining[0].RuleGroup) - require.Equal(t, fmt.Sprintf("[%s] %s", simpleGroup.Name, simpleGroup.Rules[0].Alert), remaining[0].Title) - promRuleYAML, err := yaml.Marshal(simpleGroup.Rules[0]) - require.NoError(t, err) + // Create a map of rule titles to their expected definitions + expectedRules := make(map[string]string) + for _, rule := range simpleGroup.Rules { + if rule.Alert != "" { + title := fmt.Sprintf("[%s] %s", simpleGroup.Name, rule.Alert) + promRuleYAML, err := yaml.Marshal(rule) + require.NoError(t, err) + expectedRules[title] = string(promRuleYAML) + } else if rule.Record != "" { + title := fmt.Sprintf("[%s] %s", simpleGroup.Name, rule.Record) + promRuleYAML, err := yaml.Marshal(rule) + require.NoError(t, err) + expectedRules[title] = string(promRuleYAML) + } + } - promDefinition, err := remaining[0].PrometheusRuleDefinition() - require.NoError(t, err) - require.Equal(t, string(promRuleYAML), promDefinition) + // Verify each rule matches its expected definition + for _, r := range remaining { + require.Equal(t, simpleGroup.Name, r.RuleGroup) + expectedDef, exists := expectedRules[r.Title] + require.True(t, exists, "unexpected rule title: %s", r.Title) + + promDefinition, err := r.PrometheusRuleDefinition() + require.NoError(t, err) + require.Equal(t, expectedDef, promDefinition) + } }) t.Run("should fail to replace a provisioned rule group", func(t *testing.T) { @@ -263,6 +289,58 @@ func TestRouteConvertPrometheusPostRuleGroup(t *testing.T) { response := srv.RouteConvertPrometheusPostRuleGroup(rc, "test", simpleGroup) require.Equal(t, http.StatusAccepted, response.Status()) }) + + t.Run("with disabled recording rules", func(t *testing.T) { + testCases := []struct { + name string + recordingRules bool + recordingRulesTargetDS bool + expectedStatus int + }{ + { + name: "when recording rules are enabled", + recordingRules: true, + recordingRulesTargetDS: true, + expectedStatus: http.StatusAccepted, + }, + { + name: "when recording rules are disabled", + recordingRules: false, + recordingRulesTargetDS: true, + expectedStatus: http.StatusBadRequest, + }, + { + name: "when target datasources for recording rules are disabled", + recordingRules: true, + recordingRulesTargetDS: false, + expectedStatus: http.StatusBadRequest, + }, + { + name: "when both recording rules and target datasources are disabled", + recordingRules: false, + recordingRulesTargetDS: false, + expectedStatus: http.StatusBadRequest, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var features featuremgmt.FeatureToggles + if tc.recordingRulesTargetDS { + features = featuremgmt.WithFeatures(featuremgmt.FlagGrafanaManagedRecordingRulesDatasources) + } else { + features = featuremgmt.WithFeatures() + } + + srv, _, _, _ := createConvertPrometheusSrv(t, withFeatureToggles(features)) + srv.cfg.RecordingRules.Enabled = tc.recordingRules + rc := createRequestCtx() + + response := srv.RouteConvertPrometheusPostRuleGroup(rc, "test", simpleGroup) + require.Equal(t, tc.expectedStatus, response.Status()) + }) + } + }) } func TestRouteConvertPrometheusGetRuleGroup(t *testing.T) { @@ -783,6 +861,7 @@ type convertPrometheusSrvOptions struct { provenanceStore provisioning.ProvisioningStore fakeAccessControlRuleService *acfakes.FakeRuleService quotaChecker *provisioning.MockQuotaChecker + featureToggles featuremgmt.FeatureToggles } type convertPrometheusSrvOptionsFunc func(*convertPrometheusSrvOptions) @@ -805,6 +884,12 @@ func withQuotaChecker(checker *provisioning.MockQuotaChecker) convertPrometheusS } } +func withFeatureToggles(toggles featuremgmt.FeatureToggles) convertPrometheusSrvOptionsFunc { + return func(opts *convertPrometheusSrvOptions) { + opts.featureToggles = toggles + } +} + func createConvertPrometheusSrv(t *testing.T, opts ...convertPrometheusSrvOptionsFunc) (*ConvertPrometheusSrv, datasources.CacheService, *fakes.RuleStore, *foldertest.FakeService) { t.Helper() @@ -816,6 +901,7 @@ func createConvertPrometheusSrv(t *testing.T, opts ...convertPrometheusSrvOption provenanceStore: fakes.NewFakeProvisioningStore(), fakeAccessControlRuleService: &acfakes.FakeRuleService{}, quotaChecker: quotas, + featureToggles: featuremgmt.WithFeatures(featuremgmt.FlagGrafanaManagedRecordingRulesDatasources), } for _, opt := range opts { @@ -851,9 +937,12 @@ func createConvertPrometheusSrv(t *testing.T, opts ...convertPrometheusSrvOption cfg := &setting.UnifiedAlertingSettings{ DefaultRuleEvaluationInterval: 1 * time.Minute, + RecordingRules: setting.RecordingRuleSettings{ + Enabled: true, + }, } - srv := NewConvertPrometheusSrv(cfg, log.NewNopLogger(), ruleStore, dsCache, alertRuleService) + srv := NewConvertPrometheusSrv(cfg, log.NewNopLogger(), ruleStore, dsCache, alertRuleService, options.featureToggles) return srv, dsCache, ruleStore, folderService } diff --git a/pkg/services/ngalert/prom/convert.go b/pkg/services/ngalert/prom/convert.go index 56c6e4aa96c..ccbbf89e23b 100644 --- a/pkg/services/ngalert/prom/convert.go +++ b/pkg/services/ngalert/prom/convert.go @@ -191,8 +191,9 @@ func (p *Converter) convertRule(orgID int64, namespaceUID string, promGroup Prom if isRecordingRule { record = &models.Record{ - From: queryRefID, - Metric: rule.Record, + From: queryRefID, + Metric: rule.Record, + TargetDatasourceUID: p.cfg.DatasourceUID, } isPaused = p.cfg.RecordingRules.IsPaused diff --git a/pkg/services/ngalert/prom/convert_test.go b/pkg/services/ngalert/prom/convert_test.go index 3365ba15e5c..2d7e2b26535 100644 --- a/pkg/services/ngalert/prom/convert_test.go +++ b/pkg/services/ngalert/prom/convert_test.go @@ -198,6 +198,7 @@ func TestPrometheusRulesToGrafana(t *testing.T) { require.NotNil(t, grafanaRule.Record) require.Equal(t, grafanaRule.Record.From, queryRefID) require.Equal(t, promRule.Record, grafanaRule.Record.Metric) + require.Equal(t, tc.config.DatasourceUID, grafanaRule.Record.TargetDatasourceUID) } else { require.Equal(t, fmt.Sprintf("[%s] %s", tc.promGroup.Name, promRule.Alert), grafanaRule.Title) } diff --git a/pkg/tests/api/alerting/api_convert_prometheus_test.go b/pkg/tests/api/alerting/api_convert_prometheus_test.go index 48f09332ebb..ec881bc09c5 100644 --- a/pkg/tests/api/alerting/api_convert_prometheus_test.go +++ b/pkg/tests/api/alerting/api_convert_prometheus_test.go @@ -108,7 +108,8 @@ func TestIntegrationConvertPrometheusEndpoints(t *testing.T) { EnableUnifiedAlerting: true, DisableAnonymous: true, AppModeProduction: true, - EnableFeatureToggles: []string{"alertingConversionAPI"}, + EnableFeatureToggles: []string{"alertingConversionAPI", "grafanaManagedRecordingRulesDatasources", "grafanaManagedRecordingRules"}, + EnableRecordingRules: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, gpath) @@ -219,7 +220,8 @@ func TestIntegrationConvertPrometheusEndpoints_UpdateRule(t *testing.T) { EnableUnifiedAlerting: true, DisableAnonymous: true, AppModeProduction: true, - EnableFeatureToggles: []string{"alertingConversionAPI"}, + EnableFeatureToggles: []string{"alertingConversionAPI", "grafanaManagedRecordingRulesDatasources", "grafanaManagedRecordingRules"}, + EnableRecordingRules: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, gpath) @@ -305,7 +307,8 @@ func TestIntegrationConvertPrometheusEndpoints_Conflict(t *testing.T) { EnableUnifiedAlerting: true, DisableAnonymous: true, AppModeProduction: true, - EnableFeatureToggles: []string{"alertingConversionAPI"}, + EnableFeatureToggles: []string{"alertingConversionAPI", "grafanaManagedRecordingRulesDatasources", "grafanaManagedRecordingRules"}, + EnableRecordingRules: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, gpath) @@ -392,7 +395,8 @@ func TestIntegrationConvertPrometheusEndpoints_CreatePausedRules(t *testing.T) { EnableUnifiedAlerting: true, DisableAnonymous: true, AppModeProduction: true, - EnableFeatureToggles: []string{"alertingConversionAPI"}, + EnableFeatureToggles: []string{"alertingConversionAPI", "grafanaManagedRecordingRulesDatasources", "grafanaManagedRecordingRules"}, + EnableRecordingRules: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path) @@ -507,7 +511,8 @@ func TestIntegrationConvertPrometheusEndpoints_FolderUIDHeader(t *testing.T) { EnableUnifiedAlerting: true, DisableAnonymous: true, AppModeProduction: true, - EnableFeatureToggles: []string{"alertingConversionAPI"}, + EnableFeatureToggles: []string{"alertingConversionAPI", "grafanaManagedRecordingRulesDatasources", "grafanaManagedRecordingRules"}, + EnableRecordingRules: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path) @@ -608,7 +613,8 @@ func TestIntegrationConvertPrometheusEndpoints_Delete(t *testing.T) { EnableUnifiedAlerting: true, DisableAnonymous: true, AppModeProduction: true, - EnableFeatureToggles: []string{"alertingConversionAPI"}, + EnableFeatureToggles: []string{"alertingConversionAPI", "grafanaManagedRecordingRulesDatasources", "grafanaManagedRecordingRules"}, + EnableRecordingRules: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, gpath) diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index d0d0a574443..118cb83b7de 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -295,6 +295,13 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { _, err = alertingSect.NewKey("max_attempts", "3") require.NoError(t, err) + if opts.EnableRecordingRules { + recordingRulesSect, err := cfg.NewSection("recording_rules") + require.NoError(t, err) + _, err = recordingRulesSect.NewKey("enabled", "true") + require.NoError(t, err) + } + if opts.LicensePath != "" { section, err := cfg.NewSection("enterprise") require.NoError(t, err) @@ -537,6 +544,7 @@ type GrafanaOpts struct { UnifiedStorageConfig map[string]setting.UnifiedStorageConfig GrafanaComSSOAPIToken string LicensePath string + EnableRecordingRules bool // When "unified-grpc" is selected it will also start the grpc server APIServerStorageType options.StorageType From 8cdc687abf96617f241407efecf8ee310a85992e Mon Sep 17 00:00:00 2001 From: Collin Fingar Date: Fri, 7 Mar 2025 11:12:07 -0500 Subject: [PATCH 22/39] i18n: Marking up Pagination and PageToolbar in Grafana-UI for translation (#101738) * Grafana-UI: Pagination + PageToolbar i18n * betterer results update for i18n fix * Linting fix w/ import order --- .betterer.results | 10 ---------- .../src/components/PageLayout/PageToolbar.tsx | 18 ++++++++++++++---- .../src/components/Pagination/Pagination.tsx | 8 ++++++-- public/locales/en-US/grafana.json | 10 ++++++++++ public/locales/pseudo-LOCALE/grafana.json | 10 ++++++++++ 5 files changed, 40 insertions(+), 16 deletions(-) diff --git a/.betterer.results b/.betterer.results index e26bf7cb08e..98094fcc972 100644 --- a/.betterer.results +++ b/.betterer.results @@ -615,16 +615,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] ], - "packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"] - ], - "packages/grafana-ui/src/components/Pagination/Pagination.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] - ], "packages/grafana-ui/src/components/PanelChrome/PanelContext.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] diff --git a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx index fa9a77c9d12..521b059ea8e 100644 --- a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx +++ b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx @@ -7,6 +7,7 @@ import { selectors } from '@grafana/e2e-selectors'; import { useStyles2 } from '../../themes/ThemeContext'; import { getFocusStyles } from '../../themes/mixins'; import { IconName } from '../../types'; +import { t } from '../../utils/i18n'; import { Icon } from '../Icon/Icon'; import { IconButton } from '../IconButton/IconButton'; import { Link } from '../Link/Link'; @@ -82,6 +83,15 @@ export const PageToolbar = memo( ); + const goBackLabel = t('grafana-ui.page-toolbar.go-back', 'Go back (Esc)'); + const searchParentFolderLabel = t( + 'grafana-ui.page-toolbar.search-parent-folder', + 'Search dashboard in the {{parent}} folder', + { parent } + ); + const searchDashboardNameLabel = t('grafana-ui.page-toolbar.search-dashboard-name', 'Search dashboard by name'); + const searchLinksLabel = t('grafana-ui.page-toolbar.search-links', 'Search links'); + return (