feat(slo): bulk delete APIs by kdelemme · Pull Request #217405 · elastic/kibana

kdelemme · 2025-04-07T20:51:55Z

🍒 Summary

Related to #209925

This PR introduces a POST /_bulk_delete and GET /_bulk_delete/{taskId} APIs that leverage the task manager to run a bulk deletion. We reuse the Delete SLO application service as much as possible, while grouping the delete_by_query and bulkDeleteRule for all successfully deleted SLOs in order to reduce the number of sub-task (delete_by_query) created.

We keep the result of the task for 1 hour, so we can display the result on the frontend.

Manual testing

Create some SLOs
Bulk delete some of the SLO and some inexistant:

curl --request POST \
  --url http://localhost:5601/api/observability/slos/_bulk_delete \
  --header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
  --header 'Content-Type: application/json' \
  --header 'kbn-xsrf: oui' \
  --data '{
	"list": [
	"8030bf8e-b047-4af2-a696-afb6f6bf7813",
	"8465d14b-3054-4eed-94f5-6e904d59838a",
	"9e82240f-2f0c-4fa4-a37c-1f6f4627951d",
        "inexistant"
	]
}'

Get the bulk status by taskId

curl --request GET \
  --url http://localhost:5601/api/observability/slos/_bulk_delete/{taskId} \
  --header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
  --header 'Content-Type: application/json' \
  --header 'kbn-xsrf: oui'

elasticmachine · 2025-04-17T14:13:21Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

…apping'

Copilot

Pull Request Overview

This PR implements bulk delete APIs for SLOs and enhances the existing DeleteSLO service by adding options to skip specific deletion stages.

Updated DeleteSLO service to include options (skipDataDeletion and skipRuleDeletion) and refactored the deletion steps.
Introduced new bulk delete endpoints (POST and GET) along with corresponding OpenAPI documentation updates.
Made minor adjustments in the SLO creation service and test files to align with the revised deletion behavior.

Reviewed Changes

Copilot reviewed 21 out of 27 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
x-pack/solutions/observability/plugins/slo/server/services/delete_slo.ts	Adds options to control data and rule deletion, and adjusts deletion ordering and client usage.
x-pack/solutions/observability/plugins/slo/server/services/delete_slo.test.ts	Removes the unused ElasticsearchClient mock and updates expectations.
x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts	Updates repository deletion rollback to use the options object.
x-pack/solutions/observability/plugins/slo/server/routes/slo/route.ts	Incorporates the new bulk delete routes.
x-pack/solutions/observability/plugins/slo/server/routes/slo/delete_slo.ts	Refactors imports and client usage to use the current user’s scoped client.
x-pack/solutions/observability/plugins/slo/server/routes/slo/bulk_delete.ts	Introduces the bulk delete API endpoints and status checking.
x-pack/solutions/observability/plugins/slo/server/plugin.ts	Registers the new BulkDeleteTask and maps plugins accordingly.
YAML files under docs/openapi/slo	Adds OpenAPI definitions for the new bulk delete endpoints.
x-pack/platform/packages/shared/kbn-slo-schema	Adds bulk delete schemas and route definitions.

Files not reviewed (6)

oas_docs/output/kibana.serverless.yaml: Language not supported
oas_docs/output/kibana.yaml: Language not supported
x-pack/solutions/observability/plugins/slo/docs/openapi/slo/bundled.json: Language not supported
x-pack/solutions/observability/plugins/slo/docs/openapi/slo/bundled.yaml: Language not supported
x-pack/solutions/observability/plugins/slo/docs/openapi/slo/entrypoint.yaml: Language not supported
x-pack/solutions/observability/plugins/slo/server/services/snapshots/delete_slo.test.ts.snap: Language not supported

Comments suppressed due to low confidence (2)

x-pack/solutions/observability/plugins/slo/server/services/delete_slo.ts:96

Verify that setting 'refresh' to false in 'deleteSummaryData' is intentional, as this differs from the previous behavior where it was set to true.

refresh: false,

x-pack/solutions/observability/plugins/slo/server/services/delete_slo.ts:57

Confirm that moving the repository deletion step to the first Promise.all does not interfere with the subsequent data deletion operations.

this.repository.deleteById(slo.id, { ignoreNotFound: true }),

baileycash-elastic

lgtm

dmlemeshko

x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts changes LGTM

pmuellr

I left two comments to be considered. Not blockers, but will likely improve the overall aspect of the new task.

pmuellr · 2025-04-24T17:19:20Z

...ck/solutions/observability/plugins/slo/server/services/tasks/bulk_delete/bulk_delete_task.ts

+            },
+
+            cancel: async () => {
+              this.abortController.abort('Timed out');


This is the only reference to the abort controller, so basically it's a no-op. The controller will signal the abort, but with no code depending on it, nothing will happen. Suggest you pass it in to runBulkDelete and check it there, especially since it seems to be dealing with multiple operations ...

You're right. I wanted to use it but then noticed all the services used within runBulkDelete (especially deleteSLO) need to be updated to use an abortController, and stopped there. I'll refactor these services to use it.

pmuellr · 2025-04-24T17:27:17Z

x-pack/solutions/observability/plugins/slo/server/services/tasks/bulk_delete/run_bulk_delete.ts

+    .filter((result) => result.success === true)
+    .map((result) => result.id);
+
+  await Promise.all([


So there are 3 independent things going on here - two DBQ's run "in parallel", followed by bulk rule deletion.

We've had issues in the past when ES runs into problems "in the middle" of things that you would like to wrap in a "transaction", so it would be good to think about this for a few minutes. Relatedly, you may not want to do the DBQ's "in parallel", since either one of them could fail - is the order important?

The basic question to ask is - if one of these operations fails, what is the state of the system after that? Is it possible for a user to "try again" or "continue"? Would it take manual intervention (customer using DevTools to make some HTTP requests) to get things "working" again?

Relatedly, you may not want to do the DBQ's "in parallel", since either one of them could fail - is the order important?

The order is not important since I'm deleting the data only for the successfully deleted SLO (therefore their transforms are not running anymore, and no new data is being produced). if the DBQ fails during their execution (they run async, and only preflight checks is done synchronously), I think that's acceptable, but you're right that I should handle the potential failure to continue to the next step regardless.

The basic question to ask is - if one of these operations fails, what is the state of the system after that? Is it possible for a user to "try again" or "continue"? Would it take manual intervention (customer using DevTools to make some HTTP requests) to get things "working" again?

So yes, I think catching the DBQ errors and continuing is enough.
Worst case, we did not delete the data: the user can still do it manually or not, the system won't care.

So I'll add a catch on these promises

kdelemme · 2025-04-24T18:39:42Z

@pmuellr I've changed the flow to catch potential errors while scheduling DBQs, and i'm using the abortController in most services

elasticmachine · 2025-04-24T18:44:39Z

💔 Build Failed

Buildkite Build
Commit: 7d7f4e5

Failed CI Steps

Quick Checks

History

💚 Build #295380 succeeded c402a64
💛 Build #295052 was flaky d4cd98b
💔 Build #294946 failed f76ef85
💔 Build #294592 failed 7914848
💔 Build #294402 failed 999078b

kibanamachine · 2025-04-25T00:11:42Z

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/14654066416

kibanamachine · 2025-04-25T00:17:51Z

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts You might need to backport the following PRs to 8.19: - [SLO] Bulk Purge SLI data (#218287) - [Cloud Connector] Add cloud_connectors config in Agentless API (#215421)

Manual backport

To create the backport manually run:

node scripts/backport --pr 217405

Questions ?

Please refer to the Backport tool documentation

(cherry picked from commit 1882697) # Conflicts: # x-pack/solutions/observability/plugins/slo/tsconfig.json

kdelemme · 2025-04-25T19:35:43Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.19`: - [feat(slo): bulk delete APIs (#217405)](#217405)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

kdelemme force-pushed the poc/bulk_delete branch from 64181a3 to a23c287 Compare April 7, 2025 21:11

kdelemme changed the title ~~poc: bulk delete task~~ feat(slo): bulk delete APIs Apr 17, 2025

kdelemme force-pushed the poc/bulk_delete branch from 21cf7d4 to 5ced909 Compare April 17, 2025 14:04

kdelemme marked this pull request as ready for review April 17, 2025 14:11

kdelemme requested review from a team as code owners April 17, 2025 14:11

kdelemme added 7 commits April 17, 2025 10:13

poc: bulk delete task

33bdaa9

poc: runBulkDelete

a35a303

wip

787ca49

wip: to cleanup

419dfb1

Reuse deleteSLO application service

29fed9f

Handle pending task

86bc86d

Add test

d8cdf01

kdelemme force-pushed the poc/bulk_delete branch from 5ced909 to d8cdf01 Compare April 17, 2025 14:13

botelastic bot added the Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. label Apr 17, 2025

kdelemme added release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Apr 17, 2025

kibanamachine and others added 10 commits April 17, 2025 14:22

[CI] Auto-commit changed files from 'node scripts/styled_components_m…

4cd3532

…apping'

Add docs

8e1ffd8

Fix test

c0597cd

[CI] Auto-commit changed files from 'make api-docs'

bcdf0a1

Remove transform helper usage

44a87a4

Merge branch 'main' into poc/bulk_delete

999078b

wip

342fe7b

Merge branch 'main' into poc/bulk_delete

4520366

Fix task using latest api

2de0a3d

Remove basepath config

4e24f6c

kdelemme and others added 2 commits April 21, 2025 09:09

fix import

360506f

[CI] Auto-commit changed files from 'node scripts/styled_components_m…

7914848

…apping'

kdelemme requested a review from Copilot April 21, 2025 14:37

Copilot AI reviewed Apr 21, 2025

View reviewed changes

Fix task registered type

1a216b9

kdelemme requested a review from a team as a code owner April 21, 2025 14:53

kdelemme and others added 5 commits April 22, 2025 10:30

Merge branch 'main' into poc/bulk_delete

f76ef85

Merge branch 'main' into poc/bulk_delete

d4cd98b

Merge branch 'main' into poc/bulk_delete

c8bf908

Use getScopedClients

1f2ea8a

[CI] Auto-commit changed files from 'make api-docs'

c402a64

baileycash-elastic approved these changes Apr 23, 2025

View reviewed changes

dmlemeshko approved these changes Apr 23, 2025

View reviewed changes

pmuellr approved these changes Apr 24, 2025

View reviewed changes

kdelemme added 2 commits April 24, 2025 14:06

Catch errors on data deletion queries

488ac85

Use AbortController

7d7f4e5

Merge branch 'main' into poc/bulk_delete

484fc48

kdelemme merged commit 1882697 into elastic:main Apr 25, 2025
9 checks passed

kdelemme added a commit to kdelemme/kibana that referenced this pull request Apr 25, 2025

feat(slo): bulk delete APIs (elastic#217405)

4d0a514

(cherry picked from commit 1882697) # Conflicts: # x-pack/solutions/observability/plugins/slo/tsconfig.json

kdelemme mentioned this pull request Apr 25, 2025

[8.19] feat(slo): bulk delete APIs (#217405) #219308

Merged

akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025

feat(slo): bulk delete APIs (elastic#217405)

936f7b0

Conversation

kdelemme commented Apr 7, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🍒 Summary

Manual testing

Uh oh!

elasticmachine commented Apr 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

baileycash-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

dmlemeshko left a comment

Choose a reason for hiding this comment

Uh oh!

pmuellr left a comment

Choose a reason for hiding this comment

Uh oh!

pmuellr Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

pmuellr Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme commented Apr 24, 2025

Uh oh!

elasticmachine commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

History

Uh oh!

Uh oh!

kibanamachine commented Apr 25, 2025

Uh oh!

kibanamachine commented Apr 25, 2025

💔 All backports failed

Manual backport

Questions ?

Uh oh!

kdelemme commented Apr 25, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kdelemme commented Apr 7, 2025 •

edited by kibanamachine

Loading

elasticmachine commented Apr 24, 2025 •

edited

Loading