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.

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] Possible "experimental" ECS fields are not backwards compatible for alerts as data #168959

Closed
ymao1 opened this issue Oct 16, 2023 · 4 comments · Fixed by #170571
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@ymao1
Copy link
Contributor

ymao1 commented Oct 16, 2023

There was a recent PR to update the ECS field mappings from 8.6 to the latest that we noticed caused an error when updating existing alerts as data indices due to this error:

[2023-10-13T11:06:44.254-04:00][ERROR][plugins.alerting] ResponseError: illegal_argument_exception
	Root causes:
		illegal_argument_exception: can't merge a non-nested mapping [faas.trigger] with a nested mapping
    at KibanaTransport.request (/Users/ying/Code/kibana_prs/node_modules/@elastic/transport/src/Transport.ts:535:17)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

It looks like there was an intentional change to the ECS faas.trigger field to change it from nested to object. Reading the issue, it seems that this change was approved because at the time the faas.trigger field was experimental. We've left a question to the ECS team to ask if there's a way to differentiate experimental ECS fields from non-experimental fields, but in the meantime, we are left with a lot of alerts as data indices in the wild with the experimental nested mapping type.

In order to move foward with the ECS update, which we've seen some customer requests for have been asking for, we should figure out how to handle this

Options

  • Omit this problematic field from the mapping - this is the easiest but will probably cause issues in the future
  • Update the code to either skip updating the underlying mapping of existing concrete indices entirely or skip if there's a failure, with the understanding that the AAD indices should rollover and get the updated mapping in the future. This might cause mapping conflicts across indices in an alias or data stream. We need to understand what the behavior is in the alerts table if we go this route
  • Investigate whether this problem would go away if we switched to dynamic mappings for ECS as suggested here: Replace ECS static mapping with ECS dynamic mapping for AAD indices #168497
@ymao1 ymao1 added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Oct 16, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@mikecote
Copy link
Contributor

After discussing with @ymao1 @pmuellr @kobelb @doakalexi we will go with the following option:

Omit this problematic field from the mapping - this is the easiest but will probably cause issues in the future

While we omit this field, we should also omit all other ECS experimental fields to prevent this situation for now.

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 1, 2023

According to the ECS RFC stages doc, any RFC in stage 3 is considered GA.

Here are the fields from RFCs in generated/ecs_flat.yml that are not yet in stage 3:

https://github.com/elastic/ecs/blob/main/rfcs/text/0002-rfc-environment.md - marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0009-data_stream-fields.md - not marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0015-create-file-elf.md - not marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0027-faas-fields.md - not marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0034-device-fields.md - not marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0035-tty-output.md - marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0037-host-metrics.md - 2 fields added, not marked as beta
https://github.com/elastic/ecs/blob/main/rfcs/text/0040-volume-device.md - not marked as beta

Seems like fields make it into the normal (not experimental) generated ECS file when they reach stage 2, where iterative but not breaking changes might be expected but they're still only recommended for experimental usage. There is a beta designation` but doesn't look like all fields in stage 2 mark themselves as beta.

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 2, 2023

Opened an issue in the ECS repo: elastic/ecs#2294

@ymao1 ymao1 moved this from In Progress to In Review in AppEx: ResponseOps - Execution & Connectors Nov 3, 2023
kibanamachine added a commit to fkanout/kibana that referenced this issue Nov 27, 2023
…mponent template (elastic#170571)

Resolves elastic#168959

## Summary

Hard-coding a list of experimental (RFC stage 2) ECS fields to exclude
from the ECS component template. These are only the fields that are not
currently defined in `ecs_flat.yml`. The only existing field that is
excluded is `faas.trigger` which, if included, will cause a mapping
conflict exception because of an ECS mapping change from `nested` to
`object`.

## To Verify

Compare the mappings for the `.alerts-ecs-mappings` component template
between `main` and this branch and notice that the `faas.trigger` field
is excluded from the component template on this branch.

---------

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants