-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Enables the EventLog Client to query across ILM versions of the .event-log index
#81920
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
Enables the EventLog Client to query across ILM versions of the .event-log index
#81920
Conversation
.event-log index
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
.event-log index.event-log index
pmuellr
left a comment
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; made a comment about the yuge mappings that got added. Main worry is what happens to this over time, but I think it's basically fixed in time, so ... go for now; if we have a non-trivial change to the event_log over time we need to account for, that would be more work anyway ...
| @@ -0,0 +1,2685 @@ | |||
| { | |||
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.
yikes! you'd think there would be something smaller you could do, heh.
Is this needed because the test is using the `event_log_test_ saved object fixture type? We could probably change that, or create a different test, that used alerts instead, if it would avoid having to pull in this massive wad.
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.
Yeah, it's a pain... esArchiver isn't the esasiest to debug when it just decides not to load the archive anymoe so left it as is.
I've now pruned a bunch of the mappings... 🤞 lets hope it passes
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.
Not a big deal, wouldn't spend a lot of time on it. This will always just be testing the 7.9 -> 8.0 cross-over, so it doesn't seem like it will ever need to change in the future.
YulNaumenko
left a comment
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
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
…nt-log` index (elastic#81920) Fixes a bug in the EventLog client which caused it to query for events created in the current version instead of querying across versions.
Summary
closes #81274
Fixes a bug in the EventLog client which caused it to query for events created in the current version instead of querying across versions.
Checklist
Delete any items that are not applicable to this PR.
For maintainers