[SLOs] Try async creation of resources !! #192836

shahzad31 · 2024-09-13T10:14:33Z

Summary

Try async as much as possible while creating SLO !!

Also UI form won't wait now for creating burn rate rule, it will be created async loading state in toast !!

Before

After

obltmachine · 2024-09-13T10:14:49Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-09-13T11:44:42Z

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

…ate-perf

…into slo-create-perf

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts

x-pack/plugins/observability_solution/slo/server/services/summay_transform_manager.ts

x-pack/plugins/observability_solution/slo/server/services/slo_repository.ts

…ate-perf

kdelemme

Parallelizing the operations is a good improvement, but I think the change in the repository is not acceptable for the reason cited in the comments.

Happy to discuss more.
I might have one idea if we really want to split the exist assertion and the save operation (but including the save operation in the promise.all).

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts

x-pack/plugins/observability_solution/slo/server/services/slo_repository.ts

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts

…ate-perf

kdelemme

Thanks for the changes, I think we can leverage react query callback functions in the UI. Going to test locally now

x-pack/plugins/observability_solution/slo/server/services/slo_repository.ts

kdelemme · 2024-09-26T12:43:55Z

x-pack/plugins/observability_solution/slo/server/services/slo_repository.ts

+    return slo;
+  }
+
+  async update(slo: SLODefinition): Promise<SLODefinition> {


Technically this function can also create a new SLO 🙈

i think we can live with that :D

x-pack/plugins/observability_solution/slo/public/hooks/use_create_burn_rate_rule.tsx

kdelemme

One last comment about usage of the hook

...plugins/observability_solution/slo/public/pages/slo_edit/components/slo_edit_form_footer.tsx

…ate-perf

kdelemme

Just need to fix the loadingToastId typing and usage but otherwise LGTM.

kdelemme · 2024-09-27T14:01:38Z

x-pack/plugins/observability_solution/slo/public/hooks/use_create_burn_rate_rule.tsx

+  return useMutation<
+    CreateRuleResponse<Params> & { loadingToastId?: string },
    Error,
    { rule: CreateRuleRequestBody<Params> }
  >(


loadingToastId is not part of the Data response. it belongs to the context, which is the 4th type parameter.

Suggested change

return useMutation<

CreateRuleResponse<Params> & { loadingToastId?: string },

Error,

{ rule: CreateRuleRequestBody<Params> }

>(

return useMutation<

CreateRuleResponse<Params>,

Error,

{ rule: CreateRuleRequestBody<Params> },

{ loadingToastId: string }

>(

kdelemme · 2024-09-27T14:02:42Z

x-pack/plugins/observability_solution/slo/public/hooks/use_create_burn_rate_rule.tsx

+      onSettled: (data) => {
+        if (data?.loadingToastId) {
+          toasts.remove(data?.loadingToastId);
+        }
+      },


I wonder how this works locally because loadingToastId is in the context variable, which is the 4th argument of the onSettled function: https://tanstack.com/query/v4/docs/framework/react/guides/mutations#mutation-side-effects

data is what is returned by the mutate query function (the API call basically).

Suggested change

onSettled: (data) => {

if (data?.loadingToastId) {

toasts.remove(data?.loadingToastId);

}

},

onSettled: (_, _, _, context) => {

if (context?.loadingToastId) {

toasts.remove(context?.loadingToastId);

}

},

kdelemme · 2024-09-27T14:04:08Z

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts

+      rollbackOperations.push(() => this.transformManager.stop(rollupTransformId));
+      rollbackOperations.push(() => this.summaryTransformManager.stop(summaryTransformId));
+


…ate-perf

kibana-ci · 2024-09-30T09:10:30Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 0d88c2f
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-192836-0d88c2f06195

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`slo`	854.2KB	854.7KB	+538.0B

History

💔 Build #237716 failed 6257fb2
💛 Build #237639 was flaky 8b0f5da
💔 Build #237416 failed 6219a5a
💚 Build #237305 succeeded 62c20ce
💔 Build #236657 failed a7e18e4

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kdelemme

Thanks for the changes, tested locally and works as expected!

kibanamachine · 2024-10-01T15:33:53Z

Starting backport for target branches: 8.x

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

## Summary Try async as much as possible while creating SLO !! Also UI form won't wait now for creating burn rate rule, it will be created async loading state in toast !! ### Before ![image](https://github.com/user-attachments/assets/dc73ce58-3fc4-475f-a6ae-71ac98a6432c) ### After ![image](https://github.com/user-attachments/assets/4439bb8b-6768-4d8b-b208-1bad28f17ae5) (cherry picked from commit 9e117c3)

kibanamachine · 2024-10-01T15:38:11Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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.x`: - [[SLOs] Try async creation of resources !! (#192836)](#192836)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Shahzad <shahzad31comp@gmail.com>

try async

4273279

update jest tests

0ef1af1

shahzad31 marked this pull request as ready for review September 13, 2024 11:39

shahzad31 requested a review from a team as a code owner September 13, 2024 11:39

shahzad31 added the release_note:skip Skip the PR/issue when compiling release notes label Sep 13, 2024

Merge branch 'main' into slo-create-perf

60c97c9

botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Sep 13, 2024

shahzad31 added 4 commits September 13, 2024 16:24

Merge branch 'main' of https://github.com/elastic/kibana into slo-cre…

c19ed76

…ate-perf

retry test

bb84431

Merge branch 'slo-create-perf' of https://github.com/shahzad31/kibana …

eacead6

…into slo-create-perf

Merge branch 'main' into slo-create-perf

4896d88

kdelemme reviewed Sep 16, 2024

View reviewed changes

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts Outdated Show resolved Hide resolved

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts Show resolved Hide resolved

kdelemme reviewed Sep 16, 2024

View reviewed changes

x-pack/plugins/observability_solution/slo/server/services/summay_transform_manager.ts Outdated Show resolved Hide resolved

x-pack/plugins/observability_solution/slo/server/services/slo_repository.ts Outdated Show resolved Hide resolved

shahzad31 added 2 commits September 24, 2024 13:07

Merge branch 'main' of https://github.com/elastic/kibana into slo-cre…

26899ba

…ate-perf

PR feedback

8cafcab

shahzad31 added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Sep 24, 2024

fix tests

a7e18e4

kdelemme requested changes Sep 24, 2024

View reviewed changes

shahzad31 added 3 commits September 26, 2024 12:07

Merge branch 'main' of https://github.com/elastic/kibana into slo-cre…

59c211c

…ate-perf

more PR feedback

1e5fda8

update api tests

62c20ce

shahzad31 requested a review from kdelemme September 26, 2024 11:55

kdelemme requested changes Sep 26, 2024

View reviewed changes

kdelemme reviewed Sep 26, 2024

View reviewed changes

...plugins/observability_solution/slo/public/pages/slo_edit/components/slo_edit_form_footer.tsx Show resolved Hide resolved

shahzad31 added 3 commits September 26, 2024 16:11

Merge branch 'main' of https://github.com/elastic/kibana into slo-cre…

6edd717

…ate-perf

react hook pr feedback

6e83e3b

final version feedback

6219a5a

shahzad31 requested a review from kdelemme September 26, 2024 15:02

shahzad31 added 2 commits September 27, 2024 11:43

Merge branch 'main' of https://github.com/elastic/kibana into slo-cre…

7ff11dc

…ate-perf

update jest

8b0f5da

kdelemme requested changes Sep 27, 2024

View reviewed changes

shahzad31 added 3 commits September 27, 2024 16:14

Merge branch 'main' of https://github.com/elastic/kibana into slo-cre…

9e0da5e

…ate-perf

update types

6257fb2

Merge branch 'main' into slo-create-perf

0d88c2f

shahzad31 requested a review from kdelemme September 30, 2024 14:54

kdelemme approved these changes Oct 1, 2024

View reviewed changes

shahzad31 merged commit 9e117c3 into elastic:main Oct 1, 2024
23 checks passed

shahzad31 deleted the slo-create-perf branch October 1, 2024 15:33

kibanamachine added the v9.0.0 label Oct 1, 2024

kibanamachine mentioned this pull request Oct 1, 2024

[8.x] [SLOs] Try async creation of resources !! (#192836) #194609

Merged

kibanamachine added the v8.16.0 label Oct 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SLOs] Try async creation of resources !! #192836

[SLOs] Try async creation of resources !! #192836

shahzad31 commented Sep 13, 2024 •

edited by kibanamachine

Loading

obltmachine commented Sep 13, 2024

elasticmachine commented Sep 13, 2024

kdelemme left a comment

kdelemme left a comment

kdelemme Sep 26, 2024

shahzad31 Sep 26, 2024

kdelemme left a comment

kdelemme left a comment

kdelemme Sep 27, 2024

kdelemme Sep 27, 2024

kdelemme Sep 27, 2024

kibana-ci commented Sep 30, 2024 •

edited

Loading

kdelemme left a comment

kibanamachine commented Oct 1, 2024

kibanamachine commented Oct 1, 2024

		rollbackOperations.push(() => this.transformManager.stop(rollupTransformId));
		rollbackOperations.push(() => this.summaryTransformManager.stop(summaryTransformId));

[SLOs] Try async creation of resources !! #192836

[SLOs] Try async creation of resources !! #192836

Conversation

shahzad31 commented Sep 13, 2024 • edited by kibanamachine Loading

Summary

Before

After

obltmachine commented Sep 13, 2024

🤖 GitHub comments

elasticmachine commented Sep 13, 2024

kdelemme left a comment

Choose a reason for hiding this comment

kdelemme left a comment

Choose a reason for hiding this comment

kdelemme Sep 26, 2024

Choose a reason for hiding this comment

shahzad31 Sep 26, 2024

Choose a reason for hiding this comment

kdelemme left a comment

Choose a reason for hiding this comment

kdelemme left a comment

Choose a reason for hiding this comment

kdelemme Sep 27, 2024

Choose a reason for hiding this comment

kdelemme Sep 27, 2024

Choose a reason for hiding this comment

kdelemme Sep 27, 2024

Choose a reason for hiding this comment

kibana-ci commented Sep 30, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

History

kdelemme left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 1, 2024

kibanamachine commented Oct 1, 2024

💚 All backports created successfully

Questions ?

shahzad31 commented Sep 13, 2024 •

edited by kibanamachine

Loading

kibana-ci commented Sep 30, 2024 •

edited

Loading