Add context.alertDetailsUrl to connector template when configuring a Rule by CoenWarmer · Pull Request #142854 · elastic/kibana

CoenWarmer · 2022-10-06T13:48:33Z

Summary

Adds the context.alertDetailsUrl variable when configuring a connector template.

Resolves #139923
Resolves #141497

Tested with sending an email:

Also in this PR:

Adjustment in create_lifecycle_executor: changed naming of alertId to alertInstanceId as that is actually what it is.

x-pack/plugins/infra/server/lib/alerting/common/messages.ts

fkanout

That is cool, @CoenWarmer. Did you consider Kibana spaces in the URL?

CoenWarmer · 2022-10-06T14:15:07Z

That is cool, @CoenWarmer. Did you consider it with Kibana spaces?

Thanks @fkanout; re: spaces: Not yet, will do!

benakansara

Should we include full URL in the notification which user can directly click and navigate? e.g. http://localhost:5601/abc/app/observability/alerts/24b3905c-5ae7-49b1-903f-e7510116af39

.../infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts

x-pack/plugins/infra/server/lib/alerting/common/utils.ts

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

benakansara · 2022-10-06T14:28:19Z

@CoenWarmer If I understood correctly, currently we are adding context.alertDetailsUrl for Infrastructure rules. Other rule types are not in scope for this PR, is that right?

CoenWarmer · 2022-10-06T14:48:06Z

@simianhacker We don't know the Alert UUID in the context of the executor function, right?

If I understand correctly, an instance of an Alert is created after the executor has run?

--Edit:

Turns out we do have it. PR updated with new approach.

x-pack/plugins/infra/server/lib/alerting/common/utils.ts

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

simianhacker · 2022-10-10T19:57:40Z

@CoenWarmer What's your thoughts on alerts without UUIDs? Right now the the UUID gets created after the alert is scheduled (and after the alert detail link) is generated for "new" alerts. I wonder if we shouldn't create the UUID when getAlertUuid() is called. Basically when we call getAlertUuid(), if it doesn't exist, create a new map to track the alertUuids to alertIds that we needed to create and re-use them here:

kibana/x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts

Lines 229 to 232 in 4feb397

    
           const { alertUuid, started } = state.trackedAlerts[alertId] ?? { 
        
             alertUuid: v4(), 
        
             started: commonRuleFields[TIMESTAMP], 
        
           };

CoenWarmer · 2022-10-11T12:15:53Z

@simianhacker Lets discuss this today in our 1 on 1.

fkanout · 2022-10-11T13:41:51Z

@CoenWarmer, would please share the outcomes of your meeting with @simianhacker regarding the availability of alert UUID?

…ew alerts

…in actionVariables array

weltenwort

infra plugin changes LGTM, well done 👍 and thanks for adding the space awareness to the URLs

simianhacker

LGTM

…-context-alertDetailsUrl-for-given-alert-as-an-action-variable

kibana-ci · 2022-10-27T09:43:10Z

💚 Build Succeeded

Buildkite Build
Commit: c2c42fe

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
`ruleRegistry`	202	204	+2

Unknown metric groups

API count

id	before	after	diff
`ruleRegistry`	230	232	+2

History

💚 Build #82465 succeeded 9de24d4
💔 Build #82458 failed 8b52678d7e732eff8fe56634b8dcd29eee9a1a69
💔 Build #82451 failed f84b4894cf7a21bea46fcdee5930f4dfe5e75c18
💚 Build #82097 succeeded c6f5ec6d6bfe76cf095eef9a195c65a9dcd99508
💚 Build #81789 succeeded 8ddbcd72179f7a2aedb09bf39c6f7ab19aea6770
💛 Build #81743 was flaky 77bee2d8cffd5fad94d38eab793ecca5e718b4d7

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

ymao1

LGTM. Left one more nit about alertInstanceId. Thanks!

ymao1 · 2022-10-27T11:57:55Z

x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts

-        const alertId = hit._source ? hit._source[ALERT_INSTANCE_ID] : void 0;
-        if (alertId && hit._source) {
-          trackedAlertsDataMap[alertId] = {
+        const alertInstanceId = hit._source ? hit._source[ALERT_INSTANCE_ID] : void 0;


nit: can we change this back to alertId as well?

CoenWarmer added the v8.6.0 label Oct 6, 2022

CoenWarmer requested a review from a team October 6, 2022 13:48

CoenWarmer requested review from a team as code owners October 6, 2022 13:48

CoenWarmer changed the title ~~Add context.alertDetailsUrl to connector template~~ Add context.alertDetailsUrl to connector template when configuring a Rule Oct 6, 2022

CoenWarmer added the release_note:skip Skip the PR/issue when compiling release notes label Oct 6, 2022

CoenWarmer commented Oct 6, 2022

View reviewed changes

x-pack/plugins/infra/server/lib/alerting/common/messages.ts Outdated Show resolved Hide resolved

fkanout reviewed Oct 6, 2022

View reviewed changes

CoenWarmer requested a review from simianhacker October 6, 2022 14:15

benakansara reviewed Oct 6, 2022

View reviewed changes

.../infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts Outdated Show resolved Hide resolved

x-pack/plugins/infra/server/lib/alerting/common/utils.ts Outdated Show resolved Hide resolved

CoenWarmer commented Oct 6, 2022

View reviewed changes

x-pack/plugins/alerting/server/types.ts Outdated Show resolved Hide resolved

CoenWarmer commented Oct 6, 2022

View reviewed changes

x-pack/plugins/infra/server/lib/alerting/common/utils.ts Outdated Show resolved Hide resolved

CoenWarmer force-pushed the feat/139923-add-context-alertDetailsUrl-for-given-alert-as-an-action-variable branch 2 times, most recently from 53fe49b to f50f780 Compare October 10, 2022 12:11

CoenWarmer requested a review from a team as a code owner October 10, 2022 15:39

CoenWarmer force-pushed the feat/139923-add-context-alertDetailsUrl-for-given-alert-as-an-action-variable branch 2 times, most recently from daa7fd4 to d2a5272 Compare October 10, 2022 15:41

ymao1 reviewed Oct 10, 2022

View reviewed changes

x-pack/plugins/alerting/server/types.ts Outdated Show resolved Hide resolved

CoenWarmer requested review from benakansara, fkanout and ymao1 October 10, 2022 15:57

CoenWarmer force-pushed the feat/139923-add-context-alertDetailsUrl-for-given-alert-as-an-action-variable branch 3 times, most recently from 42f44c0 to ff8ff35 Compare October 12, 2022 16:18

CoenWarmer added 10 commits October 25, 2022 14:29

Update comment

a3ed58a

Refactoring of alertIds to alertInstanceIds, correctly set UUID for n…

3036734

…ew alerts

Fix test

f62bf70

Update type signature

9f7f8c3

Changed getViewInAppUrl function name

5b87d11

Add configurable display of template variable based on feature flag

c07abc5

Add Spaces awareness to getAlertDetails and getViewInApp functions

48f4c93

Use existing addSpaceIdToPath util function

51e735b

Use conditional spread operator instead of filtering for null values …

b9ce5cd

…in actionVariables array

Remove unused spy function in mock

9de24d4

CoenWarmer force-pushed the feat/139923-add-context-alertDetailsUrl-for-given-alert-as-an-action-variable branch from ae4d760 to 9de24d4 Compare October 25, 2022 12:29

weltenwort approved these changes Oct 25, 2022

View reviewed changes

CoenWarmer removed the request for review from ymao1 October 26, 2022 08:27

simianhacker approved these changes Oct 26, 2022

View reviewed changes

CoenWarmer added 2 commits October 27, 2022 10:07

Revert refactoring of variable names

7d488df

Merge branch 'main' of github.com:elastic/kibana into feat/139923-add…

c2c42fe

…-context-alertDetailsUrl-for-given-alert-as-an-action-variable

ymao1 approved these changes Oct 27, 2022

View reviewed changes

CoenWarmer merged commit 7bc63e0 into elastic:main Oct 27, 2022

CoenWarmer deleted the feat/139923-add-context-alertDetailsUrl-for-given-alert-as-an-action-variable branch October 27, 2022 13:20

kibanamachine added the backport:skip This PR does not require backporting label Oct 27, 2022

kdelemme mentioned this pull request Nov 16, 2022

[Actionable Observability] viewInAppUrl context variables are not space aware #141497

Closed

maryam-saeidi mentioned this pull request May 23, 2023

Infra/Logs rule types > Add alertDetailsUrl parameter to redirect user to the Alerts page filtered for that alert instance #156534

Closed

Conversation

CoenWarmer commented Oct 6, 2022 • edited by kdelemme Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Uh oh!

fkanout left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CoenWarmer commented Oct 6, 2022

Uh oh!

benakansara left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benakansara commented Oct 6, 2022

Uh oh!

CoenWarmer commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simianhacker commented Oct 10, 2022

Uh oh!

CoenWarmer commented Oct 11, 2022

Uh oh!

fkanout commented Oct 11, 2022

Uh oh!

weltenwort left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simianhacker left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Oct 27, 2022

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

API count

History

Uh oh!

ymao1 left a comment

Choose a reason for hiding this comment

Uh oh!

ymao1 Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

CoenWarmer commented Oct 6, 2022 •

edited by kdelemme

Loading

fkanout left a comment •

edited

Loading

CoenWarmer commented Oct 6, 2022 •

edited

Loading

weltenwort left a comment •

edited

Loading