-
Notifications
You must be signed in to change notification settings - Fork 78
Onxy.printMetrics
MD, CSV and JSON formats
#90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
b120428
596c37e
26ec2d8
1751a0a
9602d21
6f5bfd3
9ad9555
0b0be97
5111c1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import AsciTable from 'ascii-table'; | ||
|
||
class MDTable extends AsciTable { | ||
/** | ||
* Create a CSV string from the table data | ||
* @returns {string} | ||
*/ | ||
toCSV() { | ||
return [this.getHeading(), ...this.getRows()].join('\n'); | ||
} | ||
|
||
/** | ||
* Create a JSON string from the table data | ||
* @returns {string} | ||
*/ | ||
toJSON() { | ||
return JSON.stringify(super.toJSON()); | ||
} | ||
|
||
/** | ||
* Create a MD string from the table data | ||
* @returns {string} | ||
*/ | ||
toString() { | ||
// 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 ':|'; | ||
}); | ||
|
||
// strip the top and the bottom row (----) to make an MD table | ||
const md = ascii.split('\n').slice(1, -1).join('\n'); | ||
return md; | ||
kidroca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
/** | ||
* Table Factory helper | ||
* @param {Object} options | ||
* @param {string} [options.title] - optional title center above the table | ||
* @param {string[]} options.heading - table column names | ||
* @param {number[]} [options.leftAlignedCols=[]] - indexes of columns that should be left aligned | ||
* Pass the columns that are non numeric here - the rest will be aligned to the right | ||
* @param {Array} [options.rows] The table can be initialized with row. Rows can also be added by `addRow` | ||
* @returns {MDTable} | ||
*/ | ||
MDTable.factory = ({ | ||
title, heading, leftAlignedCols = [], rows = [] | ||
}) => { | ||
const table = new MDTable({title, heading, rows}); | ||
table.leftAlignedCols = leftAlignedCols; | ||
|
||
/* By default we want everything aligned to the right as most values are numbers | ||
* we just override the columns that are not right aligned */ | ||
heading.forEach((name, idx) => table.setAlign(idx, AsciTable.RIGHT)); | ||
leftAlignedCols.forEach(idx => table.setAlign(idx, AsciTable.LEFT)); | ||
|
||
return table; | ||
}; | ||
|
||
export default MDTable; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import _ from 'underscore'; | ||
import MDTable from './MDTable'; | ||
|
||
/** | ||
* Each key is a method name and the value is an array of calls metadata | ||
|
@@ -136,55 +137,83 @@ function toDuration(millis, raw = false) { | |
* max, min, average, total time for each method | ||
* and a table of individual calls | ||
* | ||
* @param {boolean} [raw=false] setting this to true will print raw instead of human friendly times | ||
* @param {Object} [options] | ||
* @param {boolean} [options.raw=false] - setting this to true will print raw instead of human friendly times | ||
* Useful when you copy the printed table to excel and let excel do the number formatting | ||
* @param {'console'|'csv'|'json'|'string'} [options.format=console] The output format of this function | ||
* `string` is useful when __DEV__ is set to `false` as writing to the console is disabled, but the result of this | ||
* method would still get printed as output | ||
* @returns {string|undefined} | ||
*/ | ||
function printMetrics(raw = false) { | ||
const {totalTime, summaries, lastCompleteCall = {endTime: -1}} = getMetrics(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed this to print |
||
function printMetrics({raw = false, format = 'console'} = {}) { | ||
const {totalTime, summaries, lastCompleteCall} = getMetrics(); | ||
|
||
const prettyData = _.chain(summaries) | ||
const tableSummary = MDTable.factory({ | ||
heading: ['method', 'total time spent', 'max', 'min', 'avg', 'time last call completed', 'calls made'], | ||
leftAlignedCols: [0], | ||
}); | ||
|
||
const methodCallTables = _.chain(summaries) | ||
.filter(method => method.avg > 0) | ||
.sortBy('avg') | ||
.reverse() | ||
.map(({ | ||
calls, methodName, lastCall, ...summary | ||
}) => { | ||
const prettyTimes = _.chain(summary) | ||
.map((value, key) => ([key, toDuration(value, raw)])) | ||
.object() | ||
.value(); | ||
|
||
const prettyCalls = calls.map(call => ({ | ||
startTime: toDuration(call.startTime, raw), | ||
endTime: toDuration(call.endTime, raw), | ||
duration: toDuration(call.duration, raw), | ||
args: JSON.stringify(call.args) | ||
})); | ||
|
||
return { | ||
.map(({methodName, calls, ...methodStats}) => { | ||
tableSummary.addRow( | ||
methodName, | ||
...prettyTimes, | ||
'time last call completed': toDuration(lastCall.endTime, raw), | ||
calls: calls.length, | ||
prettyCalls, | ||
}; | ||
toDuration(methodStats.total, raw), | ||
toDuration(methodStats.max, raw), | ||
toDuration(methodStats.min, raw), | ||
toDuration(methodStats.avg, raw), | ||
toDuration(methodStats.lastCall.endTime, raw), | ||
calls.length, | ||
); | ||
|
||
const callsTable = MDTable.factory({ | ||
title: methodName, | ||
heading: ['start time', 'end time', 'duration', 'args'], | ||
leftAlignedCols: [3], | ||
rows: calls.map(call => ([ | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're limiting this as it breaks the table when it's too large |
||
])) | ||
}); | ||
|
||
return callsTable; | ||
}) | ||
.value(); | ||
|
||
/* eslint-disable no-console */ | ||
console.group('Onyx Benchmark'); | ||
console.info(' Total: ', toDuration(totalTime, raw)); | ||
console.info(' Last call finished at: ', toDuration(lastCompleteCall.endTime, raw)); | ||
if (/csv|json|string/i.test(format)) { | ||
const allTables = [tableSummary, ...methodCallTables]; | ||
|
||
return allTables.map((table) => { | ||
switch (format.toLowerCase()) { | ||
case 'csv': | ||
return table.toCSV(); | ||
case 'json': | ||
return table.toJSON(); | ||
default: | ||
return table.toString(); | ||
} | ||
}).join('\n\n'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any specific reason we join with 2 new lines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To distinct separate tables easier |
||
} | ||
|
||
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(); | ||
Comment on lines
-179
to
-187
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder whether we need console printing at all |
||
/* eslint-enable */ | ||
} | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
"test": "jest" | ||
}, | ||
"dependencies": { | ||
"ascii-table": "0.0.9", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was expecting to find something that will build directly as MD, but this was the closest thing I've found |
||
"expensify-common": "git+https://github.com/Expensify/expensify-common.git#2e5cff552cf132da90a3fb9756e6b4fb6ae7b40c", | ||
"lodash": "4.17.21", | ||
"underscore": "^1.13.1" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
That's why we start with
-2
on that caseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe could use a different token than
Onyx:set
likeOnyx~set
but this seems fine.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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