Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Better out-of-the-box mappings for logs, metrics and synthetics #64978
Better out-of-the-box mappings for logs, metrics and synthetics #64978
Changes from 3 commits
1100b95
dae84d6
90ca4b1
35d0727
ce72eef
2e130a5
7497f29
ce711a7
30e97b3
74cb397
67bed57
863bae7
5dc16be
9ea0a35
4707606
d6eb0af
4c04815
c9249b5
db58ab9
9950d8b
c4cb7fd
f7376e5
42c9f58
89eb06d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@jpountz With the recent development in ES, I started to ask myself if these should actually all be dynamic mapping for runtime fields? It would change the PR a bit as I think in this case we should bring back the global
message
field but everything else should match as runtime.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.
In addition, having runtime fields would allow us to potentially also have runtime fields for things like
*_time
or*_date
so these would work and still not be indexed. It would also allow an "easy" fix in case the detection was not correct (I assume).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.
Dynamic mappings are going to be a security issue. Are there other ways to do this?
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.
@ruflin Can you dig a bit more into why you're thinking about using runtime fields?
If you are worried of failing to index some documents, then maybe we should add
ignore_malformed:true
to theseip
mappings?If you would like to allow users to change their minds, then either runtime fields or concrete fields is fine since we allow runtime fields to shadow the definition of concrete fields, so users would have the leisure of configuring their field differently via a runtime field on existing indices if they wanted to regardless of the decision we would make here.
@scunningham There is no other way to do it. My understanding of the parallel thread on #68414 is that we would still allow dynamic mappings for trusted endpoints, so I believe that fine-tuning dynamic templates in these base templates still provides value.
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.
Lets assume we have a document with several
*.ip
fields inside. By default currently these are mapped to keywords which is not correct. So if someone wants to query for the*.ip
fields, the result is unexpected because of the wrong type. In the ideal case, these fields would not be indexed at all because my assumption is, in most cases these are not used. And if they are used, a more specific dataset with a template will likely exist and have the mappings for it.The above goes further: We currently index all the things as keyword. Instead we should only index a few fields and not index all the others and instead use runtime 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.
I think this holds true for our integrations, but we also need to take care of users who are onboarding new sources of data, e.g. because it's custom or because we don't have an integration for it (yet)?
My understanding is that
host.ip
should generally not be used, so we could make this one runtime. But the source IP field of anginx.access
dataset should be indexed in order to allow users to run cardinality aggregations on this field (how many users per day) or compare IPv4 traffic vs IPv6 traffic? In general it feels hard to me to make the assumption about the data that because it is an IP field, then generally it is not useful, even if that's the case most of the time in ECS.What about mapping
host.ip
as a runtime field and keeping that template for other fields whose name ends with.ip
and map them as indexedip
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.
I checked with @tsg and the Security solution leverages these IP fields in a dashboard, so we should keep them indexed, at least for now.
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.
@tsg What we discuss here is the default. Are the queries you run in the context of installed packages? If yes, these could still have a more specific definition that makes it index. The part here is only the global default if no other template exists.
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.
keep a single space after the colon?
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.
keep a single space after the colon?