Hide "analyses" that do not provide value to the user

[ssmagula]

Last week we ran some evaluative user research sessions. We used the "concurrent think aloud protocol" to understand and start to measure how easy or hard it is for experienced developers to perform basic tasks relating to tracing. Nutshell: these participants were not able to complete most tasks successfully without multiple hints and coaching. Even after they received several hints and managed to load the previously captured traces, they still faced other hurdles (see one example below). Seeing the hurdles more clearly is the first step to removing them and I hope these observations will soon enable us to come up with a more discoverable workflow to load traces, and to get started with analysis.

Below is one of many observations and implications for the product that came from last week's sessions. I'll add others, soon, after we review and prioritize a bit on Friday.

Why hide analyses that aren't yet fully functional?
After receiving some hints on how to load traces successfully, participants saw a list of "available analyses." To familiarize himself with the plugin, one participant clicked on the analysis at the top of the list, and saw a blank chart and text that said "Loading analysis..." he waited a while...then said "Is this thing still working? Is it stuck? How long will this take? Should I keep waiting here, I don't know..." Eventually he gave up, and clicked on the next analysis, which also did not work. He said something like "If it's still loading, why not show a little spinner or something, but if it's not working, why show anything at all?"

[bhufmann]

Thanks for raising this issue. It gives us some valuable feedback. I'd like give some background information about this topic.

The available analysis are provided by the trace server when using the trace server command below [1]. The theia-trace-extension is just showing what was returned from the server. Some of the providers available on the server are not implemented on the client side (i.e. type DATA_TREE e.g. function duration statistics). Some others don't have limited support on the client site (e.g. XY charts). Some others don't seem to work well.

• For DATA_TREE type the theia-trace-extension opens a placeholder view stating that it hasn't been implemented. We could filter all DATA_TREE analyses from the list of available analyses instead until it's supported client side.
• XY-Charts, I think we can keep, even if view capabilities are still limited. Improvements will come in gradually and can prioritized
• For any of the views, that don't work or don't add any value we can decide to not show them at all. I would suggest to filter them out on the server side until they are functional.
• Providing a little spinner when things are ongoing is a good suggestion.

[1] https://theia-ide.github.io/trace-server-protocol/#operation/getOutputs

[ebugden]

Is this summary correct? : The list under "Available analyses" is the result of a "get all available analysis data sources" call to the server, but not all the corresponding visualisations are implemented in the front end.

bhufmann: I would suggest to filter [views that don't work] out on the server side until they are functional.

If this is simple to do, that would be perfect. Because then the front end would never request data it can't correctly visualize.

[bhufmann]

Yes, as a first step we can do that in the trace server. Then we can reassess if there is more to do.

[ebugden]

Wouldn't it be better to do the filtering in the frontend since that's where the missing code is? My understanding is that the server returns a list of all available data providers, but that the associated visualisations are not all implemented on the front end.
Maybe before populating the list of available views the front end could filter out the items that aren't implemented.

If we filter handpicked data providers on the server side because of the front end, we're breaking scope.

[MatthewKhouzam]

I see possible two issues.

• The back-end has part of the data only.
• The front-end can't display what the back-end is giving.

I suspect this issue needs to be attacked on both sides.

[ebugden]

Here is the current strategy for resolving this issue (updated). Basically:

• Server should specify all analyses it can perform
• Client should filter out any analyses it can't visualize

Server side

• Unfinished data providers should be filtered in the server: There are some placeholder data providers that are included in the list sent from the server even though the associated models aren't fully implemented yet. (see below)
  • Selected solution: Delete the DataProviderFactory of non functional analyses. (Suggested by @MatthewKhouzam below)
    • Easily reversible, easy to maintain
  • Alternative - Blacklist: A quick fix would be filtering non-functional data providers individually using the id (a whitelist wouldn't make sense considering how many data providers there are).
    • This breaks the client by default if a new in-development data provider is added.
    • It requires remembering to update the blacklist.
  • Alternative - Robust, but code heavy: A more readable and robust solution could be to add a boolean (ex. implemented) that would be true if the data provider was finished and defaults to false.
    • More readable filter code: "filter(!implemented)" vs. "filter(id is in [12,55,89,23,7]) //Filter non-functional analyses"
    • Doesn't break code by default: Dev has to specify the boolean
    • Involves changing allll the existing data providers

The initial patch that removed analyses by deleting the data providers was rejected because:

• All the views initially determined to be filtered server-side can correctly provide data to the front end (see details from @MatthewKhouzam in issues linked below).
• Since these views only misbehave for the Theia front-end and are still used happily by Eclipse Trace Compass, removing views by removing the data provider breaks Eclipse Trace Compass (see @bhufmann's comments on the patch for details).

Based on these points, we're changing our approach and will fix the views for the Theia front end rather than remove them:

• Views that remain spinning if the required events aren't in the trace: Inform user when there is no data to perform analysis #406. @jeffGodonou will work on this.
• Views that are slow to load: Certain views are unusually slow to load #405. No action needed for now.
  • Implementing Add analysis progress bar #284 will make this less confusing.

Client side

Any views where the analysis works but can't be visualized properly because of features missing in the front end should be filtered in the front end.

• Hide analysis that is not implemented on the client side #391, Filter XY chart type #472
  • View types have "output-component" in their name and are in the components folder (ex. xy-output-component, timegraph-output-component, table-output-component, abstract-tree-output-component). This can be used to create a whitelist of views that are implemented.
  • The "type" field in the OutputDescriptor can be used to filter: DATA_TREE, TIME_GRAPH,TREE_TIME_XY, TABLE
  • The filtering should be implemented in getOutputDescriptors( ) in the trace-explorer-views-widget.tsx
• XY Chart styles (ex. scatter plot, bar chart, etc.) that aren't yet implemented should be filtered client side: Right now only the "line" chart style is supported so views with other chart styles than that should be filtered out.
  • @IbrahimFradj is intended to implement this (unless XY Chart: Add area and scatter chart styles #399 gets merged first).
  • Each data series in an xy chart has a label that specifies chart style. So each series needs to be checked.
• As a quick fix client side while the server changes are being completed, views that will eventually be filtered server side can be displayed in a faded grey (to imply they aren't available). Like this:
  

[MatthewKhouzam]

Unfinished data providers should be filtered in the server: There are some placeholder data providers that are included in the list sent from the server even though the associated models aren't fully implemented yet.

* **A quick fix** would be filtering non-functional data providers individually using the id (a whitelist wouldn't make sense considering how many data providers there are).
  
  * This breaks the client by default if a new in-development data provider is added. It requires remembering to update the blacklist.

* **A more readable and robust solution** could be to add a boolean (ex. implemented) that would be true if the data provider was finished and defaults to false. I think @MatthewKhouzam suggested this at some point?
  
  * More readable filter code: "filter(!implemented)" vs. "filter(id is in [12,55,89,23,7]) //Filter non-functional analyses"
  * Doesn't break code by default: Dev has to specify the boolean

How about flat out deleting the DataProviderFactory... that way we don't need to implement anything extra? less code to maintain. ;)

[ebugden]

MatthewKhouzam: How about flat out deleting the DataProviderFactory... that way we don't need to implement anything extra? less code to maintain. ;)

I think we should go with this solution. I'll update the original comment.

[IbrahimFradj]

After doing some tests and checking the views that were not working, I came up with this result

The views do not work well :

Views	Problem	Solution
Irq Analysis – latency vs time	Does not work with a single trace( the view stay in analysis running), you need a folder with several traces for it to work.	the server should not send this view to the client side if it is just a single trace. filter by id and number of traces
Futex contention analysis- latency vs time	Does not work with a single trace ( the view stay in analysis running), you need a folder with several traces for it to work.	the server should not send this view to the client side if it is just a single trace. filter by id and number of traces
System call latency – latency vs time	Does not work with a single trace ( the view stay in analysis running) , you need a folder with several traces for it to work.	the server should not send this view to the client side if it is just a single trace. filter by id and number of traces
Ust memory latency vs time	Does not work, does not give results. the view is empty	remove the DataProviderFactory of this view from the server to not send it to the client.
Memory usage	Does not work, does not give results. it just shows a red line	remove the DataProviderFactory of this view from the server to not send it to the client.
Flame chart	Does not work, does not give results. the view is empty	remove the DataProviderFactory of this view from the server to not send it to the client.
Event Matching Latency - Latency vs Time	Does not work, does not give results. it stay all the time in analysis running unless you refresh the page	remove the DataProviderFactory of this view from the server to not send it to the client.

Here are the views that work well :

• Cpu usage
• Histogramme
• Resources status
• Thread status
• Event table
• Flamechart lttng ust callstack (incubvator)
• lttng ust callstack (incubvator) latency vs time
• lttng ust callstack – latency vs time

[ssmagula]

Nice work! Looks like we're on good footing to unfuddle this.

…

--Stefan

On Tue, Jun 15, 2021 at 10:45 AM IbrahimFradj ***@***.***> wrote: After doing some tests and checking the views that were not working, I came up with this result Views Problem Solution Irq Analysis – latency vs time Does not work with a single trace( the view stay in analysis running), you need a folder with several traces for it to work. the server should not send this view to the client side if it is just a single trace. filter by id and number of traces Futex contention analysis- latency vs time Does not work with a single trace ( the view stay in analysis running), you need a folder with several traces for it to work. the server should not send this view to the client side if it is just a single trace. filter by id and number of traces System call latency – latency vs time Does not work with a single trace ( the view stay in analysis running) , you need a folder with several traces for it to work. the server should not send this view to the client side if it is just a single trace. filter by id and number of traces Ust memory latency vs time Does not work, does not give results. the view is empty remove this view from the server to not send it to the client according to its Id Memory usage Does not work, does not give results. it just shows a red line remove this view from the server to not send it to the client according to its Id Flame chart Does not work, does not give results. the view is empty remove this view from the server to not send it to the client according to its Id Event Matching Latency - Latency vs Time Does not work, does not give results. it stay all the time in analysis running unless you refresh the page remove this view from the server to not send it to the client according to its Id — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#296 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIOFFRBQKMGMED2DMRQAWDTS5YQ3ANCNFSM4Y42HRMQ> .

[MatthewKhouzam]

Hey, we have one thing I forgot to mention, the viewer does not support "scatter" charts. Even if the dataproviderfactory is gone, on the view side we should filter it out too.

Example: Lttng ust callstack: latency vs time shows data but it's broken.

[jeffGodonou]

So we've tried to solved the issue by deleting the data providers factories. We've decided to change the approach and solve the issue with the handlers of the XY views.
Here is the link for the first method we used. It has not been approved.
https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/182397

[bhufmann]

I looked a bit closer which XY charts don't really work in the theia-trace-extension and found the following thing:

• Event Matching Latency - Latency vs Time: This is a scatter chart and won't ever provide any data and needs to be removed. The reason is that it only has data when an experiment with LTTng traces from multiple, communicating via TSP and the event matching is triggered with "Synchronize Traces". The "Synchronize Traces" feature doesn't exist in Theia yet and is only available in Eclipse Trace Compass.
• Futex Contention Analysis - Latency vs Time: This is a scatter chart with multiple series, where each series has it's own x-values (i.e. non-common-x-axis)
• LTTng-UST Callstack - Latency vs Time: This is a scatter chart with multiple series, where each series has it's own x-values (i.e. non-common-x-axis)
• Other Latency vs Time scatter charts

In all other XY charts, like the Histogram, the series have a common x axis, where each series have number of pixel (x,y) pairs. These type of XY work quite well and should be kept.

What then non-working XY charts of the Trace Compass Server above have in common is that they are created the same way, and are all scatter charts with non-common-x-axis for each series. That means drawing series with non-common-x-axis are not working at the moment and should be filtered out on the client side. The issue is that Data Provider Descriptor doesn't distinguish between both types, it only knows about chart type TREE_TIME_XY.

Another thing in common, is that all have IDs that start with org.eclipse.tracecompass.internal.analysis.timing.core.segmentstore.scatter.dataprovider.

How would you feel to filter them out by ID even if this only works for the Trace Compass specific server?

[ebugden]

@bhufmann Thank you for looking into this! Your comments about which analyses don't work seem consistent with a discussion I had with @arfio last week (I'll put the notes below for reference).

bhufmann: How would you feel to filter them out by ID even if this only works for the Trace Compass specific server?

I'm ok with this, but it does feel like a hack since the client is supposed to be server agnostic.

• Filtering client-side makes sense to me. If the client asks "send me all the analyses you can perform on this trace" and the server's response is correct and respects the TSP, it's the client's responsibility to decide whether it can visualize the data.
• Is the real problem that the data model returned by the server doesn't give the client enough information to determine whether or not it can visualize the data correctly? Ex. Needs to be visualized using a scatter, needs server-specific trace sync feature, etc. It would make sense to create issues for these eventual improvements now while we're thinking about it.

---

Which charts work?

Notes from recent discussion with @arfio about which charts aren't working well.

• Filter out XYs that have chart styles that aren't yet implemented
  • All latency vs. time (because they should be displayed as scatters)
  • Lttng callstack latency vs time (both the incubator and not incubator version work badly) (time sync doesn’t work, laggy (with only 3 functions selected), missing chart style (should be a scatter))
• Flame chart isn’t working well (but flame chart lttng ust works)
• XY charts that have value even with only relative information (no scale):
  • Memory usage
  • CPU usage
  • Disk IO
  • Histogram
• Memory Usage analysis is still acting up (spinning infinitely) for some unknown reason. My guess is Inform user when analysis fails #460 will fix this.

[bhufmann]

@bhufmann Thank you for looking into this! Your comments about which analyses don't work seem consistent with a discussion I had with @arfio last week (I'll put the notes below for reference).

bhufmann: How would you feel to filter them out by ID even if this only works for the Trace Compass specific server?

I'm ok with this, but it does feel like a hack since the client is supposed to be server agnostic.

Yes, it's a hack. I don't really like it myself and it makes me cringe. I don't see a better "quick workaround" right now, before they are actually implemented.

* Filtering client-side makes sense to me. If the client asks "send me all the analyses you can perform on this trace" and the server's response is correct and respects the TSP, it's the client's responsibility to decide whether it can visualize the data.

Yes, it totally makes sense. I could filter it out on the server as well, but other clients would miss out.

* Is the real problem that the data model returned by the server doesn't give the client enough information to determine whether or not it can visualize the data correctly? Ex. Needs to be visualized using a scatter, needs server-specific trace sync feature, etc. It would make sense to create issues for these eventual improvements now while we're thinking about it.

The information that it's a non-common-x-axis, a scatter chart etc. is part of the series model when performing the fetchXY() TSP call. But then it's to late to filter out in the available analysis view.

Which charts work?

Notes from recent discussion with @arfio about which charts aren't working well.

* **Filter out XYs that have chart styles that aren't yet implemented**
  
  * All latency vs. time (because they should be displayed as scatters)
  * Lttng callstack latency vs time (both the incubator and not incubator version work badly) (time sync doesn’t work, laggy (with only 3 functions selected), missing chart style (should be a scatter))

* **Flame chart isn’t working well** (but flame chart lttng ust works)

There is only the LTTng UST Flame chart and LTTNg UST Flame chart (Incubator) both work. The server recently updated to fix a problem we had. I don't see any other Flame Chart, or do I missing something.

* XY charts that have value even with only relative information (no scale):
  
  * Memory usage
  * CPU usage
  * Disk IO
  * Histogram

These ones should stay. Each series in it have a common-x-axis as I described earlier. Short-comings about y-axis info hopefully can be fixed shortly.

* **Memory Usage analysis is still acting up** (spinning infinitely) for some unknown reason. My guess is [Inform user when analysis fails #460](https://github.com/theia-ide/theia-trace-extension/issues/460) will fix this.

Yes, this patch will help to notify if the trace has no data. If it has data, it will work as well like the Histogram

Finally, in every case we need to remove the event matching latency vs time one, because it's always empty.

[ebugden]

ebugden: Is the real problem that the data model returned by the server doesn't give the client enough information to determine whether or not it can visualize the data correctly? Ex. Needs to be visualized using a scatter, needs server-specific trace sync feature, etc. It would make sense to create issues for these eventual improvements now while we're thinking about it.

bhufmann: The information that it's a non-common-x-axis, a scatter chart etc. is part of the series model when performing the fetchXY() TSP call. But then it's to late to filter out in the available analysis view.

The client should know this information while building the available analysis list, no? Otherwise it implies that all clients should support all possible chart styles and non-common x axis, etc.

---

ebugden: Flame chart isn’t working well (but flame chart lttng ust works)

bhufmann: There is only the LTTng UST Flame chart and LTTNg UST Flame chart (Incubator) both work. The server recently updated to fix a problem we had. I don't see any other Flame Chart, or do I missing something.

If changes were made recently, the Flame chart comment could be out of date. I can't remember why arfio mentioned it wasn't working well. This comment refers to the the view just listed as "Flame Chart":

---

• XY charts that have value even with only relative information (no scale):
  • Memory usage
  • CPU usage
  • Disk IO
  • Histogram

bhufmann: These ones should stay. Each series in it have a common-x-axis as I described earlier. Short-comings about y-axis info hopefully can be fixed shortly.

Yes, I'm ok with this compromise.

[bhufmann]

I just merged a patch in the server to not send the Event Matching Analysis - Latency vs Time data provider. See bug 576402 for explanation.

[bhufmann]

ebugden: Flame chart isn’t working well (but flame chart lttng ust works)

bhufmann: There is only the LTTng UST Flame chart and LTTNg UST Flame chart (Incubator) both work. The server recently updated to fix a problem we had. I don't see any other Flame Chart, or do I missing something.

If changes were made recently, the Flame chart comment could be out of date. I can't remember why arfio mentioned it wasn't working well. This comment refers to the the view just listed as "Flame Chart":

Both show the LTTng UST Flame Chart. However, "Flame Chart" is the one from Trace Compass mainline. Some updates have been made to make it work (i.e add styles in that data provider). "LTTng UST Flame Chart (Incubator)" is the one from the incubator. The incubator one has some additions that the mainline Flame Chart doesn't have.

[ebugden]

bhufmann: I just merged a patch in the server to not send the Event Matching Analysis - Latency vs Time data provider. See bug 576402 for explanation.

Would this remove the Event Matching Analysis in Trace Compass as well? I thought the reason we hadn't filtered data providers earlier was because it would remove the analysis from TC even though it worked fine there.

---

Thank you for taking the time to update this issue with related changes made in another repo. It is super useful and appreciated!

[ebugden]

ebugden: Flame chart isn’t working well (but flame chart lttng ust works)

bhufmann: There is only the LTTng UST Flame chart and LTTNg UST Flame chart (Incubator) both work. The server recently updated to fix a problem we had. I don't see any other Flame Chart, or do I missing something.

ebugden: If changes were made recently, the Flame chart comment could be out of date. I can't remember why arfio mentioned it wasn't working well. This comment refers to the the view just listed as "Flame Chart":

bhufmann: Both show the LTTng UST Flame Chart. However, "Flame Chart" is the one from Trace Compass mainline. Some updates have been made to make it work (i.e add styles in that data provider). "LTTng UST Flame Chart (Incubator)" is the one from the incubator. The incubator one has some additions that the mainline Flame Chart doesn't have.

@arfio Could you clarify what problems you were observing with the Flame Chart and check whether they are still issues?

[bhufmann]

bhufmann: I just merged a patch in the server to not send the Event Matching Analysis - Latency vs Time data provider. See bug 576402 for explanation.

Would this remove the Event Matching Analysis in Trace Compass as well? I thought the reason we hadn't filtered data providers earlier was because it would remove the analysis from TC even though it worked fine there.

Thank you for taking the time to update this issue with related changes made in another repo. It is super useful and appreciated!

No, it is still available in Trace Compass (Eclipse version) as before. It's filtered out in the Trace Server at the endpoint level.

[bhufmann]

#508 was merged and at the moment no scatter charts are visible in the trace viewer.

[bhufmann]

By the way, the fix for Bug 576647 will make sure that the latency scatter chart data providers won't return status FAILED while the analysis is still running. Once scatter charts are supported in the front-end again, the views will be populated properly and won't show the spinning wheel indefinitely.

[ebugden]

Shall we close this issue? Now that #508 is merged, the biggest related piece remaining is #421, but we've already decided that the remaining XY charts provide value by showing the shape of the data so I'm comfortable closing this issue. I think any new bugs that are noticed can become new issues.

[bhufmann]

I'm ok to close it.