Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Actionable Observability] Add context.alertDetailsUrl variable to action connector template for APM rule types #144791

Merged
merged 27 commits into from
Nov 14, 2022
Merged

[Actionable Observability] Add context.alertDetailsUrl variable to action connector template for APM rule types #144791

merged 27 commits into from
Nov 14, 2022

Conversation

CoenWarmer
Copy link
Contributor

@CoenWarmer CoenWarmer commented Nov 8, 2022 •
edited
Loading

Closes #144439, closes #144438, closes #144436, closes #144434.

Summary

This PR adds the context.alertDetailsUrl action template variable for the following APM rule types:

  • Anomaly
  • Latency Threshold
  • Error Threshold
  • Failed Transaction Rate Threshold

What it looks like

Configure a rule, with {{context.alertDetailsUrl}} variable:
Screenshot 2022-11-08 at 11 48 59

Leads to output: (chosen server output in screenshot):
Screenshot 2022-11-08 at 11 49 18

Checklist

  • Alert detail url context variable for APM > Latency Threshold rule type added
  • Alert detail url context variable for APM > Error Threshold rule type added
  • Alert detail url context variable for APM > Anomaly Threshold rule type added
  • Alert detail url context variable for APM > Failed Transaction Rate Threshold rule type added
  • Displaying of context variable in selection list (see screenshot) depends on feature flag: it will not show up in the list if feature flag xpack.observability.unsafe.alertDetails.apm.enabled is set tofalse
  • Alert detail url is Space-aware
  • Also made the context.viewInAppUrl variable Space-aware.

@CoenWarmer CoenWarmer added the release_note:skip Skip the PR/issue when compiling release notes label Nov 8, 2022
@CoenWarmer CoenWarmer requested review from a team as code owners November 8, 2022 10:50
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Nov 8, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@CoenWarmer CoenWarmer force-pushed the feat/144439-add-context-alertdetailsurl-to-apm-latency-threshold-rule branch from c1aeca3 to 73e2053 Compare November 8, 2022 12:32
@CoenWarmer CoenWarmer force-pushed the feat/144439-add-context-alertdetailsurl-to-apm-latency-threshold-rule branch from 73e2053 to f11a5ad Compare November 8, 2022 14:05
@CoenWarmer CoenWarmer requested review from a team as code owners November 8, 2022 14:05
@CoenWarmer CoenWarmer changed the title [Actionable Observability] Add context.alertDetailsUrl to connector template for APM Latency Threshold rule [Actionable Observability] Add context.alertDetailsUrl to connector template for APM Latency Threshold & Error Threshold rule types Nov 8, 2022
@CoenWarmer
Merge branch 'main' of github.com:elastic/kibana into feat/144439-add…
93a0ee8 
…-context-alertdetailsurl-to-apm-latency-threshold-rule
@CoenWarmer CoenWarmer force-pushed the feat/144439-add-context-alertdetailsurl-to-apm-latency-threshold-rule branch from f11a5ad to 93a0ee8 Compare November 8, 2022 14:07
@CoenWarmer CoenWarmer marked this pull request as draft November 8, 2022 14:20
pmuellr
pmuellr requested changes Nov 8, 2022
View reviewed changes
@CoenWarmer CoenWarmer changed the title [Actionable Observability] Add context.alertDetailsUrl to connector template for APM Latency Threshold & Error Threshold rule types [Actionable Observability] Add context.alertDetailsUrl variable to action connector template for APM rule types Nov 8, 2022
@CoenWarmer CoenWarmer marked this pull request as ready for review November 8, 2022 15:17
kdelemme
kdelemme approved these changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM 👍🏻
Tested locally all APM rules except Anomaly (can't make an alert trigger).
image

CoenWarmer and others added 5 commits November 9, 2022 09:57
@CoenWarmer
Update tests
7ac391c
@CoenWarmer
Merge branch 'main' of github.com:elastic/kibana into feat/144439-add…
fbefd38 
…-context-alertdetailsurl-to-apm-latency-threshold-rule
@dgieselaar
Expand types of createLifecycleRuleTypeFactory
3a4ed3d
@CoenWarmer
Merge branch 'feat/144439-add-context-alertdetailsurl-to-apm-latency-…
7b56b75 
…threshold-rule' of github.com:CoenWarmer/kibana into feat/144439-add-context-alertdetailsurl-to-apm-latency-threshold-rule
@dgieselaar
Make getAlertUuid idempotent
6011172
sorenlouv
sorenlouv reviewed Nov 14, 2022
View reviewed changes
Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall lgtm but re-orderings make it quite a bit harder to review

@CoenWarmer
Copy link
Contributor Author

Overall lgtm but re-orderings make it quite a bit harder to review

Thanks for the feedback. Main objective was to lower the noise and improve legibility in the files. As a consequence the noise in the PR goes up a bit. Apologies for that. Will refrain from doing it in the future.

@CoenWarmer CoenWarmer requested a review from pmuellr November 14, 2022 11:52
@CoenWarmer CoenWarmer force-pushed the feat/144439-add-context-alertdetailsurl-to-apm-latency-threshold-rule branch from 0995e90 to a1cfd27 Compare November 14, 2022 12:17
@sorenlouv
Copy link
Member

sorenlouv commented Nov 14, 2022 •
edited
Loading

Thanks for the feedback. Main objective was to lower the noise and improve legibility in the files. As a consequence the noise in the PR goes up a bit. Apologies for that. Will refrain from doing it in the future.

No worries. I think the re-orderings look good. I'd just prefer them done in a separate PR.

@dgieselaar dgieselaar requested a review from a team as a code owner November 14, 2022 13:14
@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Nov 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@CoenWarmer
Merge branch 'main' of github.com:elastic/kibana into feat/144439-add…
0c78f52 
…-context-alertdetailsurl-to-apm-latency-threshold-rule
@CoenWarmer
Merge branch 'feat/144439-add-context-alertdetailsurl-to-apm-latency-…
83d3269 
…threshold-rule' of github.com:CoenWarmer/kibana into feat/144439-add-context-alertdetailsurl-to-apm-latency-threshold-rule
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

History

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

pmuellr
pmuellr approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alerting-related changes in rule_registry LGTM.

I guess the typing change of AlertTypeWithExecutor got the services in the executor to be typed the way you wanted.

@CoenWarmer CoenWarmer merged commit 53bab70 into elastic:main Nov 14, 2022
@kibanamachine kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Nov 14, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 15, 2022
@jloleysens
Merge branch 'main' into files-management
6c5fb8a 
* main: (65 commits)
  Migrate server-side `Root` and `Server` to packages (elastic#144990)
  [Discover] Handle no data views state for `esQuery` alert (elastic#145052)
  [ML] Allow updates for number of allocations and priority for trained model deployments (elastic#144704)
  [api-docs] 2022-11-15 Daily api_docs build (elastic#145203)
  [Security solution] remove guided onboarding feature flag (elastic#144247)
  [DOCS] Automate final case APIs (elastic#145007)
  [Enterprise Search] Name and description flyout for connectors (elastic#143827)
  [Guided onboarding] Update header button logic (elastic#144634)
  [Lens] Multi metric partition charts (elastic#143966)
  [Dashboard] [Controls] Add unmapped runtime field support to options list (elastic#144947)
  [Security Solution] Add Task Metric Collection to New Tasks (elastic#145181)
  [TriggersActionsUi] disable jest config in CI (elastic#145186)
  [TableListView] Enhance tag filtering (elastic#142108)
  [Cloud Posture] Compliance by CIS section table (elastic#145114)
  [8.6][Session View] Fix hidden alert flyout  in session view (elastic#145141)
  [customIntegrations] async load all components (elastic#145166)
  Fix time for logs smoke tests in integration test (elastic#145130)
  [RAM] Update rule status (elastic#140882)
  Update babel (main) (elastic#145060)
  [Actionable Observability] Add context.alertDetailsUrl variable to action connector template for APM rule types (elastic#144791)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sorenlouv sorenlouv sorenlouv left review comments

@mikecote mikecote mikecote left review comments

@pmuellr pmuellr pmuellr approved these changes

@dgieselaar dgieselaar dgieselaar approved these changes

@kdelemme kdelemme kdelemme approved these changes

@shahzad31 shahzad31 shahzad31 approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.6.0
Projects
No open projects
AppEx: ResponseOps - Execution & Connectors
Archived in project
Milestone
No milestone
10 participants
@CoenWarmer @elasticmachine @sorenlouv @kibana-ci @pmuellr @dgieselaar @kdelemme @shahzad31 @mikecote @kibanamachine