-
Notifications
You must be signed in to change notification settings - Fork 117
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
Add support for multifields added by ecs@mappings #2035
Add support for multifields added by ecs@mappings #2035
Conversation
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10859 |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10859 |
test integrations |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10859 |
💚 Build Succeeded
History
cc @jsoriano |
This reverts commit 811fc3f.
Reverting the change to enable logsdb by default, to see what would be the current status of failures by applying only the changes for the multifields added by |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10859 |
@@ -383,6 +389,86 @@ func allVersionsIncludeECS(kibanaConstraints *semver.Constraints) bool { | |||
// return !kibanaConstraints.Check(lastStackVersionWithoutEcsMappings) | |||
} | |||
|
|||
func ecsPathWithMultifieldsMatch(name string) bool { |
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.
Ideally, I'd like to avoid hard coding things from ecs@mappings
here as changing that component template will cause tests to fail somewhere else.
While we're figuring the right path of whether to adapt ecs@mappings
or ECS itself (elastic/ecs#2353), could we make the validation less strict and not fail if the mapping contains additional sub-fields?
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.
Ideally, I'd like to avoid hard coding things from ecs@mappings here as changing that component template will cause tests to fail somewhere else.
If ecs@mappings
doesn't differ further from ECS we should be fine, and hopefully this should be the case.
While we're figuring the right path of whether to adapt
ecs@mappings
or ECS itself (elastic/ecs#2353), could we make the validation less strict and not fail if the mapping contains additional sub-fields?
The problem is that if we relax a constraint it is difficult to put it back later. This is why I would keep this to the minimum we know.
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
ecs@mappings
component template adds multifields that are not defined in ECS or in packages using these mappings. These fields are present in the ingested documents and cannot be validated byelastic-package
.Add the definitions for these multifields when
ecs@mappings
is used and no other definition is present.Also, improve the error reported when there is no definition for a possible multifield.
Closes #1971