Add ADR about adding support for Flame Graph in TSP and trace viewer#1203
Add ADR about adding support for Flame Graph in TSP and trace viewer#1203bhufmann merged 1 commit intoeclipse-cdt-cloud:masterfrom
Conversation
bhufmann
force-pushed
the
flame-graph-adr
branch
2 times, most recently
from
64fef64 to
4597347
Compare
aaanh
left a comment
aaanh
left a comment
There was a problem hiding this comment.
I've read through the information. Thank you Bernd!
doc/adr/0013-flame-graph-support.md
Outdated
|
|
||
| **Note**: | ||
|
|
||
| - This solution will allow us to fetch data depending on the zoom level. Since the flame graph data is limited in size (in comparison to regular time graphs), all the data can fetched once and all the zooming is done in FE only. |
There was a problem hiding this comment.
I'm confused, are both sentences contradicting each other?
There was a problem hiding this comment.
Yes, it's not clear. What I wanted to say that it's possible to get the whole data once and the FE handles the zooming.
doc/adr/0013-flame-graph-support.md
Outdated
| - The annotation endpoints and styles endpoint are independent of the chart type (`TIME_GRAPH`) and can be provided for any chart type. | ||
| - The endpoints and data structures for the time based gantt chart (providerType `TIME_GRAPH`) are designed around timestamps. For example, states have a start and end, or annotations have a `start` and `duration`, or query parameter `requested_time_range` is meant for time stamps. Those names should be more generic, i.e. instead of `duration` it should be `delta`, instead of `requested_time_range` it should be `requested_range` and so on. Having generic names it won't be necessary to duplicate endpoints, data structes etc. for different x-axis. | ||
| - To be able to get a flame graph for a given time range of the trace the `requested_time_range` should be used for that and a new query parameter `requested_range` for the view range (see bullet above). | ||
| - Arrows for flame chart won't make make sense and don't need to be supported. |
There was a problem hiding this comment.
Maybe not for flame chart but perhaps for other uses of this new provider type?
There was a problem hiding this comment.
Yes, flame graph. I'll update it.
doc/adr/0013-flame-graph-support.md
Outdated
|
|
||
| ## Decision | ||
|
|
||
| Flame Graph support will be added. Re-using the flame graph data provider (solution 1) will allow client FE to leverage existing gantt chart view implementation. It also provides known API. |
There was a problem hiding this comment.
Is it just a time graph that isn't synchronized with the global view range? Maybe we could keep using the same timegraph endpoint but add a new capability that indicates whether the data provider is dependent on global view range? Unfortunately though, everywhere in the chain 'time' is used as implied x-unit.
There was a problem hiding this comment.
The endpoint will support the requested_time range query parameter which will allow to get the flame graph for a given time range if it is omitted then the flame graph whole trace will be shown.
|
I squashed the fix-up commit by @PatrickTasse into the main commit and added the decision. The PR is also rebased to the latest. Could you please review it again? Thanks! |
doc/adr/0013-flame-graph-support.md
Outdated
| The view itself is very similar to data providers of type `TIME_GRAPH`. The main difference is that the x-axis is the duration and not time. The Trace Compass back-end currently has such a data provider available with the same API than `TIME_GRAPH` data providers. However, this data provider is not exposed to the server through the `getDescriptor()` method of the corresponing data provider factory. It will need a new type to be added to the `ProviderType` enum. | ||
|
|
||
| With this solution similar endpoints can be defined which have the advantage that the endpoints and data structures are known. Back-end filtering and highlighting would be supported out of the box. Assuming the new `Provider Type` is called `gantt` the following endpoints will be defined: | ||
| With this solution the existing endpoint `TIME_GRAPH` endpoint and be agumented or similar endpoints can be defined which have the advantage that the endpoints and data structures are known. Back-end filtering and highlighting would be supported out of the box. |
There was a problem hiding this comment.
can be augmented?
do we want to indicate that the additions would include a parameter to indicate that the timegraph should be independent and not be synchronized with the time range of other views?
There was a problem hiding this comment.
Not sure if we need to say that here. In the end it's the FE decision to synchronize with the current zoom range or selection range or not at all. I understand that it might be beneficial that the BE provides this information for each data provider, though.
Signed-off-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
3 participants
What it does
Add ADR about adding support for Flame Graph in TSP and trace viewer.
How to test
Read and verify ADR markdown file.
Follow-ups
Implement selected solution
Review checklist