-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(ci): monitor typescript perf #31757
Changes from 4 commits
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,105 @@ | ||||||
/* eslint-env node */ | ||||||
|
||||||
// eslint-disable-next-line | ||||||
const fs = require('fs'); | ||||||
const Sentry = require('@sentry/node'); | ||||||
|
||||||
Sentry.init({ | ||||||
// jest project under Sentry organization (dev productivity team) | ||||||
dsn: 'https://[email protected]/4857230', | ||||||
tracesSampleRate: 1.0, | ||||||
environment: 'ci', | ||||||
}); | ||||||
|
||||||
if (!fs.existsSync('/tmp/typescript-monitor.log')) { | ||||||
// We failed to retrieve the log file, but we dont want to impact the pipeline so just exit. | ||||||
// The absence of metrics in sentry should warn us here | ||||||
process.exit(0); | ||||||
} | ||||||
|
||||||
const ALLOWED_KEYS = new Set([ | ||||||
'Files', | ||||||
'Lines', | ||||||
'Nodes', | ||||||
'Identifiers', | ||||||
'Symbols', | ||||||
'Types', | ||||||
'Instantiations', | ||||||
'Memory used', | ||||||
'I/O Read', | ||||||
'I/O Write', | ||||||
'Parse time', | ||||||
'Bind time', | ||||||
'Check time', | ||||||
'Emit time', | ||||||
'Total time', | ||||||
]); | ||||||
|
||||||
const tsLogFile = fs.readFileSync('/tmp/typescript-monitor.log', 'utf8'); | ||||||
|
||||||
function toKeyValue(input) { | ||||||
try { | ||||||
return input | ||||||
.split('\n') | ||||||
.filter(n => !!n) | ||||||
.reduce((acc, line) => { | ||||||
const [k, v] = line.split(':'); | ||||||
|
||||||
if (typeof k !== 'string' || typeof v !== 'string') { | ||||||
return acc; | ||||||
} | ||||||
|
||||||
const key = k.trim(); | ||||||
const value = v.trim(); | ||||||
|
||||||
if (ALLOWED_KEYS.has(key)) { | ||||||
let number = parseFloat(value); | ||||||
let unit = undefined; | ||||||
|
||||||
if (!isNaN(number)) { | ||||||
acc[key] = {value: number, unit}; | ||||||
} | ||||||
|
||||||
if (/([a-zA-Z]$)/.test(value)) { | ||||||
number = parseFloat(v.substring(0, v.length - 2)); | ||||||
unit = v.match(/([a-zA-Z]$)/)?.[0]; | ||||||
acc[key] = {value: number, unit}; | ||||||
} | ||||||
} | ||||||
|
||||||
return acc; | ||||||
}, {}); | ||||||
} catch (e) { | ||||||
return null; | ||||||
} | ||||||
} | ||||||
|
||||||
const parsedDiagnostics = toKeyValue(tsLogFile); | ||||||
|
||||||
if (parsedDiagnostics === null) { | ||||||
// We failed to parse here, but we dont want to impact the pipeline so just exit. | ||||||
// The absence of metrics in sentry should warn us here | ||||||
process.exit(0); | ||||||
} | ||||||
|
||||||
const bindTime = parsedDiagnostics['Bind time']; | ||||||
const checkTime = parsedDiagnostics['Check time']; | ||||||
const parseTime = parsedDiagnostics['Parse time']; | ||||||
const totalTime = parsedDiagnostics['Total time']; | ||||||
const memoryUsage = parsedDiagnostics['Memory used']; | ||||||
|
||||||
const transaction = Sentry.startTransaction({name: 'typescript.compilation'}); | ||||||
|
||||||
transaction.setMeasurements({ | ||||||
'typescript.time.check': {value: checkTime.value}, | ||||||
'typescript.time.bind': {value: bindTime.value}, | ||||||
'typescript.time.parse': {value: parseTime.value}, | ||||||
'typescript.time.total': {value: totalTime.value}, | ||||||
'typescript.memory.used': {value: memoryUsage.value}, | ||||||
}); | ||||||
|
||||||
transaction.finish(); | ||||||
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. This feel weird to me (even though I totally get it). It's a consequence of spans not being indexed, so even if the "right" solution is to have values like Setting a transaction duration might be nice though! (idk if you have to adjust with
Suggested change
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. @AbhiPrasad 100%, I would want each compiler step to be a span, but since we are hacking it right now, it'll have to do. I wonder if the tsc compiler exposes some hooks that we could plug into if we called it programatically. I'll investigate. I'll add the total duration :) 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. Tried adding the start/end timestamps, but couldn't get the transactions to show up in the dashboard. Sent output to @AbhiPrasad and going to merge this as is - I'm happy to add them back later :) |
||||||
|
||||||
(async () => { | ||||||
await Sentry.flush(5000); | ||||||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,6 +201,7 @@ const config: Config.InitialOptions = { | |
testEnvironmentOptions: { | ||
sentryConfig: { | ||
init: { | ||
// jest project under Sentry organization (dev productivity team) | ||
dsn: 'https://[email protected]/4857230', | ||
environment: !!CI ? 'ci' : 'local', | ||
tracesSampleRate: 1.0, | ||
|
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.
We have node-ts, any reason we couldn't have written this as a typescript script?
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.
@evanpurkhiser I did not know we had it. I just looked at the other script in the gh folder. I'll make a PR to update it