-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Logs UI] Correctly update the expanded log rate table rows #60306
[Logs UI] Correctly update the expanded log rate table rows #60306
Conversation
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
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.
Looking at this today, good catch 👍
I suppose it's important to also add: the examples are definitely updating when I update the time range :) 👍 |
@elasticmachine merge upstream |
Thanks for the review, @jasonrhodes. This PR changes the log rate tab while your screenshots show the log categories tab. That aside, could you elaborate in which way these timestamps violate your expectations? Both seem to span 24 hours, with one aligned at your local day boundaries (today) while the other is aligned to your local time (now-24h to now). The error message about the missing tab property in the datepicker seems to be a bug in EUI, which can be observed everywhere in Kibana and has to be resolved upstream. I'll open an issue or PR there. |
@elasticmachine merge upstream |
@weltenwort sorry, I'll check Log Rate and report back. As for the screenshots, if I have results from today, I'd expect them to show up in "Today" and in "Last 24 hours", and I can't easily explain why they wouldn't. |
"Last 24 hours" also covers parts of yesterday most of the time, so assuming the time of testing was before 16:49:57 on March 18 the first screenshot seems valid. I guess you're surprised that the five samples are not the newest ones in respect to the time range? |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Yes, exactly. How are those chosen? |
right now it's ascending by |
Interesting so it's the earliest ones in the given window. I think I expect the most recent ones but I have no idea if I'm representative there. Anyway that's not part of this PR though. :) |
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.
Fix for the log rate tab LGTM.
…60306) This ensures that the content of the expanded rows in the log rate table reflect the most recent results. Fixes elastic#60300
Backports the following commits to 7.x: - [Logs UI] Correctly update the expanded log rate table rows (#60306)
* master: [ML] Use a new ML endpoint to estimate a model memory (elastic#60376) [Logs UI] Correctly update the expanded log rate table rows (elastic#60306) fixes drag and drop flakiness (elastic#60625) Removing isEmptyState from embeddable input (elastic#60511) [Cross Cluster Replication] NP Shim (elastic#60121) Clear changes when canceling an edit to an alert (elastic#60518) Update workflow syntax (elastic#60626) Updating project assigner workflows to v2.0.0 of the action and back to default tokens (elastic#60577) migrate saved objects management edition view to react/typescript/eui (elastic#59490)
Summary
This ensures that the content of the expanded rows in the log rate table reflect the most recent results.
Fixes #60300
Testing
See #60300 for reproduction of the fixed issue.