[Response Ops][Actions] Keep track of action execution source and show in connector exec log #152030

ymao1 · 2023-02-23T19:30:10Z

Summary

This PR persists an execution source field to the action event log execute document which indicates the source of the action. Currently, we have 3 sources of action execution:

http_request - these are executions triggered from the _execute route in the actions API. This is used by the test connector functionality in the UI but could also be used directly via curl command. We do not differentiate between the two.
notification - these are executions triggered by the notifications service. currently, this only occurs when a case is assigned to a user.
saved_object - these are executions triggered by another saved object, like the alert saved object. Cases also triggers action execution in x-pack/plugins/cases/server/client/cases/push.ts. The source will be saved as the type of the saved object that triggered the action

After persisting the source, we also updated the connector exec log to show the source in a new column (hidden by default).

To Verify

Create a connector and run it via the Test Connector tab
Create a rule that will fire an action using the connector and let it run
Create a case an assign it to a user (other than yourself)
Go to the connector exec log view, add the "Source" column and see that it is populated. You can also inspect the event log docs for provider: "actions" and see that kibana.action.execution.source is populated.

…ce type in action task params. Writing source to event log

…on-source

ymao1 · 2023-02-27T14:49:42Z

x-pack/plugins/actions/server/saved_objects/mappings.json

@@ -45,6 +45,10 @@
      "relatedSavedObjects": {
        "enabled": false,
        "type": "object"
+      },
+      "source": {


Adding source field but not indexing it because we don't need to query on it. Also not adding a migration because this is not a required field.

Do you really need to add it explicitly to the mappings then?

I thought we had dynamic: strict for saved object mappings so we had to identify all the fields even if we didn't need to map them? If that's not the case, I am happy to remove!

We're using dynamic:strict at the root level, but you can add dynamic: false at the top level of your type's mapping to override it. I thought it was already the case for response ops types tbh.

It looks like that is the case for the cases SO type but not for action, action_task_params or alert SO types. Is that the preferred method? I can add it to action_task_params in this PR and create a separate PR for the other SO types if so

I think check w/ @XavierM - he's been working on fine-tuning the mappings for dynamic false; he probably has a feel whether we should do these adhoc or wait and do them all at once.

We did recently just add a dynamic: false to an "inner" struct in the rule SO, and I posted some comments on it. https://github.com/elastic/kibana/pull/148038/files#r1091213922

he probably has a feel whether we should do these adhoc or wait and do them all at once

Doing it in another PR works for me, as long as we make sure this new field is in the radar of the other PR/PR author.

I'll approve to unblock and will let you figure out how/when you want to do the field removal.

Discussed with Xavier and I will add dynamic:false to the action_task_params mappings in this PR, he will make note of the new source field in his PR: #152857 which will also set dynamic:false for the other Response Ops SO types.

ymao1 · 2023-02-27T16:17:10Z

x-pack/plugins/actions/server/constants/event_log.ts

@@ -9,6 +9,5 @@ export const EVENT_LOG_PROVIDER = 'actions';
 export const EVENT_LOG_ACTIONS = {
  execute: 'execute',
  executeStart: 'execute-start',
-  executeViaHttp: 'execute-via-http',


Typically different event log actions means a different document is written out (we write out separate docs for execute and execute-start. In this case, it doesn't seem to make sense to write out a different doc for execute-via-http and it made more sense to store the source in the execute doc. This value was unused anyway, so I removed it.

…to actions/execution-source

elasticmachine · 2023-02-27T18:12:02Z

Pinging @elastic/response-ops (Team:ResponseOps)

ymao1 · 2023-02-27T18:45:06Z

@lcawl What do you think would be more user friendly text to show in the UI instead of HTTP_REQUEST, NOTIFICATION and SAVED_OBJECT?

lcawl · 2023-03-01T19:16:58Z

@lcawl What do you think would be more user friendly text to show in the UI instead of HTTP_REQUEST, NOTIFICATION and SAVED_OBJECT?

I think that depends on who we think will make use of this information. If it's for debugging purposes only and is hidden by default, then it's less important to clarify their meanings. If, however, we want users to make sense of them, IMO we can keep the keywords but add a legend to explain their meanings (i.e. akin to what you've put in the PR summary). This legend/explanation could exist in a tooltip or popover on that column. If you choose to go that route, I can help with the text.

…on-source

ymao1 · 2023-03-01T21:14:55Z

I think that depends on who we think will make use of this information. If it's for debugging purposes only and is hidden by default, then it's less important to clarify their meanings. If, however, we want users to make sense of them, IMO we can keep the keywords but add a legend to explain their meanings (i.e. akin to what you've put in the PR summary). This legend/explanation could exist in a tooltip or popover on that column. If you choose to go that route, I can help with the text.

Thanks @lcawl! With that feedback, I will leave it as is but lowercase the word so it doesn't sound like it's shouting at you.

…fix'

ymao1 · 2023-03-06T13:09:54Z

@elasticmachine merge upstream

pmuellr

LGTM - testing all the source styles works as expected in the UI and event log.

I didn't see an obvious FT for the notification source; if we can add one easily, would be nice to have here, otherwise create an issue to add one?

pmuellr · 2023-03-07T22:07:33Z

x-pack/plugins/actions/server/lib/task_runner_factory.ts

@@ -7,19 +7,17 @@

 import { v4 as uuidv4 } from 'uuid';
 import { pick } from 'lodash';
-import { pipe } from 'fp-ts/lib/pipeable';
-import { map, fromNullable, getOrElse } from 'fp-ts/lib/Option';


thank you :-)

pmuellr · 2023-03-07T22:15:34Z

x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts

@@ -95,6 +96,7 @@ export default function ({ getService }: FtrProviderContext) {
        outcome: 'success',
        message: `action executed: test.index-record:${createdAction.id}: My action`,
        startMessage: `action started: test.index-record:${createdAction.id}: My action`,
+        source: ActionExecutionSourceType.HTTP_REQUEST,


Doesn't seem like we have explicit tests for notification? I imagine there must be a cases FT to test changing assignees that presumably would check to make sure the notification was attempted. That we could add an event log check to. If not, feels like we should open an issue to add one, unless it's super-simple to do in this PR.

I looked up the PR for notifications and there's a comment in the description that there were no integration tests: #144391

I did not add integration tests due to the complexity of simulating an email server. I added unit test coverage. If integration test coverage is needed we can add the tests on another PR.

I can create a followup issue for it.

Ya, please do, and reference that PR. I think we have a fairly simple way of simulating an email server, maybe it's not appropriate for this behavior though.

Created issue here: #152950

pgayvallet

See #152030 (comment)

…to actions/execution-source

…on-source

kibana-ci · 2023-03-08T20:15:12Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 0318ba9

Failed CI Steps

FTR Configs #46

Test Failures

[job] [logs] FTR Configs #46 / Cases cases list filtering assignees filtering filters cases by the first cases all user assignee

Metrics [docs]

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
`actions`	251	254	+3

Async chunks

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

id	before	after	diff
`triggersActionsUi`	1.4MB	1.4MB	+240.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`actions`	24	26	+2

Unknown metric groups

API count

id	before	after	diff
`actions`	256	259	+3

ESLint disabled line counts

id	before	after	diff
`securitySolution`	431	433	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	508	510	+2

History

💔 Build #112772 failed 3337e33
💛 Build #112033 was flaky ce7e26c
💚 Build #111536 succeeded 8400bcc
💚 Build #110732 succeeded 705d5f8
💔 Build #110679 failed 8e270b4

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

cc @ymao1

…w in connector exec log (elastic#152030) Resolves elastic#143984 ## Summary This PR persists an execution source field to the action event log `execute` document which indicates the source of the action. Currently, we have 3 sources of action execution: * `http_request` - these are executions triggered from the `_execute` route in the actions API. This is used by the test connector functionality in the UI but could also be used directly via curl command. We do not differentiate between the two. * `notification` - these are executions triggered by the notifications service. currently, this only occurs when a case is assigned to a user. * `saved_object` - these are executions triggered by another saved object, like the `alert` saved object. Cases also triggers action execution in `x-pack/plugins/cases/server/client/cases/push.ts`. The source will be saved as the type of the saved object that triggered the action After persisting the source, we also updated the connector exec log to show the source in a new column (hidden by default). <img width="1710" alt="Screenshot 2023-03-01 at 4 12 35 PM" src="https://user-images.githubusercontent.com/13104637/222266192-2877ad2b-0309-408e-ad44-ec37ef597afb.png"> ## To Verify 1. Create a connector and run it via the Test Connector tab 2. Create a rule that will fire an action using the connector and let it run 3. Create a case an assign it to a user (other than yourself) 4. Go to the connector exec log view, add the "Source" column and see that it is populated. You can also inspect the event log docs for `provider: "actions"` and see that `kibana.action.execution.source` is populated. --------- Co-authored-by: kibanamachine <[email protected]>

ymao1 added 11 commits February 23, 2023 14:06

Requiring source in action executor execute function. Persisting sour…

53ed1fa

…ce type in action task params. Writing source to event log

Merge

0da4e47

Fixing unit tests

7e7c492

Fixing types and tests

10f09a0

Merge branch 'main' of github.com:elastic/kibana into actions/executi…

56b0869

…on-source

Making sourceType optional

9c43d47

Merge branch 'main' of github.com:elastic/kibana into actions/executi…

963ddd2

…on-source

Showing source on exec log

1114477

Fixing tests

bfe1318

Merge branch 'main' of github.com:elastic/kibana into actions/executi…

1da3486

…on-source

Adding functional tests

885c1de

ymao1 changed the title ~~Actions/execution source~~ [Response Ops][Actions] Keep track of action execution source and show in connector exec log Feb 27, 2023

dont need to index new field

8e270b4

ymao1 commented Feb 27, 2023

View reviewed changes

cleanup

6a523cc

ymao1 commented Feb 27, 2023

View reviewed changes

ymao1 added 2 commits February 27, 2023 11:23

cleanup

79febfc

Merge branch 'actions/execution-source' of github.com:ymao1/kibana in…

705d5f8

…to actions/execution-source

ymao1 self-assigned this Feb 27, 2023

ymao1 added Feature:Actions Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.8.0 release_note:skip Skip the PR/issue when compiling release notes labels Feb 27, 2023

ymao1 marked this pull request as ready for review February 27, 2023 18:11

ymao1 requested review from a team as code owners February 27, 2023 18:11

Merge branch 'main' of github.com:elastic/kibana into actions/executi…

4a548f6

…on-source

Differentiating between saved object sources and lowercasing the string

91209de

ymao1 force-pushed the actions/execution-source branch from 2d6b8a4 to 91209de Compare March 1, 2023 21:20

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

8400bcc

…fix'

Merge branch 'main' into actions/execution-source

ce7e26c

pmuellr approved these changes Mar 7, 2023

View reviewed changes

pgayvallet approved these changes Mar 8, 2023

View reviewed changes

ymao1 added 5 commits March 8, 2023 10:26

Merging in main

7ddb632

Adding dynamic false to action task param mappings

af17277

Merge branch 'actions/execution-source' of github.com:ymao1/kibana in…

3337e33

…to actions/execution-source

Fixing test

6b3609b

Merge branch 'main' of github.com:elastic/kibana into actions/executi…

0318ba9

…on-source

ymao1 mentioned this pull request Mar 8, 2023

[Response Ops][Cases] Add functional tests for notifying assignee when assigned to case #152950

Open

ymao1 merged commit ca3c0dc into elastic:main Mar 8, 2023

kibanamachine added the backport:skip This commit does not require backporting label Mar 8, 2023

ymao1 deleted the actions/execution-source branch March 8, 2023 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Response Ops][Actions] Keep track of action execution source and show in connector exec log #152030

[Response Ops][Actions] Keep track of action execution source and show in connector exec log #152030

ymao1 commented Feb 23, 2023 •

edited

Loading

ymao1 Feb 27, 2023

pgayvallet Mar 7, 2023

ymao1 Mar 7, 2023

pgayvallet Mar 7, 2023

ymao1 Mar 7, 2023

pmuellr Mar 7, 2023

pgayvallet Mar 8, 2023

ymao1 Mar 8, 2023

ymao1 Feb 27, 2023

elasticmachine commented Feb 27, 2023

ymao1 commented Feb 27, 2023

lcawl commented Mar 1, 2023

ymao1 commented Mar 1, 2023

ymao1 commented Mar 6, 2023

pmuellr left a comment

pmuellr Mar 7, 2023

pmuellr Mar 7, 2023

ymao1 Mar 7, 2023

pmuellr Mar 8, 2023

ymao1 Mar 8, 2023

pgayvallet left a comment

kibana-ci commented Mar 8, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

[Response Ops][Actions] Keep track of action execution source and show in connector exec log #152030

[Response Ops][Actions] Keep track of action execution source and show in connector exec log #152030

Conversation

ymao1 commented Feb 23, 2023 • edited Loading

Summary

To Verify

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

elasticmachine commented Feb 27, 2023

ymao1 commented Feb 27, 2023

lcawl commented Mar 1, 2023

ymao1 commented Mar 1, 2023

ymao1 commented Mar 6, 2023

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

pgayvallet left a comment

Choose a reason for hiding this comment

kibana-ci commented Mar 8, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

API count

ESLint disabled line counts

Total ESLint disabled count

History

ymao1 commented Feb 23, 2023 •

edited

Loading