search

microsoft / microsoft-performance-toolkit-sdk

Software Development Kit for the Microsoft Performance ToolKit
MIT License
151 stars 53 forks source link

Engine BuildTable ITableResult should make it intuitive to access table data #202

Open ivberg opened 2 years ago

ivberg commented 2 years ago

Is your feature request an entirely new concept? Accessing table data is not new, but I didn't find it intuitive and had to ask the SDK experts for a solution.

Is your feature request related to an existing component? Yes Engine BuildTable ITableResult

Is your feature request related to a problem? Please describe. Our Unit Tests need to be able to find bugs & increase code coverage by testing not only RuntimeExecutionResults.QueryOutput but the tables used by the UI code. I have found a lot of bugs in the table code and projectors by only using WPA UI - that we should be able to find faster using Unit Tests on tables. WPA UI slows down dev loop and can throw a lot of unrelated exceptions wasting time in the debugger.

However, accessing table data was not intuitive (to me)

Describe the solution you'd like @mslukebo pointed at an example In Perfetto UT we also implemented some extensions to ITableResult based on recommendations

More refined versions of these should be baked into the SDK.

Also, side note - in using Kusto for query data, they had an intuitive solution I liked and that is generic. Just putting here as an example for inspiration. Kusto returns something that implements IDataReader. IDataReader exposes similar concepts to what the SDK is exposing in that it exposes columns and arrays of object data in a column with a row iterator. Things like FieldCount, Columns via GetName, GetFieldType, and ToEnumerableObjectArray via LINQ which made it easy to work with the data

Describe alternatives you've considered See extensions we have already implemented. But this should be easy and part of SDK

Additional context

ivberg commented 2 years ago

Just putting this "3rd request" here instead of making a new issue since it's tightly coupled with this issue.

Also related to the above issue and in the exts we should be able to access the data by default within the bounds of DataSourceInfo and set the VisibleDomain.

This would help with #201 problem in that by default accessing data (with that calculating relativestamp data incorrectly bug in place) should have returned 1 row instead of 100 (just like WPA UI). Having the table data filtering by default DataSourceInfo (or VisibleDomain) would also allow things like % CPU usage (PercentGenerator) to work without hacky workaround trying to disable that since VisiableDomain is either not set, or it's a PITA to set.