-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Response Ops][Alerting] Migrate installation of context-specific component templates, index templates and concrete write index to framework for alerts-as-data #151792
Conversation
c24a423
to
9ea206c
Compare
@@ -0,0 +1,20 @@ | |||
/* |
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.
Moved from x-pack/plugins/rule_registry/common/assets/field_maps/experimental_rule_field_map.ts
x-pack/plugins/rule_registry/server/rule_data_plugin_service/resource_installer.ts
Show resolved
Hide resolved
…mework install context specific resources
22a7324
to
cc24f4d
Compare
x-pack/plugins/alerting/common/alert_schema/field_maps/component_template_from_field_map.ts
Outdated
Show resolved
Hide resolved
if (!isEqual(fieldMap, registeredFieldMap)) { | ||
throw new Error(`${context} has already been registered with a different mapping`); | ||
const registeredOptions = this.registeredContexts.get(context); | ||
if (!isEqual(opts, registeredOptions)) { |
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.
Testing equality of all options to catch the possibility of rule types registering with the same context but different useEcs
or useLegacyAlerts
flags.
@@ -326,6 +361,14 @@ export class AlertsService implements IAlertsService { | |||
) { | |||
this.options.logger.info(`Installing index template ${indexPatterns.template}`); | |||
|
|||
const indexMetadata: Metadata = { |
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.
Added this to align with metadata that rule registry is outputting
/** | ||
* Optional secondary alias to use. This alias should not include the namespace. | ||
*/ | ||
secondaryAlias?: string; |
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.
Added to allow detection alerts to specify siem.signals
alias
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.
Great work!! I finished reviewing the code and have the comments below from our discussion. I will move onto testing the PR 👍 🎉
x-pack/plugins/alerting/common/alert_schema/field_maps/component_template_from_field_map.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/alerting/server/alerts_service/alerts_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/alerting/server/alerts_service/alerts_service.ts
Outdated
Show resolved
Hide resolved
@@ -16,6 +16,7 @@ import { | |||
DEFAULT_ALERTS_ILM_POLICY, |
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.
I'm not sure if this is a concern given we can do a diff down the line but:
question: do we have something in place to prevent registrationContext
from differentiating to what is provided to the rule type via alerts
object? (thinking in case a new rule type is in the works somewhere or soemthing.. until we remove the need for registrationContext
)
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.
If a new rule type is added to the rule registry without also being registered with the framework and the framework handles the installation the context specific resources, any run of the rule type that generates an alert will throw an error on execution. I believe this should be caught during development but if we want an additional check, I think we could add a check to the initializeIndex
function in the rule registry RuleDataService
that verifies any context registered with the rule data service is also registered in the framework.
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.
I think adding the function within the rule registry's initializeIndex
or doing a manual check when switching the framework alerts feature flag will be ok. If it's trivial to do the manual check, maybe we just do that when validating the feature flag PR?
x-pack/plugins/rule_registry/server/rule_data_plugin_service/resource_installer.ts
Show resolved
Hide resolved
x-pack/plugins/alerting/server/alerts_service/create_resource_installation_helper.ts
Outdated
Show resolved
Hide resolved
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.
Completed my testing this morning, everything looks great! I used observability.metrics
for my testing and all the differences were as outlined in the PR. Great work!
@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.
LGTM! Tested locally with an existing SLO burn rate rule running.
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.
Uptime changes LGTM
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.
Detection alerts changes LGTM
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 on behalf of Infra Observability UI
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @ymao1 |
…ponent templates, index templates and concrete write index to framework for alerts-as-data (elastic#151792) Resolves elastic#151697 ## Summary In a previous [PR](elastic#145581) we started installing a context-specific component templates, index templates and concrete write indices for framework alerts as data when the `xpack.alerting.enableFrameworkAlerts` config flag is set to true. In that PR we used a different naming pattern than what is used by the rule registry for those resources. In this PR, we are aligning the naming of these resources with the rule registry and installing these resources on alerting plugin setup when `enableFrameworkAlerts: true`. If the flag is set to false, the rule registry will continue to handle this resource installation. In this PR we are doing the following: * Registering all rules currently registered with the rule registry with the alerting framework. This registration allows the alerting framework to build context specific component templates. Because this PR only addresses resource installation, rules will continue to be registered with the rule registry. * When `enableFrameworkAlerts: true`: * The framework installs the context specific component template with the following naming convention: `.alerts-{context}.alerts-mappings`. This matches what the rule registry currently installs so the transition should be seamless * The framework installs the context specific index template for the `default` space with the following name: `.alerts-{context}.alerts-default-index-template`. Space awareness will be addressed in a followup PR. This matches the current rule registry naming.This index template will reference (1) ECS component template (if `useEcs: true`), (2) context-specific component template, (3) legacy alert component template and (4) framework component template where the legacy alert component template + framework component template = technical component template (from the rule registry). * The framework creates or updates the concrete write index for the `default` space with the naming convention: `.internal.alerts-{context}.alerts-default-000001`. Space awareness will be addressed in a followup PR. This matches the current rule registry naming. * The installation of the index template & write index differs from the rule registry in that it occurs on alerting plugin start vs the first rule run. * We modified the rule registry resource installer to skip installation of these resources when `enableFrameworkAlerts: true`. In addition, it will wait for the alerting resource installation promise so if a rule runs before its resources are fully initialized, it will wait for initialization to complete before writing. ## To Verify The following rule registry contexts are affected: `observability.apm` `observability.logs` `observability.metrics` `observability.slo` `observability.uptime` `security` For each context, we should verify the following: `Note that if your rule context references the ECS mappings, there may be differences in those mappings between main and this branch depending on whether you're running main with enableFrameworkAlerts true or false. These differences are explained in the summary of this prior PR: elastic#150384 but essentially we're aligning with the latest ECS fields. In the instructions, I suggest running main with enableFrameworkAlerts: true to minimize the differences caused by ECS changes` **While running `main` with `enableFrameworkAlerts: true`:** 1. Get the context specific component template `GET _component_template/.alerts-{context}.alerts-mappings` 2. Create rule for this context that creates an alert and then 3. Get the index template `GET _index_template/.alerts-{context}.alerts-default-index-template` 4. Get the index mapping for the concrete index: `GET .internal.alerts-{context}.alerts-default-000001/_mapping` **While running this branch with `xpack.alerting.enableFrameworkAlerts: true` (with a fresh ES instance):** 5. Get the context specific component template `GET _component_template/.alerts-{context}.alerts-mappings` 6. Get the index template `GET _index_template/.alerts-{context}.alerts-default-index-template` 7. Get the index mapping for the concrete index: `GET .internal.alerts-{context}.alerts-default-000001/_mapping` Note that you should not have to create a rule that generates alerts before seeing these resources installed. **Compare the component templates** Compare 1 and 5. The difference should be: * component template from this branch should have `_meta.managed: true`. This is a flag indicating to the user that these templates are system managed and should not be manually modified. **Compare the index templates** Compare 3 and 6. The differences should be: * index template from this branch should have `managed: true` in the `_meta` fields * index template from this branch should not have a `priority` field. This will be addressed in a followup PR * index template from this branch should be composed of `.alerts-legacy-alert-mappings` and `.alerts-framework-mappings` instead of `.alerts-technical-mappings` but under the hood, these mappings are equivalent. **Compare the index mappings** Compare 4 and 7. The difference should be: * index mappings from this branch should have `_meta.managed: true`. ### Verify that installed resources templates work as expected 1. Run this branch on a fresh ES install with `xpack.alerting.enableFrameworkAlerts: true`. 2. Create a rule in your context that generates alerts. 3. Verify that there are no errors during rule execution. 4. Verify that the alerts show up in your alerts table as expected. 5. (For detection rules only): Run this branch with `xpack.alerting.enableFrameworkAlerts: true` and verify rules in a non-default space continue to create resources on first rule run and run as expected. 6. (For detection rules only): Run this branch with `xpack.alerting.enableFrameworkAlerts: true` and verify rule preview continue to work as expected ### Verify that installed resources templates work with existing rule registry resources. 1. Run `main` or a previous version and create a rule in your context that generates alerts. 2. Using the same ES data, switch to this branch with `xpack.alerting.enableFrameworkAlerts: false` and verify Kibana starts with no rule registry errors and the rule continues to run as expected. 3. Using the same ES data, switch to this branch with `xpack.alerting.enableFrameworkAlerts: true` and verify Kibana starts with no alerting or rule registry errors and the rule continues to run as expected. 4. Verify the alerts show up on the alerts table as expected. 5. (For detection rules only): Run this branch with `xpack.alerting.enableFrameworkAlerts: true` and verify rules in a non-default space continue to create resources on first rule run and run as expected. 6. (For detection rules only): Run this branch with `xpack.alerting.enableFrameworkAlerts: true` and verify rule preview continue to work as expected --------- Co-authored-by: Kibana Machine <[email protected]>
Resolves #151697
Summary
In a previous PR we started installing a context-specific component templates, index templates and concrete write indices for framework alerts as data when the
xpack.alerting.enableFrameworkAlerts
config flag is set to true. In that PR we used a different naming pattern than what is used by the rule registry for those resources. In this PR, we are aligning the naming of these resources with the rule registry and installing these resources on alerting plugin setup whenenableFrameworkAlerts: true
. If the flag is set to false, the rule registry will continue to handle this resource installation.In this PR we are doing the following:
enableFrameworkAlerts: true
:.alerts-{context}.alerts-mappings
. This matches what the rule registry currently installs so the transition should be seamlessdefault
space with the following name:.alerts-{context}.alerts-default-index-template
. Space awareness will be addressed in a followup PR. This matches the current rule registry naming.This index template will reference(1) ECS component template (if
useEcs: true
),(2) context-specific component template,
(3) legacy alert component template and
(4) framework component template
where the legacy alert component template + framework component template = technical component template (from the rule registry).
default
space with the naming convention:.internal.alerts-{context}.alerts-default-000001
. Space awareness will be addressed in a followup PR. This matches the current rule registry naming.enableFrameworkAlerts: true
. In addition, it will wait for the alerting resource installation promise so if a rule runs before its resources are fully initialized, it will wait for initialization to complete before writing.To Verify
The following rule registry contexts are affected:
observability.apm
observability.logs
observability.metrics
observability.slo
observability.uptime
security
For each context, we should verify the following:
Note that if your rule context references the ECS mappings, there may be differences in those mappings between main and this branch depending on whether you're running main with enableFrameworkAlerts true or false. These differences are explained in the summary of this prior PR: https://github.com/elastic/kibana/pull/150384 but essentially we're aligning with the latest ECS fields. In the instructions, I suggest running main with enableFrameworkAlerts: true to minimize the differences caused by ECS changes
While running
main
withenableFrameworkAlerts: true
:GET _component_template/.alerts-{context}.alerts-mappings
GET _index_template/.alerts-{context}.alerts-default-index-template
GET .internal.alerts-{context}.alerts-default-000001/_mapping
While running this branch with
xpack.alerting.enableFrameworkAlerts: true
(with a fresh ES instance):5. Get the context specific component template
GET _component_template/.alerts-{context}.alerts-mappings
6. Get the index template
GET _index_template/.alerts-{context}.alerts-default-index-template
7. Get the index mapping for the concrete index:
GET .internal.alerts-{context}.alerts-default-000001/_mapping
Note that you should not have to create a rule that generates alerts before seeing these resources installed.
Compare the component templates
Compare 1 and 5. The difference should be:
_meta.managed: true
. This is a flag indicating to the user that these templates are system managed and should not be manually modified.Compare the index templates
Compare 3 and 6. The differences should be:
managed: true
in the_meta
fieldspriority
field. This will be addressed in a followup PR.alerts-legacy-alert-mappings
and.alerts-framework-mappings
instead of.alerts-technical-mappings
but under the hood, these mappings are equivalent.Compare the index mappings
Compare 4 and 7. The difference should be:
_meta.managed: true
.Verify that installed resources templates work as expected
xpack.alerting.enableFrameworkAlerts: true
.xpack.alerting.enableFrameworkAlerts: true
and verify rules in a non-default space continue to create resources on first rule run and run as expected.xpack.alerting.enableFrameworkAlerts: true
and verify rule preview continue to work as expectedVerify that installed resources templates work with existing rule registry resources.
main
or a previous version and create a rule in your context that generates alerts.xpack.alerting.enableFrameworkAlerts: false
and verify Kibana starts with no rule registry errors and the rule continues to run as expected.xpack.alerting.enableFrameworkAlerts: true
and verify Kibana starts with no alerting or rule registry errors and the rule continues to run as expected.xpack.alerting.enableFrameworkAlerts: true
and verify rules in a non-default space continue to create resources on first rule run and run as expected.xpack.alerting.enableFrameworkAlerts: true
and verify rule preview continue to work as expected