Skip to content

Conversation

@jsoriano
Copy link
Member

Proposal to add dynamic mappings for non-indexed ECS fields.

This is needed for some fields like event.original that are documented and defined in ECS as non-indexed, with index: false and doc_values: false.

@jsoriano jsoriano requested a review from a team March 19, 2024 16:08
@jsoriano jsoriano self-assigned this Mar 19, 2024
@jsoriano
Copy link
Member Author

/test integrations

@mrodm
Copy link
Contributor

mrodm commented Mar 21, 2024

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repository to test this version. Check elastic/integrations#9406

@jsoriano
Copy link
Member Author

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repository to test this version. Check elastic/integrations#9406

type: keyword
index: false
doc_values: false
path_match: 'event.original'
Copy link
Member Author

Choose a reason for hiding this comment

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

If we just apply these changes here, rebuilding current packages will give different results. So I think we may need to include them only after something is updated in the package. I see two options for this:

Or we can just go on and apply it to existing packages if rebuilt, as this can be considered a fix, even if it breaks a bit of reproducibility.

@ruflin @flash1293 @mrodm thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think users will insist on this level of reproducibility - it's unlikely anyone actually relied on this behavior and by not requiring the integration author to jump through hoops, we make it easier for this fix to get adopted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking into this it would be great that it would be easy for owners to get this fix in their packages without any change (e.g. updating package-spec version) to not force to more changes (e.g. new validation rules).

So, it looks ok to me if this applied as it is too.

@jsoriano jsoriano marked this pull request as ready for review March 26, 2024 13:44
@jsoriano
Copy link
Member Author

/test

@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 26, 2024

💔 Build Failed

Failed CI Steps

History

cc @jsoriano

@jsoriano
Copy link
Member Author

Unrelated failures in benchmarks.

@jsoriano jsoriano merged commit a44250e into elastic:main Mar 26, 2024
@jsoriano jsoriano deleted the ecs-index-false branch March 26, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants