[RAM] Add example log system action type #175057
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Towards elastic/response-ops-team#164 Registering alerting example rules with framework AAD. This creates a new alerts index `.alerts-default.alerts-default` that will eventually hold alerts for all rules that have not customized their registration. This index contains only the mappings for the basic alerts as data documents, no custom context or payload fields. Run kibana using `--run-examples` flag. Create one of the example rule types and let it alert and then resolve and see an alert document get created in the `.alerts-default.alerts-default` index. --------- Co-authored-by: Kibana Machine <[email protected]>
Zacqary
added
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
XavierM
reviewed
Jan 22, 2024
...s_actions_ui_example/public/connector_types/system_log_example/system_log_example_params.tsx
Show resolved
Hide resolved
XavierM
reviewed
Jan 22, 2024
| body: transformResolveResponseV1<RuleParamsV1>(rule), | ||
| }; | ||
| return res.ok(response); | ||
| try { |
There was a problem hiding this comment.
why did we have to add the try/catch?
There was a problem hiding this comment.
This route was failing because some schemas were missing system actions support so I added this for debugging, but it's not needed anymore. I'll remove.
XavierM
reviewed
Jan 22, 2024
XavierM
reviewed
Jan 22, 2024
| frequency: DEFAULT_FREQUENCY, | ||
| uuid: uuidv4(), | ||
| }); | ||
| actions.push( |
There was a problem hiding this comment.
can we create a function to avoid duplication code?
There was a problem hiding this comment.
Pushing a simplified version of this control flow
I looked into the git blame to find out why DEFAULT_FREQUENCY was being set sometimes, and defaultRuleFrequency was being set other times, and it turns out it's due to a bad merge, so I'm just setting defaultRuleFrequency no matter what going forward. This may end up fixing a bug that we haven't yet detected.
JiaweiWu
reviewed
Jan 24, 2024
x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.tsx
Outdated
Show resolved
Hide resolved
JiaweiWu
reviewed
Jan 26, 2024
x-pack/plugins/actions/server/routes/connector/get_all/get_all.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+84
to
+91
| clonedRule.actions = clonedRule.actions.map((action: SanitizedRuleAction) => { | ||
| if (actionHasDefinedGroup(action as SanitizedRuleAction)) { | ||
| return { | ||
| ...action, | ||
| frequency, | ||
| } as SanitizedRuleAction; | ||
| } | ||
| return action as SanitizedRuleAction; |
There was a problem hiding this comment.
Let's add some jest tests to this as well, also curious to know why we have to cast the types so often, maybe worth trying to fix them, unless we're migrating types like we were the the http versioning stuff
There was a problem hiding this comment.
This migration code is for pre-8.6 rules, note the if (hasRuleLevelNotifyWhen || hasRuleLevelThrottle) check.
Adding this change here covers the highly unlikely edge case where a system action somehow gets added to a pre-8.6 rule that still has global frequency params before it's ever opened in the UI and migrated, and even then whatever frequency param we add would get stripped on rewrite. This was just to cover our bases in case something unexpected happens but I'm 99% sure the UI will function without problems if we don't push this. Happy to revert this code if it's a blocker.
...lugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx
Show resolved
Hide resolved
@XavierM For which API are you referring to? The connectors' APIs (Get all connectors API, Get connector API, List connector types API) do not return system actions. |
|
@cnasikas I think in this PR, we are using the Get all connectors API to get the system actions on the UI with a new query param to include them. I feel like we should have a different APIs for getting all or the detail of the system action. |
You are right @XavierM. You can see in the System Actions doc that this was the decision we took. We should not expose system actions to any of the connector's APIs and we should use separate internal APIs dedicated to system actions. |
|
I missed that in the doc, I just realized when reviewing the PR! |
|
@cnasikas I was reading over the System Actions doc and discussion but it was never clear where in the UI system actions actually do show up. Only that they're not supposed to be in the connectors list. I didn't have anywhere to put this example action except in the connectors list with everything else. I combined fetching system actions with the |
⏳ Build in-progress, with failuresFailed CI Steps
Test Failures
History
To update your PR or re-run it, just comment with: |
@XavierM Yea I was actually thinking something like this while reviewing this PR, it seems like the checks to determine whether or not its system action is going to be added everywhere. |
|
Pushed the following changes:
|
This was referenced Feb 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This adds an example action type called System Log, which is enabled when running Kibana with the
--run-examplesflag.This action will log out to Kibana at the same rate as summarized alerts.
Please only refer to 29fa70d when reviewing. Other code changes were cherry-picked from
main