-
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
[RAC] - Update field names #107857
[RAC] - Update field names #107857
Conversation
x-pack/plugins/rule_registry/server/utils/create_lifecycle_rule_type.test.ts
Outdated
Show resolved
Hide resolved
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
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.
LGTM - thanks for consolidating the fields!
...lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_registry_log_client.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/timelines/server/search_strategy/timeline/index.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.
Thank you for doing this! Tested locally with @machadoum and looks great, just one minor comment 😄
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.
Reviewed the Alerting code- looks like the only change is the input and output of a couple of unit tests and looks like it was made to make maintenance easier so all I can say is thanks Yara :)
LGTM
Just a comment to uncomment these when it's all sorted: #108034 |
@@ -17,25 +17,18 @@ const CONSUMERS = `${KIBANA_NAMESPACE}.consumers` as const; | |||
const ECS_VERSION = 'ecs.version' as const; | |||
const EVENT_ACTION = 'event.action' as const; | |||
const EVENT_KIND = 'event.kind' as const; | |||
const RULE_CATEGORY = 'rule.category' as const; |
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.
rule.*
should only refer to source/event document fields, not our kibana rules.
[Fields.ALERT_PRODUCER]: { type: 'keyword' }, | ||
[Fields.ALERT_RULE_TYPE_ID]: { type: 'keyword', required: true }, | ||
[Fields.ALERT_RULE_CONSUMER]: { type: 'keyword', required: true }, | ||
[Fields.ALERT_RULE_PRODUCER]: { type: 'keyword' }, |
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.
Producer can be derived from the rule type id (apm.error_count
) --> apm
, so while convenient, we don't need to require it as long as we have the consumer and rule type id, we have enough for rbac.
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 LGTM!
@elasticmachine merge upstream |
cb03411
to
759decb
Compare
@michaelolo24 Note that after most recent CI failure, took the opportunity to make these changes here so once this goes in, should be golden 👍🏽 |
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @yctercero |
### Summary ### Fields used moving forward `kibana.alert.rule.consumer` will refer to the context in which a rule instance is created. Rules created in: - stack --> `alerts` - security solution --> `siem` - apm --> `apm` `kibana.alert.rule.producer` will refer to the plugin that registered a rule type. Rules registered in: - stack --> `alerts` - security solution --> `siem` - apm --> `apm` So an `apm.error_rate` rule created in stack will have: - consumer: `alerts` and producer: `apm` An `apm.error_rate` rule created in apm will have: - consumer: `apm` and producer: `apm` `kibana.alert.rule.rule_type_id` will refer to a rule's rule type id. Examples: - `apm.error_rate` - `siem.signals` - `siem.threshold` Also renamed the following because `rule.*` fields are meant to be ecs fields pulled from the source/event document, not refer to our rule fields. `rule.name` --> `kibana.alert.rule.name` will refer to the rule's name. `rule.category` --> `kibana.alert.rule.category` will refer to the rule's category. `rule.id` --> `kibana.alert.rule.uuid` will refer to the rule's uuid.
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
### Summary ### Fields used moving forward `kibana.alert.rule.consumer` will refer to the context in which a rule instance is created. Rules created in: - stack --> `alerts` - security solution --> `siem` - apm --> `apm` `kibana.alert.rule.producer` will refer to the plugin that registered a rule type. Rules registered in: - stack --> `alerts` - security solution --> `siem` - apm --> `apm` So an `apm.error_rate` rule created in stack will have: - consumer: `alerts` and producer: `apm` An `apm.error_rate` rule created in apm will have: - consumer: `apm` and producer: `apm` `kibana.alert.rule.rule_type_id` will refer to a rule's rule type id. Examples: - `apm.error_rate` - `siem.signals` - `siem.threshold` Also renamed the following because `rule.*` fields are meant to be ecs fields pulled from the source/event document, not refer to our rule fields. `rule.name` --> `kibana.alert.rule.name` will refer to the rule's name. `rule.category` --> `kibana.alert.rule.category` will refer to the rule's category. `rule.id` --> `kibana.alert.rule.uuid` will refer to the rule's uuid. Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>
Summary
Trying to align on field naming. In the code there seems to be the following that are all referencing the context in which a rule instance was created (rules created in stack have consumer of
alerts
):rule.consumers
kibana.alert.consumer
kibana.alert.consumers
kibana.alert.rule.consumers
kibana.alert.owner
In the code there are also the following all referencing a rule's alert type (ex:
apm.error_rate
):rule.id
kibana.alert.rule.rule_type_id
It's my understanding that these are all meant to refer to the same thing - the single feature which created the rule instance. So if a rule was created in security solution, the consumer is
siem
, if created in apm it'sapm
. Observability and the rule registry were usingkibana.alert.owner
though it'd been on the list to update tokibana.alert.consumer
and security solution was usingkibana.alert.owners
. The other ones were there as fields but not being used.I think there is some confusion as to what
consumer
is meant to denote. It should denote the same thing it does for rules rbac. If there's other context we want to save on an alert, that should be considered a different field, theconsumer
field was meant to be specific to role based access logic shared with alerting framework.Fields used moving forward
kibana.alert.rule.consumer
will refer to the context in which a rule instance is created. Rules created in:alerts
siem
apm
kibana.alert.rule.producer
will refer to the plugin that registered a rule type. Rules registered in:alerts
siem
apm
So an
apm.error_rate
rule created in stack will have:alerts
and producer:apm
An
apm.error_rate
rule created in apm will have:apm
and producer:apm
kibana.alert.rule.rule_type_id
will refer to a rule's rule type id. Examples:apm.error_rate
siem.signals
siem.threshold
Also renamed the following because
rule.*
fields are meant to be ecs fields pulled from the source/event document, not refer to our rule fields.rule.name
-->kibana.alert.rule.name
will refer to the rule's name.rule.category
-->kibana.alert.rule.category
will refer to the rule's category.rule.id
-->kibana.alert.rule.uuid
will refer to the rule's uuid.To test
Add the following to your
kibana.dev.yml
:Create a custom rule with query
*:*
.Run
yarn test:generate
withinx-pack/plugins/security_solution
Observe the magic (alerts should show up in table).