[Discover] Implementing click action for Stacktrace and Degraded Fields on Discover

[achyutjhunjhunwala]

Closes: #190670

The PR adds click actions to Stacktrace and Degraded Fields Actions in Discover.

For Eg - When you click on stacktrace icon in Discover, it opens the Flyout and Scroll Into View the Stacktrace Section

Whats pending

• Stateful FTR Tests - Discover
• Stateful FTR Tests - Discover inside a Dashboard
• Serverless FTR Tests - Discover
• Serverless FTR Tests - Discover inside a Dashboard
• Appropriate Unit tests

Test Plan

• 'should open the flyout with stacktrace and quality issues accordion closed when expand is clicked'
• 'should open the flyout with stacktrace accordion open and quality issues accordion closed when stacktrace icon is clicked',
• 'should open the flyout with stacktrace accordion close and quality issues accordion open when quality issues icon is clicked'
• 'should keep old accordion open when 1st stacktrace and then quality issue control for the same row is clicked'
• 'should toggle to quality issue accordion when 1st stacktrace and then quality issue control is clicked for different row'
• 'should keep old accordion open when 1st quality issue and then stacktrace control for the same row is clicked'
• 'should toggle to stacktrace accordion when 1st quality issue and then stacktrace control is clicked for different row'
• 'should switch tab to logs overview and open quality issues accordion, when user clicks on quality issue control of same row and flyout is already open with some other tab'
• 'should switch tab to logs overview and open quality issues accordion, when user clicks on quality issue control of different row and flyout is already open with some other tab'
• 'should switch tab to logs overview and open stacktrace accordion, when user clicks on stacktrace control of same row and flyout is already open with some other tab'
• 'should switch tab to logs overview and open stacktrace accordion, when user clicks on stacktrace control of different row and flyout is already open with some other tab'

[achyutjhunjhunwala]

@davismcphee @weltenwort I am working on this feature where when the user clicks on Stacktrace or DegradedField icon on Discover, it should -

• open the flyout
• expand the chosen section (by default both these accordions are collapsed)#
• scroll into view the sections

I have opened this Draft PR just to discuss the code approach with both of you to understand how we can actually achieve this. Please keep in mind this PR is not a complete work and only sort of a Demo Work and only works for Stacktrace icon.

What i have done

1. To achieve opening flyout on click for the leading control columns, i have to pass the setExpandedDoc function around. Please let me know if you think if there is another way of doing this.

2. The biggest challenge i am facing is around the state of accordions. Since these accordions are always closed by default, they must be kept opened when the user clicks on the icon. This means we need some sort of state management. I am not sure if a URL based state management does work here or not. I haven't explored but open to hear your opinion given discover also works as embeddables. What i did was i introduced a top level useState in discover_documents (can do the same for embeddables by doing it in saved_search_grid.tsx which is the top level file there)and passed the setter to dataGrid component and the getter to the docViewer. But now its not so simple -

• When user clicks on the stacktrace icon, it opens the Accordion
• But if you close the accordion now, we need to again reset the state again.
• If a user clicks on a regular expand icon next, since the useState is persisting the previous state, the accordions are always open.

Basically just to maintain the accordion state, its super nasty prop drilling which i have to do. The question here is

• Is there a better way you think this can be done ?
• If we keep these accordions always open, we may be don't even need this complexity.

3. Finally to scroll the item into view, i am relying on data-test-sub, i will change that to a custom id in future. Its done simply for this demo work.

[weltenwort]

Since these accordions are always closed by default, they must be kept opened when the user clicks on the icon.

Could you help me understand this? Kept open when clicking which icon?

I am not sure if a URL based state management does work here or not.

Currently the fly-out doesn't seem to be represented in the url state at all, so it wouldn't be much help here. And, as you mentioned, the embeddable use-case forbids that anyway.

---

Maybe this is not the right place to open that discussion again, but wouldn't it be cleaner to have the stack trace on a separate tab? Then we could forego all that scrolling business and just enhance the doc viewer to take an initial active tab.

[achyutjhunjhunwala]

Could you help me understand this? Kept open when clicking which icon?

@weltenwort The leading-column-icon present in discover table. We have 2 icons, for obs, 1 for Stack trace and other for Degraded Docs.

Maybe this is not the right place to open that discussion again, but wouldn't it be cleaner to have the stack trace on a separate tab? Then we could forego all that scrolling business and just enhance the doc viewer to take an initial active tab.

The idea does solve the problem with state management, but this will only solve the problem for StackTrace. We have Quality Issues accordion as well (second icon). That would mean we will have to have Quality Issues / degraded Fields as well in a separate tab which i doubt would scale well then.

[weltenwort]

I see, thanks for the clarifications. While appreciate the attempt to work around the limitations of the fly-out API right now, establishing all these side-channels to juggle the collapse and scroll state seems brittle to me.

How feasible would it be to enhance the fly-out API (setExpandedDoc) to allow carrying more information like an initial state hint, which the overview tab can use to initialize the accordion expansion state without leaking anything about its internals? Please excuse my ignorance if that is too naive 😇

[davismcphee]

@achyutjhunjhunwala Thanks for looking into this. I took a look at it today, and the overall direction makes sense to me, but I agree with @weltenwort's thoughts on not leaking the overview tab internals into Discover or Unified Doc Viewer.

The suggestion to extend setExpandedDoc to pass along more info seems like it could work, and might be necessary to some degree. I tested locally and noticed the action doesn't work as intended if the user's last tab wasn't "Log overview" since the doc viewer restores the last used tab by default. So I think in any case we'll need a top level API change to Unified Doc Viewer to support opening an initial tab, and we could use setExpandedDoc to pass that from extension points to Discover.

For managing overview tab state, we could try something like passing an untyped context object through Unified Doc Viewer into the tab component where it could be casted, or another option would be to use the profile state management tools the context awareness framework currently offers. Your two options would be using React context + the getRenderAppWrapper extension point in a root profile, or using custom context objects (or a combo of both). These approaches are documented with examples here in our docs.

I took a quick stab at modifying the approach to use custom context objects instead and created an example PR: #215072. I threw it together quickly so it's not very elegant or mergeable, but it should give an example of one of the options 🙂

[achyutjhunjhunwala]

@weltenwort @davismcphee Thank you both for your comments.

@davismcphee Highly appreciate the PR you did, gave me a better idea of how the Profile work and i must appreciate the documentation around Context Awareness which you shared is real Gold. I was not aware that it exist. Saved me quite a lot of time.

I have achieved the functionality which i intended with 1 minor bug, which i am unable to figure out on how to fix it. I must day this was a complex problem to solve 😆

What is expected

I am pointing our the Scenarios which this logic should handle and what i was able to achieve and where is the bug

Scenarios -:

1. User clicks on expand icon - ✅

• The flyout open normally
• With 1st tab
• With Stacktrace and Quality Issue accordions collapsed

2. While the flyout is still open, user click on stacktrace icon of the SAME row. ✅

• The Stacktrace accordion expands
• Screen scrolls to show this section in view.
• Quality Issue Accordion is collapsed.

3. While the flyout is still open, user clicks on Quality issue icon of the SAME row, ✅

• The Quality Issue accordion now expands,
• Screen scrolls to show this section in view,
• Stacktrace section must be collapsed

4. While the Flyout is still open, user clicks on any of the Quality issue icon or Stacktrace Icon of a Different Row, ✅

• The corresponding document updates in the Flyout
• The selected section accordion expands.
• The selected section scrolls into the view
• The other section accordion must stay collapsed

5. While the Flyout is still open, user goes to JSON or Document Tab, now user clicks on the same row or any other row Stacktrace or Quality Issue Icon ✅

• All the above expected for Scenarios 4 work as expected.

6. User clicks on Stacktrace or Quality Issue icon, which opens the flyout. Now the user closes the flyout. Now the user uses the Expand icon to open the Flyout again.

• Flyout should open normally with both Stacktrace and Quality Issue Accordion collapsed. 🔴
  Unfortunately this is not happening. Even if user tries to collapse the accordion, close the flyout and then again click on Expand icon, the flyout always opens with the last expanded accordion.

This happens because the Behavior Subject's last value was set when click action on Stacktrace-Quality Issue icon happens and we cannot reset it.

Given i need to remount the component again to handle tab switches scenario, i am unable to find a spot to pass undefined to the Behavior Subject, once the action is completed.

I hope i am able to explain the problem here. I will open this code for review now and start working on tests so that the other parts of the PR can be reviewed.

[achyutjhunjhunwala]

@davismcphee @tonyghiani Hi Both

i have merged the last bit of pending review comments. Can you please help me with an approvals

[davismcphee]

We're so very close! Just noticed one potential issue with the latest commit.

[davismcphee]

I believe the issue from the video I shared is still present, but I really want to unblock this PR so we can get all these nice test fixes in, and we can follow up on that issue separately 👍

Thanks again for all the effort put into this, I know it was probably super frustrating but also much appreciated. Now let's merge this thing and never speak of it again! (Kidding of course, let's consider how we can make things less painful for future contributors)

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: 7885e1c

Failed CI Steps

• Defend Workflows Cypress Tests #5

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
unifiedDocViewer	312	313	+1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/ftr-common-functional-ui-services	515	517	+2
@kbn/unified-data-table	109	111	+2
@kbn/unified-doc-viewer	17	20	+3
unifiedDocViewer	12	15	+3
total			+10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
discover	1002.2KB	1004.2KB	+2.0KB
securitySolution	9.1MB	9.1MB	+1.6KB
unifiedDocViewer	209.3KB	210.7KB	+1.4KB
total			+4.9KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
@kbn/unified-doc-viewer	5	4	-1

Unknown metric groups

API count

id	before	after	diff
@kbn/ftr-common-functional-ui-services	555	557	+2
@kbn/unified-data-table	188	190	+2
@kbn/unified-doc-viewer	18	22	+4
unifiedDocViewer	19	22	+3
total			+11

History

• 💔 Build #298774 failed 8c0b7c7
• 💔 Build #298617 failed 2b9fcff
• 💔 Build #298519 failed 870e393
• 💔 Build #298299 failed 2d072c8
• 💚 Build #297149 succeeded 326e85f

cc @achyutjhunjhunwala

[gbamparop]

@achyutjhunjhunwala shouldn't this be backported to 8.19 too?

[achyutjhunjhunwala]

@gbamparop thats a good question. I don't remember the backport rule we are currently having, but my understanding was all new features in 9.x and only important bug fixes in 8.19.

[flash1293]

All features should be backported to 8.19 as well, so this PR applies

[achyutjhunjhunwala]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation