-
Notifications
You must be signed in to change notification settings - Fork 418
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
[RFC] Wildcard - stage 3 proposal #1015
Conversation
I saw a few typos. These say "leading wildcard" but the example is a trailing wildcard: ecs/rfcs/text/0001-wildcard-data-type.md Lines 148 to 149 in e9b9681
Inconsistent escaping, should do ecs/rfcs/text/0001-wildcard-data-type.md Line 182 in e9b9681
Similarly, I think these should be escaped with ecs/rfcs/text/0001-wildcard-data-type.md Line 241 in e9b9681
Worth double checking if this is valid syntax. I'm not 100% sure if |
We should remove |
We could add a link to the wildcard docs in the references https://www.elastic.co/guide/en/elasticsearch/reference/current/keyword.html#wildcard-field-type |
Whoa, I just came across this
And here I thought people were as imaginative as me, calling their threads "thread1", "thread2", "worker1" and so on. I think we should make this field wildcard as well. |
@webmat - I, as well, must not be very imaginative 😂 I've captured |
Just like I mentioned in today's meeting, I think we should make obvious that this RFC is solely about the migration of pre-existing Using |
Yes agreed! I've actually made the suggestion to consider I will adjust the RFC's messaging to make it clear this RFC is documenting the initial proposal focused on existing fields, but any net new fields should consider using |
I made some adjustments to the title and intro trying to better focus on the migration over wildcard's general presence in ECS: I'm happy to flesh this out with more detail if that feels appropriate. |
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.
Noted a few things around the recent changes 👍
PR to apply |
Including The Wildcard is part of the
A simple path may be to omit @webmat @leehinman Any thoughts? |
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 we're good on the current list of fields to migrate. This LGTM other than a few small adjustments.
Nitpick: we're missing organization.name
in the table :-)
I'm good with reverting event.original
to keyword
with index: false
for now, as the way forward. We could adjust the field definition in #1098 to say it's currently not indexed, and that if users want to override this and make it indexed, they should consider wildcard
.
Co-authored-by: Mathieu Martin <webmat@gmail.com>
Co-authored-by: Mathieu Martin <webmat@gmail.com>
Co-authored-by: Mathieu Martin <webmat@gmail.com>
Latest round of changes:
|
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 looks good, let's change that date and merge 👍
RFC 0001 is also the first to reach stage 3 🎉 😃
Summary
Revisions to the stage two wildcard RFC for consideration of advancement for stage three.
Opening this PR with minimal changes. Feedback and concerns as the wildcard changes are adopted can be captured in this PR and incorporated into the proposal as appropriate.
Criteria for consideration
Markdown preview of this RFC