-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
License checks for alerts plugin #85649
Conversation
…#84997) * Define minimum license required for each alert type * fixed typechecks * fixed tests * fixed tests * fixed due to comments * fixed due to comments * removed file * removed casting to LicenseType
#85223) * [Alerts][License] Add license checks to alerts HTTP APIs and execution * fixed typechecks * resolved conflicts * resolved conflicts * added router tests * fixed typechecks * added license check support for alert task running * fixed typechecks * added integration tests * fixed due to comments * fixed due to comments * fixed tests * fixed typechecks
…sn't support it. (#85496) * [Alerting UI][License] Disable alert types in UI when the license doesn't support it. * fixed typechecks * added licensing for alert list and details page * fixed multy select menu * fixed due to comments * fixed due to comments * fixed due to comments
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving—nothing looks different from the previous PR :)
Pinging @elastic/uptime (Team:uptime) |
Pinging @elastic/apm-ui (Team:apm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Infra change LGTM
Suggestions for texxt: Error in 1 alert View | Dismiss Cannot run alert Alert Dismiss | Manage license @alexfrancoeur Can you please weigh in the wording around licensing? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APM changes look good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feature LGTM! I ran a scenario where I created an alert in gold, downgraded my license and saw the proper error and UX. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Works as expected.
What do you think about this @gchaps? Adding a CTA.
|
@alexfrancoeur, @YulNaumenko. I like the the second sentence with the CTA to contact your administrator. In which, case I think we should remove the "Manage license" link":
|
I think it's fine keeping the manage your license link, if they are the administrator, a streamlined navigation experience couldn't hurt. Is there a way we can phrase the text in which the CTA can live with the manage license button? |
Need to check this with @mikecote. |
For the message Remove the period at the end of the title: Cannot run alert We could make the text "upgrade your license" the link. But that still means removing the Manage license button. |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Distributable file count
Page load bundle
History
To update your PR or re-run it, just comment with: |
* master: (66 commits) [Alerting] fixes broken Alerting Example plugin (elastic#85774) [APM] Service overview instances table (elastic#85770) [Security Solution] Unskip timeline creation Cypress test (elastic#85871) properly recognize enterprise licenses (elastic#85849) [SecuritySolution][Detections] Adds SavedObject persistence to Signals Migrations (elastic#85690) [TSVB] Fix functional tests flakiness and unskip them (elastic#85388) [Fleet] Change permissions for Fleet enroll role (elastic#85802) Gauge visualization can no longer be clicked to filter on values since Kibana 7.10.0 (elastic#84768) [Security Solution][Detections] Add alert source to detection rule action context (elastic#85488) [Discover] Don't display hide/show button for histogram when there's no time filter (elastic#85424) skip flaky suite (elastic#78553) License checks for alerts plugin (elastic#85649) skip flaky suite (elastic#84992) skip 'query return results valid for scripted field' elastic#78553 Allow action types to perform their own mustache variable escaping in parameter templates (elastic#83919) [ML] More machine learning links in doc_links_service.ts (elastic#85365) Removed Alerting & Event Log deprecated fields that should not be using (elastic#85652) Closes elastic#79995 by adding new tab in transaction details to show related trace logs. (elastic#85859) Fix outdated jest snapshot [Maps] Surface on prem EMS (elastic#85729) ...
Resolve #60767
In this PR, we're making alert types license aware and adding the proper behaviors to the APIs.
This branch is a feature branch that sub PRs are merging into until the feature is complete.
[Alerts][License] Define minimum license required for each alert type (#84997)
minimumLicenseRequired
attribute to each alert type definition[Alerts][License] Add license checks to alerts HTTP APIs and execution (#85223)
[Alerting UI][License] Disable alert types in UI when the license doesn't support it. (#85496)
Current PR covers the licensing UI for the alert types.
The sort order is a bit more complicated then for action types, because we have groups of alert types by the solution.
Current PR implements sort order by keeping the solutions alphabet order if the solution contains at least one available alert type and if all alert types under solution is disabled by the license restriction it will be sorted after the solutions with available alert types.
Then inside the solution group we sort solution items inside by the license availability and then alphabetically by alert type name.
User have a possibility to delete alerts with the higher order alert type license (same as for the action types).
There is an option to navigate to the alert details page. The purpose of this usage is to see the error message generated by the current alert execution behavior for the expired alerts - continue running with the error license reason.
User is able to manage the license from this message.