`Onxy.printMetrics` MD, CSV and JSON formats #90

kidroca · 2021-07-16T18:30:39Z

@marcaaron

Details

This is a continuation of #89
This PR adds more print options:

print data as Markdown tables
as csv strings (easy import to Excel)
as json

It necessary to have something like this as it aids capturing and saving benchmark information

This PR does not change how Onyx works and only affects dev, debug, benchmark usages

Some examples

MD (Console)

CSV

When the data is too much it's not printed in full, but can be expanded and/or copied

JSON

MD (String)

This is useful when __DEV__ is false as calling console.info would not output anything

Related Issues

N/A

Automated Tests

Covered by existing tests,
Manually tested the printing functionality

Linked PRs

…available Since the update for Reanimated 2 (TurboModules/JSI) It's no longer possible to use the old (Chrome) debugging flow `react-native` runtime does not implement the `console.table` we're using to print information It was provided by Chrome, by launching the (old) debug flow `AsciiTable` allows us to print a MarkDown compatible table which is handy for posting data to github and other places that support MD

kidroca · 2021-07-16T18:40:08Z

Oops some leftovers that should be cleaned up, will push another commit

kidroca · 2021-07-16T19:09:51Z

lib/MDTable.js

+        // Ignore modifying the first |---| for titled tables
+        let idx = this.getTitle() ? -2 : -1;
+        const ascii = super.toString()
+            .replace(/-\|/g, () => {
+                /* we replace "----|" with "---:|" to align the data to the right in MD */
+                idx++;
+
+                if (idx < 0 || this.leftAlignedCols.includes(idx)) {
+                    return '-|';
+                }
+
+                return ':|';
+            });


I guess this is the most weird part of the PR

Markdown tables are left aligned by default
To align a column you add a : column on the side you want it to align (or on both sides for centered)

|----------|---------:|----------:|

When we're printing a table with a title we want to skip the first row which looks like:

| Onyx:set | |--------------------------------|

That's why we start with -2 on that case

maybe could use a different token than Onyx:set like Onyx~set but this seems fine.

Individual tables would be labeled with whatever alias is provided here:
https://github.com/Expensify/react-native-onyx/blob/master/lib/Onyx.js#L771-L777

lib/MDTable.js

kidroca · 2021-07-16T19:13:37Z

lib/decorateWithMetrics.js

 */
-function printMetrics(raw = false) {
-    const {totalTime, summaries, lastCompleteCall = {endTime: -1}} = getMetrics();


Changed this to print Last call finished at: N/A when there are no calls made yet (or were cleared)

kidroca · 2021-07-16T19:18:29Z

lib/decorateWithMetrics.js

+                    toDuration(call.startTime, raw),
+                    toDuration(call.endTime, raw),
+                    toDuration(call.duration, raw),
+                    call.args.map(String).join(', ').slice(0, 60), // Restrict cell width to 60 chars max


We're limiting this as it breaks the table when it's too large
I find it better to not JSON.stringify the data and instead keep it simple like: account, [object Object]
otherwise it adds too much noise

kidroca · 2021-07-16T19:20:37Z

lib/decorateWithMetrics.js

-    console.table(prettyData.map(({prettyCalls, ...summary}) => summary));
+    const mainOutput = [
+        '### Onyx Benchmark',
+        `  - Total: ${toDuration(totalTime, raw)}`,
+        `  - Last call finished at: ${lastCompleteCall ? toDuration(lastCompleteCall.endTime, raw) : 'N/A'}`,
+        '',
+        tableSummary.toString()
+    ];

-    prettyData.forEach((method) => {
-        console.groupCollapsed(`[${method.methodName}] individual calls: `);
-        console.table(method.prettyCalls);
+    /* eslint-disable no-console */
+    console.info(mainOutput.join('\n'));
+    methodCallTables.forEach((table) => {
+        console.groupCollapsed(table.getTitle());
+        console.info(table.toString());
        console.groupEnd();
    });
-
-    console.groupEnd();


I wonder whether we need console printing at all
The only convenience here is console.groupCollapsed

kidroca · 2021-07-16T19:27:02Z

package.json

@@ -12,6 +12,7 @@
    "test": "jest"
  },
  "dependencies": {
+    "ascii-table": "0.0.9",


I was expecting to find something that will build directly as MD, but this was the closest thing I've found
This library required the least modifications to achieve an MD table
I've tried these alternatives:

https://www.npmjs.com/package/tty-table

https://www.npmjs.com/package/console-table-printer

https://www.npmjs.com/package/as-table

This allows us to capture start/end time between different interactions e.g. resetMetrics then switch to a new chat When you print the stats you'll see the end time compared relative to the point before making the switch

This helps identify the tables when they are exported to excel

johnmlee101 · 2021-07-20T19:30:35Z

Is this ready for review?

kidroca · 2021-07-20T19:43:18Z

@johnmlee101 Yes

johnmlee101 · 2021-07-21T19:30:34Z

lib/decorateWithMetrics.js

+                default:
+                    return table.toString();
+            }
+        }).join('\n\n');


Any specific reason we join with 2 new lines?

To distinct separate tables easier

johnmlee101

Looks good to me AFAICT, I asked @marcaaron to do final review

marcaaron · 2021-07-21T19:37:39Z

lib/MDTable.js

+        // Ignore modifying the first |---| for titled tables
+        let idx = this.getTitle() ? -2 : -1;
+        const ascii = super.toString()
+            .replace(/-\|/g, () => {
+                /* we replace "----|" with "---:|" to align the data to the right in MD */
+                idx++;
+
+                if (idx < 0 || this.leftAlignedCols.includes(idx)) {
+                    return '-|';
+                }
+
+                return ':|';
+            });


maybe could use a different token than Onyx:set like Onyx~set but this seems fine.

kidroca added 5 commits July 16, 2021 16:33

MD formatting with alignment

596c37e

Add json|string|csv output options to printMetrics

26ec2d8

Fix alignment when the table has a title

1751a0a

Reduce args cell size to 60 chars

9602d21

kidroca requested a review from a team as a code owner July 16, 2021 18:30

MelvinBot requested review from johnmlee101 and removed request for a team July 16, 2021 18:30

Print Last call finished at: N/A when there are no calls yet

6f5bfd3

kidroca marked this pull request as draft July 16, 2021 18:40

Cleanup and documentation

9ad9555

kidroca commented Jul 16, 2021

View reviewed changes

kidroca marked this pull request as ready for review July 16, 2021 19:27

kidroca added 2 commits July 17, 2021 02:01

Reset performance offset when we call Onyx.resetMetrics()

0b0be97

This allows us to capture start/end time between different interactions e.g. resetMetrics then switch to a new chat When you print the stats you'll see the end time compared relative to the point before making the switch

Add table name above CSV records

5111c1b

This helps identify the tables when they are exported to excel

kidroca force-pushed the kidroca/md-print-format branch from edfc47e to 5111c1b Compare July 19, 2021 10:19

johnmlee101 reviewed Jul 21, 2021

View reviewed changes

johnmlee101 approved these changes Jul 21, 2021

View reviewed changes

marcaaron approved these changes Jul 21, 2021

View reviewed changes

marcaaron merged commit 0b0e089 into Expensify:master Jul 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Onxy.printMetrics` MD, CSV and JSON formats #90

`Onxy.printMetrics` MD, CSV and JSON formats #90

kidroca commented Jul 16, 2021 •

edited

Loading

kidroca commented Jul 16, 2021

kidroca Jul 16, 2021

marcaaron Jul 21, 2021

kidroca Jul 21, 2021 •

edited

Loading

kidroca Jul 16, 2021

kidroca Jul 16, 2021

kidroca Jul 16, 2021

kidroca Jul 16, 2021 •

edited

Loading

johnmlee101 commented Jul 20, 2021

kidroca commented Jul 20, 2021

johnmlee101 Jul 21, 2021

kidroca Jul 21, 2021

johnmlee101 left a comment

marcaaron Jul 21, 2021

Onxy.printMetrics MD, CSV and JSON formats #90

Onxy.printMetrics MD, CSV and JSON formats #90

Conversation

kidroca commented Jul 16, 2021 • edited Loading

Details

Some examples

MD (Console)

CSV

JSON

MD (String)

Related Issues

Automated Tests

Linked PRs

kidroca commented Jul 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidroca Jul 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidroca Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

johnmlee101 commented Jul 20, 2021

kidroca commented Jul 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnmlee101 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

`Onxy.printMetrics` MD, CSV and JSON formats #90

`Onxy.printMetrics` MD, CSV and JSON formats #90

kidroca commented Jul 16, 2021 •

edited

Loading

kidroca Jul 21, 2021 •

edited

Loading

kidroca Jul 16, 2021 •

edited

Loading