[RAM] Add data model for scheduled and recurring snoozes #131019

Zacqary · 2022-04-26T20:40:50Z

Summary

Migrates the data model for rules from ~~muteAll~~/snoozeEndTime to ~~snoozeIndefinitely~~/snoozeSchedule.

~~snoozeIndefinitely is a straight rename of muteAll and functions the same way.~~ (Removed this as it's too large of a code change; muteAll is used in a ton of places)

snoozeSchedule is an array of scheduled snooze times, including options for recurring on a time interval or certain days of the week. Recurrence is formatted in the form of an iCal RRULE, and this PR adds the rrule package to parse these. Also adds luxon which is its required peer dependency for dealing with time zones.

This just changes the data model and the task manager's parsing of it. APIs and UIs remain broadly unchanged for now.

The only change to the APIs and UIs are the addition of an is_snoozed_until property in the API response. This property is identical to the previously existing snooze_end_time, but it gets derived on the server side from the snooze schedule using the getRuleSnoozeEndTime function. The reason we do this is because making the rrule package available in the frontend balloons the alerting plugin size, so all RRULE parsing needs to happen on the server. This may not last, as we're eventually going to need to generate RRULE strings from the snooze UI, but I don't want to include the decision to increase the alerting bundle size in this PR.

UPDATE 5/12: To aid in filtering, isSnoozedUntil will now be stored in the SavedObject. This property is computed and added to the SavedObject either when the _snooze API is called (for snoozes that start as soon as they're added), OR when the task runner attempts to execute the rule (for recurring snoozes). As far as the task runner is concerned, the actual snooze source of truth will always be the snoozeSchedule. isSnoozedUntil will purely be for display and sorting.

UPDATE 5/16: Closes #132164 and uses rRule even for simple snoozes. When the user clicks "Snooze for 3 days" this will now set a snooze with an rRule of { dtstart: NOW, count: 1 }

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…-datamodel # Conflicts: # x-pack/plugins/alerting/server/saved_objects/migrations.ts # x-pack/plugins/alerting/server/task_runner/task_runner.ts

x-pack/plugins/alerting/common/is_rule_snoozed.test.ts

…-datamodel

…cqary/kibana into 130724-snooze-sched-datamodel

…-datamodel

…-datamodel # Conflicts: # x-pack/plugins/alerting/server/saved_objects/mappings.json

…-datamodel # Conflicts: # package.json # yarn.lock

…-datamodel # Conflicts: # x-pack/plugins/alerting/server/lib/index.ts # x-pack/plugins/alerting/server/routes/find_rules.ts # x-pack/plugins/alerting/server/rules_client/rules_client.ts

Zacqary · 2022-05-12T18:08:42Z

Updated the description with this but reposting here:

To aid in filtering, isSnoozedUntil will now be stored in the SavedObject. This property is computed and added to the SavedObject either when the _snooze API is called (for snoozes that start as soon as they're added), OR when the task runner attempts to execute the rule (for recurring snoozes). As far as the task runner is concerned, the actual snooze source of truth will always be the snoozeSchedule. isSnoozedUntil will purely be for display and sorting.

pmuellr

Added some comments. It feels like we have too many combinations of properties that can be used to define "mute all forever", "mute all with a duration", and the eventual rRule-based snoozes. Wondering if we can boil this down to a simpler set of properties, where we ALWAYS have an rRule in place.

I could totally believe we can't! I just need to be convinced.

And I think we need a bit of doc in the rule_snooze_type module describing these properties, as well as some doc in the PR describing all the combinations of things that can happen today, with an hint of how we plan on dealing with rRule schedules in the future.

yarn.lock

x-pack/plugins/alerting/common/rule_snooze_type.ts

x-pack/plugins/alerting/server/rules_client/rules_client.ts

pmuellr · 2022-05-12T22:40:39Z

x-pack/plugins/alerting/server/rules_client/rules_client.ts

@@ -2817,3 +2860,9 @@ function parseDate(dateString: string | undefined, propertyName: string, default

  return parsedDate;
 }
+
+function clearUnscheduledSnooze(attributes: { snoozeSchedule?: RuleSnooze }) {


Not completely clear what this function is doing, but I think it's removing "mute all" snoozes that have a duration.

Not exactly. This clears any "simple" snoozes, i.e. the user selects "Snooze for 3 days (starting now)" in the currently-existing UX, instead of e.g. "Snooze on Friday for 3 days" or "Snooze every Friday for 3 days" as made possible by the new functionality. It's the functional equivalent of setting snoozeEndTime: null in the previous version of the data model.

Ah ... somehow we need to get all this documented, at least for ourselves. Doesn't need to be this PR though, in the spirit of time. I added a request to add doc to #132266 ...

…-datamodel

pmuellr

LGTM; noted some things for follow ups - more doc (somewhere) and more robust parsing for the wacky day-of-week with optional numeric suffix thing (probably could be made into a grep pattern).

x-pack/plugins/alerting/common/rule_snooze_type.ts

pmuellr · 2022-05-17T19:02:59Z

x-pack/plugins/alerting/server/rules_client/rules_client.ts

@@ -2817,3 +2860,9 @@ function parseDate(dateString: string | undefined, propertyName: string, default

  return parsedDate;
 }
+
+function clearUnscheduledSnooze(attributes: { snoozeSchedule?: RuleSnooze }) {


Ah ... somehow we need to get all this documented, at least for ourselves. Doesn't need to be this PR though, in the spirit of time. I added a request to add doc to #132266 ...

pmuellr · 2022-05-17T19:45:14Z

x-pack/plugins/alerting/server/lib/is_rule_snoozed.ts

+function parseByWeekday(byweekday: Array<string | number>): ByWeekday[] {
+  return byweekday.map((w) => {
+    if (typeof w === 'number') return w;
+    if (['SU', 'MO', 'TU', 'WE', 'TH', 'FR', 'SA'].includes(w)) return w as WeekdayStr;


nit: you could create a const Set for these strings at the top-level, and then just use WeekdaySet.has(w)

pmuellr · 2022-05-17T19:51:18Z

x-pack/plugins/alerting/server/lib/is_rule_snoozed.ts

+  return byweekday.map((w) => {
+    if (typeof w === 'number') return w;
+    if (['SU', 'MO', 'TU', 'WE', 'TH', 'FR', 'SA'].includes(w)) return w as WeekdayStr;
+    const dayOfWeek = w.slice(-2) as WeekdayStr;


Feels like we may need a more robust parser for these BYDAY values - I had to look it up to make sure I was understanding this code :-). Who proposed using this wacky format!!! :-). https://icalendar.org/iCalendar-RFC-5545/3-3-10-recurrence-rule.html

Yeah unfortunately the rrule library will parse an entire rrule string, but not just a segment of it, so we can't rely on its already robust parsing of BYDAY. I added a comment though.

Not that we need to do it for this PR, but could we construct a legal but fake rRule with the BYDAY value, then send it to the rRule parser to just extract the relevant fields?

Oh wait actually yeah that does work

No, may have spoken too soon, the parsed RRule object only seems to return its byweekday in the form of an array of numbers, and I'm not sure how to extract an nth day of week from it. Still playing with it though

Oops nevermind had to use origOptions, this works fine

…-datamodel

…-ref HEAD~1..HEAD --fix'

kibana-ci · 2022-05-18T17:07:55Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`alerting`	26	27	+1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`alerting`	330	349	+19

Async chunks

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

id	before	after	diff
`triggersActionsUi`	782.0KB	782.0KB	+57.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`triggersActionsUi`	103.9KB	103.9KB	+38.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id	before	after	diff
`alert`	54	74	+20

Unknown metric groups

API count

id	before	after	diff
`alerting`	339	358	+19

History

💔 Build #45574 failed 25f8f7d
💔 Build #45475 failed f8fe600
💔 Build #45441 failed ffeb94d
💔 Build #45119 failed 409c606
💚 Build #45009 succeeded 083782c

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

Zacqary added 3 commits April 26, 2022 15:16

[RAM] Add data model for scheduled and recurring snoozes

a98faba

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

fdbe1b5

…-datamodel # Conflicts: # x-pack/plugins/alerting/server/saved_objects/migrations.ts # x-pack/plugins/alerting/server/task_runner/task_runner.ts

Update migration tests

bc20443

Zacqary added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX v8.3.0 labels Apr 26, 2022

Zacqary commented Apr 26, 2022

View reviewed changes

x-pack/plugins/alerting/common/is_rule_snoozed.test.ts Outdated Show resolved Hide resolved

Zacqary and others added 22 commits April 26, 2022 16:15

Make snoozeIndefinitely required

de7a37f

Fix remaining muteAlls

7fec109

Replace snoozeEndTime with snoozeSchedule

ab4f21b

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

27c5702

…-datamodel

Fix typecheck

29173d8

Merge branch 'main' into 130724-snooze-sched-datamodel

0822788

Fix typecheck

274c0c0

Merge branch '130724-snooze-sched-datamodel' of https://github.com/Za…

cabd5c3

…cqary/kibana into 130724-snooze-sched-datamodel

Revert muteAll => snoozeIndefinitely rename

7c918a5

Revert more snoozeIndefinitely refs

5b57cb8

Revert README

5df3868

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

7f8acf1

…-datamodel

Restore updated taskrunner test

d128eac

Fix RuleStatusDropdown test

59c5a1b

Add timeZone to SO

03d2d29

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

32344fe

…-datamodel # Conflicts: # x-pack/plugins/alerting/server/saved_objects/mappings.json

Update timezone usage

e3c28e8

Implement RRule

b93d460

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

f9dfc6e

…-datamodel # Conflicts: # package.json # yarn.lock

Fix task runner test

370fd45

Add rrule types

6dda9a2

Push snoozeEndTime from server and fix unsnooze

128950f

Zacqary added 5 commits May 11, 2022 15:11

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

92511d2

…-datamodel # Conflicts: # x-pack/plugins/alerting/server/lib/index.ts # x-pack/plugins/alerting/server/routes/find_rules.ts # x-pack/plugins/alerting/server/rules_client/rules_client.ts

Fix jest

2434080

Fix snoozedUntil date parsing

70a2aa7

Fix rewriterule

3646458

Add error handling for RRule

2c53cb7

Zacqary requested a review from XavierM May 12, 2022 18:08

Zacqary mentioned this pull request May 12, 2022

[RAM] Strongly type RRule in rule snooze SavedObjects #132164

Closed

pmuellr reviewed May 12, 2022

View reviewed changes

Zacqary added 3 commits May 16, 2022 11:27

Fix re-snoozing

ed75133

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

20c37dc

…-datamodel

Add comments to rulesnoozetype

083782c

Zacqary mentioned this pull request May 16, 2022

[RAM] Simplify snooze data model to remove muteAll #132266

Open

Zacqary added 5 commits May 16, 2022 16:29

Restructure data model to use rRule for everything

f11125d

Fix functional tests

409c606

Fix jest

eb1f391

Fix functional tests

ffeb94d

Fix functional tests

f8fe600

pmuellr approved these changes May 17, 2022

View reviewed changes

Zacqary added 2 commits May 17, 2022 19:34

Fix functional tests

22e839e

Clarify isRuleSnoozed

25f8f7d

Zacqary enabled auto-merge (squash) May 18, 2022 00:42

Merge remote-tracking branch 'upstream/main' into 130724-snooze-sched…

3edaa0c

…-datamodel

Zacqary disabled auto-merge May 18, 2022 15:48

Use rrule to parse byweekday

02baeea

Zacqary enabled auto-merge (squash) May 18, 2022 15:59

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

07bb7e9

…-ref HEAD~1..HEAD --fix'

Zacqary merged commit 64689c0 into elastic:main May 18, 2022

kibanamachine added the backport:skip This commit does not require backporting label May 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAM] Add data model for scheduled and recurring snoozes #131019

[RAM] Add data model for scheduled and recurring snoozes #131019

Zacqary commented Apr 26, 2022 •

edited

Loading

Zacqary commented May 12, 2022

pmuellr left a comment

pmuellr May 12, 2022

Zacqary May 16, 2022

pmuellr May 17, 2022

pmuellr left a comment

pmuellr May 17, 2022

pmuellr May 17, 2022

pmuellr May 17, 2022

Zacqary May 18, 2022

pmuellr May 18, 2022

Zacqary May 18, 2022

Zacqary May 18, 2022

Zacqary May 18, 2022

kibana-ci commented May 18, 2022

API count

[RAM] Add data model for scheduled and recurring snoozes #131019

[RAM] Add data model for scheduled and recurring snoozes #131019

Conversation

Zacqary commented Apr 26, 2022 • edited Loading

Summary

Checklist

Zacqary commented May 12, 2022

pmuellr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented May 18, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

Saved Objects .kibana field count

API count

History

Zacqary commented Apr 26, 2022 •

edited

Loading