[Logs] Scaffolding for Log Explorer within Discover #135913
Conversation
| @@ -43,6 +44,9 @@ export const discoverRouter = (services: DiscoverServices, history: History, isD | |||
| <Route path="/view/:id"> | |||
| <DiscoverMainRoute isDev={isDev} /> | |||
| </Route> | |||
| <Route path="/log-explorer"> | |||
| <DiscoverLogExplorerRoute isDev={isDev} /> | |||
There was a problem hiding this comment.
FYI: Discover uses hash based routing.
The saved search ID lands as a query parameter, not part of the path.
| </DiscoverTourProvider> | ||
| )} | ||
| <div className="dscDiscoverGrid"> | ||
| <DataGridMemoized |
There was a problem hiding this comment.
To be replaced with the new DataGrid functionality from the EUI POC.
| * State related logic | ||
| */ | ||
| const { | ||
| data$, |
There was a problem hiding this comment.
useDiscoverState calls through to useSavedSearch this is where the bulk of the data querying heavy lifting happens. This is where we'll want to refactor the querying logic. The data$ observable in standard Discover would contain the documents, chart data, and total hits for populating the count.
| id: id || 'new', | ||
| }); | ||
|
|
||
| const loadDefaultOrCurrentIndexPattern = useCallback( |
There was a problem hiding this comment.
We don't really need to change this, as it works fine, and we want the exact same functionality. But, this is a bit of a loaded function, it does do what it says, but it also performs a lot of other logic (such as various no data states, determining whether there are data views, whether there is ES data etc).
There was a problem hiding this comment.
One thing is the logic to detect the no data state. this can stay as it's similar in other places. other logic is about the saved search and data view(s). this should be refactored (It's a leftover how it worked in the Angular world)
| return; | ||
| } | ||
|
|
||
| const { appStateContainer } = getState({ history, uiSettings: config }); |
There was a problem hiding this comment.
I found this function a little surprising. It sets up the state container, state storage, and calls syncState, but it seems that this function gets called in multiple places within Discover. We might want to look at only calling this once, via a provider / context.
Okay, that might actually just be because of the legacy Discover.
There was a problem hiding this comment.
yes, it's definitely a leftover :). there should just be one location the appStateContainer is created
|
|
||
| const { appStateContainer } = getState({ history, uiSettings: config }); | ||
| const { index } = appStateContainer.getState(); | ||
| const ip = await loadIndexPattern(index || '', data.dataViews, config); |
There was a problem hiding this comment.
loadIndexPattern doesn't return an index pattern directly, it returns IndexPatternData which contains the list, but also some extra properties like stateVal and stateValFound, which are used in other parts of code to determine certain things (like showing a toast via resolveIndexPattern).
| hasData: { | ||
| ...data.dataViews.hasData, | ||
|
|
||
| // We've already called this, so we can optimize the analytics services to |
There was a problem hiding this comment.
In general Discover seems to have more granular (and probably much more meaningful) analytics. We probably want to bear this in mind for functionality we add.
…fstuermer' into 134764-log-explorer-randd
…a into 134764-log-explorer-randd
…-ref HEAD~1..HEAD --fix'
…a into 134764-log-explorer-randd
- Still some areas WIP but merge conflicts have been fixed here
…achine is in an initialized state
…a into 134764-log-explorer-randd
…-ref HEAD~1..HEAD --fix'
…a into 134764-log-explorer-randd
|
/oblt-deploy |
💔 Build FailedFailed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
This is a awesome & HUGE PR in every way 👍 , started to dig into it, test it and onboard on xstate, just wanted to report a first issue I've found while scrolling. What I did was scrolling up and down, so I don't think I did something forbidden:
Uncaught Error: Expected both chunks to have status "loaded", but found "loaded" and "empty".
at load_before_service.ts:113:1
at utils.js:413:1
at Array.reduce (<anonymous>)
at Object.updateContext (utils.js:401:1)
at Object.resolveActions (actions.js:442:1)
at StateNode.resolveTransition (StateNode.js:890:1)
at StateNode.transition (StateNode.js:829:1)
at interpreter.js:901:1
at Object.provide (serviceScope.js:12:1)
at Interpreter._nextState (interpreter.js:900:1)
BTW will the jumpy behavior of the scrollbar when loading improve when iterating on the grid? And I guess showing a loading indication would be something that's next, right?
hm, there might be some edge cases we didn't handle correctly in the prototype. especially the empty and loading failed states are not fleshed out. we tried to get the "happy" path working within the time restrictions.
Yes, when I write it again I would not initialize an empty chunk when loading starts, but only when it finishes. That way there should be less jumping.
Indeed, we didn't include that in the prototype, but the state can be easily used to render something. We were imagining an indicator that hovers over the grid at the top or bottom to work around the grid's row rendering limitations. |
| } = useDiscoverStateContext(); | ||
|
|
||
| // Inspector | ||
| // TODO: Fix this and move to independent hook |
There was a problem hiding this comment.
here we go!
#140466
good call, thx!
| sortCriteria: [ | ||
| // TODO: don't hard-code this | ||
| [dataView.timeFieldName!, 'asc'], | ||
| ['_doc', 'asc'], // _shard_doc is only available when used inside a PIT |
Summary
Ticket: #134764 (comment)
This adds a new
/log-explorerroute within Discover to serve as a "base" for the Log Explorer POC. There isn't really any new code here, it's mostly copied and pasted from the existing Discover code and then slightly altered. We can adjust further and remove certain functionality (that we don't need) as we go. We'll also be scoping down types, props, and state to those we need. There will also be a state machine to back the more complex state.This is a preliminary draft PR to mainly collaborate on code, but we can also have discussions here pinpointed against certain lines.
I've left some initial comments which highlight things that I found interesting, confusing, or useful putting in the scaffolding.