[SLO] Enable burn rate alert by default during creation via UI #176317

simianhacker · 2024-02-06T15:54:42Z

Summary

This PR changes the SLO creation behavior via the UI. Instead of having a checkbox to create the Burn Rate rule, with this PR, the Burn Rate Rule will be created by default. The Burn Rate rule is only created by default when using the UI, the create SLO API does not create a rule by default.

- Adds useCreateRule hooks - Updates createWindow to use CreateSLOInput - Creates rule when SLO is created via UI

apmmachine · 2024-02-06T15:54:57Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

elasticmachine · 2024-02-06T16:28:27Z

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

paulb-elastic · 2024-02-06T16:31:12Z

Does the flow risk that burn rate rules will be created without connectors? Should this be called out somehow?

shahzad31 · 2024-02-06T16:31:21Z

x-pack/plugins/observability/public/hooks/use_create_rule.ts

+        const ruleId = v4();
+        const body = JSON.stringify(rule);
+        return http.post(`${http.basePath.prepend(BASE_ALERTING_API_PATH)}/rule/${ruleId}`, {
+          body,
+        });


i don't think we should be calling another plugin public API.
We should create a route and use AlertingClient exposed on server side to create an alert.

@XavierM thoughts?

If it's public, it should be ok to use them. We use at least 2 other RAC apis for fetching the rules and alerts.

dedemorton · 2024-02-06T17:58:44Z

It sounds like this change has a small impact on the documentation here. Can you open a docs issue or--if you feel inclined--open a docs PR to make sure the docs get updated? Thanks!

kdelemme · 2024-02-06T19:04:43Z

x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx

+      updateSlo({ sloId: slo.id, slo: processedValues });
+      navigate(basePath.prepend(paths.observability.slos));
    } else {
      const processedValues = transformCreateSLOFormToCreateSLOInput(values);
-
-      if (isCreateRuleCheckboxChecked) {
-        const { id } = await createSlo({ slo: processedValues });
-        navigate(
-          basePath.prepend(`${paths.observability.sloEdit(id)}?${CREATE_RULE_SEARCH_PARAM}=true`)
-        );
-      } else {
-        createSlo({ slo: processedValues });
-        navigate(basePath.prepend(paths.observability.slos));
-      }
+      const resp = await createSlo({ slo: processedValues });
+      await createBurnRateRule({
+        rule: createBurnRateRuleRequestBody({ ...processedValues, id: resp.id }),
+      });
+      navigate(basePath.prepend(paths.observability.slos));


Nice simplification

kdelemme · 2024-02-06T19:05:18Z

x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx

-      <BurnRateRuleFlyout
-        slo={slo as GetSLOResponse}
-        isAddRuleFlyoutOpen={isAddRuleFlyoutOpen}
-        canChangeTrigger={false}
-      />


We can know add the form into a flyout without much difficulty 👏🏻

simianhacker · 2024-02-06T20:27:47Z

@paulb-elastic We decided via Slack to go ahead and create the rule without connectors:

@grabowskit said:

I think it would be a mistake to create actions with a default alert rule. There is too many variables. I think it works best to create the alert rule, and let users edit it later to add actions later. I considered the idea of notifying users which alert rules don't have a connector, but it is probably unnecessary. Let's see what the user feedback looks like 🙂

kdelemme

Tested locally and work as expected

…by-default

kibana-ci · 2024-02-07T00:21:24Z

💚 Build Succeeded

Buildkite Build
Commit: d239a8b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`observability`	594	595	+1

Async chunks

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

id	before	after	diff
`observability`	625.6KB	634.7KB	+9.1KB

History

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

* main: (224 commits) [Http] Replace `buildNr` with `buildSha` in static asset paths (#175898) [Ops] Fix GCS bucket access for future buildkite agents (#174756) [api-docs] 2024-02-07 Daily api_docs build (#176362) skip flaky suite (#176002) skip failing es promotion suite (#176359) [Cloud Security] [Grouping] Add URL Params support to the grouping components (#175749) chore(NA): update versions after v8.12.2 bump (#176309) chore(NA): update versions after v7.17.19 bump (#176313) skip failing test suite (#176352) [SLO] Enable burn rate alert by default during creation via UI (#176317) [Fleet] Add the uptime capability to observability projects (#176285) [Security Solution][Endpoint] Fix Manifest Manger so that it works with large (>10k) (#174411) [ResponseOps] Alert creation delay based on user definition (#175851) [data views] Default field formatters based on field meta values (#174973) [Cloud Security]Detection Rules counter on Rules Flyout (#176041) [Security Solution] Data Quality Dashboard persistence (#175673) [Ent Search] Connector client copy cleanup (#176290) [ML] Anomaly Detection: Adds actions menu to anomaly markers in Single Metric Viewer chart. (#175556) [ML] Anomaly Detection: Fix `values-dots` colors (#176303) [Fleet] Logstash Output - being compliant to RFC-952 (#176298) ...

…ic#176317) ## Summary This PR changes the SLO creation behavior via the UI. Instead of having a checkbox to create the Burn Rate rule, with this PR, the Burn Rate Rule will be created by default. The Burn Rate rule is only created by default when using the UI, the create SLO API does not create a rule by default.

paulb-elastic · 2024-02-08T14:53:04Z

It sounds like this change has a small impact on the documentation here. Can you open a docs issue or--if you feel inclined--open a docs PR to make sure the docs get updated? Thanks!

Thank you for the prompt @dedemorton I've raised elastic/observability-docs#3600 for this

…ic#176317) ## Summary This PR changes the SLO creation behavior via the UI. Instead of having a checkbox to create the Burn Rate rule, with this PR, the Burn Rate Rule will be created by default. The Burn Rate rule is only created by default when using the UI, the create SLO API does not create a rule by default.

[SLO] Enable burn rate alert by default during creation via UI

a695f94

- Adds useCreateRule hooks - Updates createWindow to use CreateSLOInput - Creates rule when SLO is created via UI

simianhacker added release_note:enhancement Feature:SLO Team:obs-ux-management Observability Management User Experience Team v8.13.0 labels Feb 6, 2024

Updating tests

aeeb622

simianhacker marked this pull request as ready for review February 6, 2024 16:28

simianhacker requested a review from a team as a code owner February 6, 2024 16:28

shahzad31 reviewed Feb 6, 2024

View reviewed changes

kdelemme reviewed Feb 6, 2024

View reviewed changes

Fixing issue where the windows don't reflect what's saved

9dff5d8

simianhacker added 2 commits February 6, 2024 14:22

fixing a bug when creating the rule from SLO Action menu

6bea146

Removing basePath

2748650

kdelemme approved these changes Feb 6, 2024

View reviewed changes

simianhacker added 4 commits February 6, 2024 15:07

Removing obsolete translations

669559e

Removing obsolete translations

d976f7f

Fixing types for useCreateRule hook

fe55bb8

Merge branch 'main' of github.com:elastic/kibana into burn-rate-rule-…

d239a8b

…by-default

simianhacker enabled auto-merge (squash) February 6, 2024 22:59

simianhacker merged commit ca525c0 into elastic:main Feb 7, 2024
27 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Feb 7, 2024

paulb-elastic mentioned this pull request Feb 7, 2024

[SLO] Updated burn rate threshold value is not properly shown in the slo detail page #170605

Closed

paulb-elastic mentioned this pull request Feb 8, 2024

[SLO] Update docs to highlight that burn rate rule is automatically enabled now elastic/observability-docs#3600

Closed

simianhacker deleted the burn-rate-rule-by-default branch April 17, 2024 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SLO] Enable burn rate alert by default during creation via UI #176317

[SLO] Enable burn rate alert by default during creation via UI #176317

simianhacker commented Feb 6, 2024

apmmachine commented Feb 6, 2024

elasticmachine commented Feb 6, 2024

paulb-elastic commented Feb 6, 2024

shahzad31 Feb 6, 2024

simianhacker Feb 6, 2024

kdelemme Feb 6, 2024

dedemorton commented Feb 6, 2024

kdelemme Feb 6, 2024

kdelemme Feb 6, 2024

simianhacker commented Feb 6, 2024

kdelemme left a comment

kibana-ci commented Feb 7, 2024

paulb-elastic commented Feb 8, 2024

[SLO] Enable burn rate alert by default during creation via UI #176317

[SLO] Enable burn rate alert by default during creation via UI #176317

Conversation

simianhacker commented Feb 6, 2024

Summary

apmmachine commented Feb 6, 2024

🤖 GitHub comments

elasticmachine commented Feb 6, 2024

paulb-elastic commented Feb 6, 2024

shahzad31 Feb 6, 2024

Choose a reason for hiding this comment

simianhacker Feb 6, 2024

Choose a reason for hiding this comment

kdelemme Feb 6, 2024

Choose a reason for hiding this comment

dedemorton commented Feb 6, 2024

kdelemme Feb 6, 2024

Choose a reason for hiding this comment

kdelemme Feb 6, 2024

Choose a reason for hiding this comment

simianhacker commented Feb 6, 2024

kdelemme left a comment

Choose a reason for hiding this comment

kibana-ci commented Feb 7, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

paulb-elastic commented Feb 8, 2024