-
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
[RAC][Rule Registry] Improve RuleDataService API and index bootstrapping implementation #108115
[RAC][Rule Registry] Improve RuleDataService API and index bootstrapping implementation #108115
Conversation
a48f27d
to
a6fbb74
Compare
d799fa8
to
8d75711
Compare
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/uptime (Team:uptime) |
Pinging @elastic/apm-ui (Team:apm) |
componentTemplates: [ | ||
{ | ||
name: 'mappings', | ||
version: 0, |
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.
Proper versioning according to the recent discussions will be implemented in a follow-up PR.
const { ruleDataService } = plugins.ruleRegistry; | ||
const ruleDataClient = ruleDataService.initializeIndex({ | ||
feature: 'observability', | ||
registrationContext: 'observability', | ||
dataset: Dataset.alerts, | ||
componentTemplateRefs: [], | ||
componentTemplates: [], | ||
indexTemplate: { | ||
version: 0, | ||
}, | ||
}); |
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.
This replaces the previous implementation:
const ruleDataClient = plugins.ruleRegistry.ruleDataService.getRuleDataClient(
'observability',
plugins.ruleRegistry.ruleDataService.getFullAssetName(),
() => Promise.resolve()
);
Seems like it's a stub. Should I keep it? Why it's needed?
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.
The observability
plugin apparently needs a ruleDataClient
for use in some route handlers without intending to write any alerts itself. Does this initializeIndex
create any assets which the previous implementation didn't?
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.
Does this
initializeIndex
create any assets which the previous implementation didn't?
It will see if it should bootstrap any resources for .alerts-observability
. Specifically:
- ILM policy if it's provided. In this case, it's not.
- Component templates if they are provided. In this case, they're not.
And then it will update mappings of all concrete indices matching .alerts-observability-*
. This pattern shouldn't match any indices, I guess, because you're saying this ruleDataClient
is not used for writing alerts? Other indices containing alerts are named like .alerts-observability.apm-*
etc.
Is this ruleDataClient
used for reading all the observability alerts - from .alerts-observability.apm-*
, .alerts-observability.logs-*
and other indices?
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.
It seems for this use case it would be useful to expose some kind of getRuleDataReader
API in RuleDataPluginService
that skips all the index management steps and just returns an IRuleDataReader
. If I understand correctly the registrationContext would be observability.*
.
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 was also thinking about that
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 created a ticket for that: #111173
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 went through and did a first pass of the changes here. I found a few ways that I think we can simplify the implementation further, especially regarding signalling when asynchronous template installation functions are complete and the next operations can proceed.
But, very exciting to see the new usage of ruleDataService.initializeIndex
! It's a huge improvement to move the initialization and signalling logic into a single place rather than having each plugin implement it separately.
x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_client/rule_data_client.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/resource_installer.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/resource_names.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/index_names.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts
Show resolved
Hide resolved
6895ff9
to
f625838
Compare
adfe310
to
077ca24
Compare
…ing implementation (elastic#108115) **Addresses:** elastic#106421, elastic#106428, elastic#102089, elastic#106433 ## Summary This PR focuses on consolidation of indexing implementations in `rule_registry` (elastic#101016). It addresses some of the sub-tasks of the parent ticket. - [x] Encapsulate index bootstrapping logic in a new improved API exposed by `RuleDataService`. - [x] Enforce allowed values for the `datasetSuffix` on the API level. - [x] Migrate plugins using the existing `RuleDataService` API to the improved one. - [x] Make sure index names comply with design architecture. - elastic#102089 - [x] Improve the API of `RuleDataClient`. - [x] Enhance index bootstrapping: support custom ILM policy per index (`{registrationContext}.{datasetSuffix}`). - [x] Enhance index bootstrapping: create index template per namespace and support rollovers properly - based on elastic#107700 - [x] Enhance index bootstrapping: support secondary aliases - based on elastic#107700 - [x] Remove `EventLogService` implementation - elastic#106433 This will be addressed in follow-up PRs: - [ ] Enhance index bootstrapping: implement suggestions for backwards compatibility (naming scheme for alias and backing indices; versioning). - [ ] Enhance index bootstrapping: implement upgrades of existing index templates. - [ ] Make index bootstrapping logic more robust. This _is partially addressed_ in this PR, but more improvements are needed. - [ ] Change the way index prefix works. - [ ] Add support for optional TS schema (static typing). - [ ] Update `README` in `rule_registry`. ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…ing implementation (#108115) (#108638) **Addresses:** #106421, #106428, #102089, #106433 ## Summary This PR focuses on consolidation of indexing implementations in `rule_registry` (#101016). It addresses some of the sub-tasks of the parent ticket. - [x] Encapsulate index bootstrapping logic in a new improved API exposed by `RuleDataService`. - [x] Enforce allowed values for the `datasetSuffix` on the API level. - [x] Migrate plugins using the existing `RuleDataService` API to the improved one. - [x] Make sure index names comply with design architecture. - #102089 - [x] Improve the API of `RuleDataClient`. - [x] Enhance index bootstrapping: support custom ILM policy per index (`{registrationContext}.{datasetSuffix}`). - [x] Enhance index bootstrapping: create index template per namespace and support rollovers properly - based on #107700 - [x] Enhance index bootstrapping: support secondary aliases - based on #107700 - [x] Remove `EventLogService` implementation - #106433 This will be addressed in follow-up PRs: - [ ] Enhance index bootstrapping: implement suggestions for backwards compatibility (naming scheme for alias and backing indices; versioning). - [ ] Enhance index bootstrapping: implement upgrades of existing index templates. - [ ] Make index bootstrapping logic more robust. This _is partially addressed_ in this PR, but more improvements are needed. - [ ] Change the way index prefix works. - [ ] Add support for optional TS schema (static typing). - [ ] Update `README` in `rule_registry`. ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Georgii Gorbachev <[email protected]>
💚 Build SucceededMetrics [docs]Unknown metric groupsAPI count
API count missing comments
Non-exported public API item count
History
To update your PR or re-run it, just comment with: cc @banderror |
Addresses: #106421, #106428, #102089, #106433
Summary
This PR focuses on consolidation of indexing implementations in
rule_registry
(#101016). It addresses some of the sub-tasks of the parent ticket.RuleDataService
.datasetSuffix
on the API level.RuleDataService
API to the improved one.RuleDataClient
.{registrationContext}.{datasetSuffix}
).EventLogService
implementationThis will be addressed in follow-up PRs:
README
inrule_registry
.Checklist