Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Logs UI] Refactor log stream page time state #145137

Closed
Tracked by #142714
, #134412
...
weltenwort opened this issue Nov 14, 2022 · 1 comment · Fixed by #149052
Closed
Tracked by #142714
, #134412
...

[Logs UI] Refactor log stream page time state #145137

weltenwort opened this issue Nov 14, 2022 · 1 comment · Fixed by #149052
Assignees
@Kerry350
Labels
Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@weltenwort
Copy link
Member

weltenwort commented Nov 14, 2022 •
edited
Loading

📓 Summary

⚠️ In light of our learnings about the interaction between the timefilter and the saved query, we might want to add this functionality to the existing LogStreamQuery state machine instead of creating a separate machine for this. Talk to @weltenwort before picking this up.

In order for the time state management to play well with the page state machine, we want the time-related state to be managed by a dedicated machine that is spawned by the page state machine in the hasLogView state.

✔️ Acceptance criteria

  • The time-related state is managed by a dedicated state machine.
  • The state machine syncs the query to the timefilter service.
  • The state machine syncs the query to the url.
  • The hasLogView state waits for the initial time range (from url and service) until it transitions to initialized.
  • An adapter provider makes data from the time machine's context available to the log stream page components.

💡 Implementation hints
@weltenwort weltenwort added Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Nov 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

This was referenced Nov 14, 2022
Closed
Closed
@weltenwort weltenwort mentioned this issue Dec 2, 2022
Merged
@weltenwort weltenwort mentioned this issue Jan 3, 2023
Closed
@weltenwort weltenwort assigned weltenwort and unassigned weltenwort Jan 5, 2023
@Kerry350 Kerry350 self-assigned this Jan 9, 2023
@Kerry350 Kerry350 mentioned this issue Jan 17, 2023
Merged
@weltenwort weltenwort mentioned this issue Feb 6, 2023
Closed
Kerry350 added a commit that referenced this issue Feb 21, 2023
@Kerry350 @weltenwort
[Logs UI] Refactor Time and Position log stream state (#149052)
8583231 
## Summary

Closes #145137

Initially this issue was just going to include moving the time context
into the query state machine. However, this actually made working with
the dependant log position code a lot harder. As such the log position
code has also been moved to it's own state machine.

## 🕵️‍♀️ Reviewer hints and notable changes

- There are some comments left inline (*previous logic* notes might be
useful)

- There is now a new machine for dealing with Log Position state (target
position, latest position, visible positions).

- Time based context (time range, timestamps, refresh interval) is now
moved to the query machine (this will also make dealing with saved
queries easier).

- The page state machine is the only machine that the UI interacts with
(either reading context or sending events). The page state machine
co-ordinates forwarding necessary events to other internal machines.

- Ensure relevant notifications reach their targets, e.g. when time is
changed, positions should also update.

- [There is some documentation regarding URL state and
precedence](f9ca0f7).

- `updateContextInUrl` now always sets the full URL-relevant context in
the URL when called (since the `urlStateStorage.set()` call replaces the
whole key).

- Within the Log Stream Query state machine the `initialized` state node
is now modelled as a parallel state node, this is so `query` and `time`
can act independently (time needs to handle things like the refresh
interval, for example).

## 🕹 Testing

(Just some ideas)

- Can the time range be changed?
- Can the refresh interval be changed?
- Is state synchronised to the URL and to relevant Kibana services (time
filter service etc)?
- When streaming is enabled, are requests dispatched correctly on an
interval?
- Do positions update correctly whilst interacting with the stream?
(scrolling etc)
- Does the backwards compatibility of initialising from the URL work?

## 🎨 State machine diagrams

### Log stream page

![Screenshot 2023-02-06 at 15 29
27](https://user-images.githubusercontent.com/471693/217014687-1e1b6ae4-a7b5-4e3c-930f-5d9cf7406453.png)

### Log stream query

![Screenshot 2023-02-06 at 15 31
38](https://user-images.githubusercontent.com/471693/217014757-7c6b1788-65a2-460e-abb6-a1f9fee1131a.png)

### Log stream position

![Screenshot 2023-02-06 at 15 30
36](https://user-images.githubusercontent.com/471693/217014809-017a4b23-9a4a-4784-8ac3-36cbbecfd72e.png)

## ⚠️ Warnings

- [There is a known bug with
streaming](#136159 (comment)).
- [There is a known issue with a console
error](#149052 (comment))

---------

Co-authored-by: Felix Stürmer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Kerry350 Kerry350

Labels
Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Logs UI] Refactor Time and Position log stream state Kerry350/kibana
3 participants
@Kerry350 @weltenwort @elasticmachine