Skip to content

[Reponse Ops][Alerting] Excluding ECS experimental fields from ECS component template#170571

Merged
ymao1 merged 15 commits intoelastic:mainfrom
ymao1:exclude-experimental-ecs-fields
Nov 27, 2023
Merged

[Reponse Ops][Alerting] Excluding ECS experimental fields from ECS component template#170571
ymao1 merged 15 commits intoelastic:mainfrom
ymao1:exclude-experimental-ecs-fields

Conversation

@ymao1
Copy link
Contributor

@ymao1 ymao1 commented Nov 3, 2023 •
edited
Loading

Resolves #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.

@ymao1 ymao1 self-assigned this Nov 3, 2023
@ymao1 ymao1 added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Feature:Alerting/Alerts-as-Data Issues related to Alerts-as-data and RuleRegistry v8.12.0 labels Nov 3, 2023
@ymao1 ymao1 marked this pull request as ready for review November 3, 2023 20:33
@ymao1 ymao1 requested a review from a team as a code owner November 3, 2023 20:33
@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 3, 2023

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

@ymao1 ymao1 requested a review from a team as a code owner November 3, 2023 21:42
@ymao1 ymao1 requested a review from vitaliidm November 3, 2023 21:42
@ymao1
Copy link
Contributor Author

ymao1 commented Nov 6, 2023

@elasticmachine merge upstream

vitaliidm
vitaliidm reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question regarding impact on existing customers before alerts index rolls out

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 20, 2023

@elasticmachine merge upstream

@pmuellr
Copy link
Contributor

pmuellr commented Nov 20, 2023

I tried a "migration" from main to this PR, and got the following:

[2023-11-20T10:35:57.010-05:00][INFO ][plugins.alerting] Creating concrete write index - .internal.alerts-observability.metrics.alerts-default-000001
[2023-11-20T10:35:57.047-05:00][ERROR][plugins.alerting] Failed to PUT mapping for .alerts-observability.metrics.alerts-default: illegal_argument_exception
	Root causes:
		illegal_argument_exception: can't merge a non-nested mapping [faas.trigger] with a nested mapping
[2023-11-20T10:35:57.047-05: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/pmuellr/Projects/elastic/kibana-170571-exclude-experimental-ecs-fields/node_modules/@elastic/transport/src/Transport.ts:535:17)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

@ymao1 ymao1 requested a review from pmuellr November 20, 2023 16:22
pmuellr
pmuellr approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@pmuellr pmuellr left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - survives "migrating" from main to this PR; adding the "children" of faas to be excluded (added in subsequent commits) resolved the migration issue I reported ^^^

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 21, 2023

@elasticmachine merge upstream

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 21, 2023

@elasticmachine merge upstream

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 21, 2023

@elasticmachine merge upstream

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 22, 2023

@elasticmachine merge upstream

@ymao1
Copy link
Contributor Author

ymao1 commented Nov 27, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link

kibana-ci commented Nov 27, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.7MB 3.7MB +716.0B
infra 1.9MB 1.9MB +711.0B
observability 1.1MB 1.1MB +711.0B
securitySolution 12.8MB 12.8MB +711.0B
triggersActionsUi 1.4MB 1.4MB +711.0B
total +3.5KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ymao1

@ymao1 ymao1 merged commit 618cc48 into elastic:main Nov 27, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Nov 27, 2023
@ymao1 ymao1 deleted the exclude-experimental-ecs-fields branch November 27, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vitaliidm vitaliidm vitaliidm left review comments

@pmuellr pmuellr pmuellr approved these changes

Assignees

@ymao1 ymao1

Labels

backport:skip This PR does not require backporting Feature:Alerting/Alerts-as-Data Issues related to Alerts-as-data and RuleRegistry release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.12.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Response Ops][Alerting] Possible "experimental" ECS fields are not backwards compatible for alerts as data

6 participants

@ymao1 @elasticmachine @pmuellr @kibana-ci @vitaliidm @kibanamachine