-
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
[Fleet] Generate dynamic template mappings for field with wildcard in name #137978
[Fleet] Generate dynamic template mappings for field with wildcard in name #137978
Conversation
9a937a9
to
6255698
Compare
Pinging @elastic/fleet (Team:Fleet) |
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.
One minor refactor suggestion otherwise LGTM
export function processFieldsWithWildcard(fields: Fields): Fields { | ||
const newFields: Fields = []; | ||
for (const field of fields) { | ||
if (field.name.includes('*') && (field.type !== 'object' || !field.object_type)) { |
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 condition may be complex enough to warrant breaking out into a few variables, e.g.
const hasWildcard = field.name.includes('*');
const isObjectField = field.type === 'object' || field.object_type;
if (hasWildcard && !isObjectField) {
// ...
} else {
// ...
}
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.
Looks good, thanks!
const newFields: Fields = []; | ||
for (const field of fields) { | ||
const hasWildcard = field.name.includes('*'); | ||
const hasNotObjectType = !field.object_type; |
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 would probably be a bit more sensible as a "positive" condition that's negated below
const hasObjectType = !!field.object_type
if (hasWildcard && !hasObjectType) ...
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.
yes it make sense, having a bad time naming variables on this one
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
… name (elastic#137978) (cherry picked from commit 88eae3e)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Description
Resolve #129344
Fleet create datastream mappings based on fields yaml provided by the integration, it was decided that the fields that contains a
*
in the name should be installed as dynamic template and not property mappings that PR change that.If a field name contains a
*
the field will be transformed to be of typeobject
with${object_type:field.type}
, for example the followed field will be transformed like thisAnd with the work introduced in #137772 this will result in dynamic_templates in the final mappings.
@jsoriano Does it make sense to you?