-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
chore(gatsby-plugin-benchmark-reporting): Add more meta data #21369
Conversation
(Requests for feedback were meant as a ping, but feedback is welcome. This cannot merge until db is updated to support the new fields.) |
Hm, should it also report the size of images? As a rough guide for triage of why image processing took so long or whatever? |
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.
Still need CI_NAME
1706b55
to
976f899
Compare
@nicholascapo after some discussion we're pulling the siteId explicitly from the analytics object for now. If in the future more things get added to this env object it we can update accordingly to various implementations. Mainly wanted to prevent the footgun where the analytics object gets a new field while the benchmarking schema isn't expecting it and the server just refuses it. |
The |
This adds a few new fields to the benchmark plugin: git repo hash, ci status, siteId, cwd, ci name, node version, gatsby version, gatsby cli version, sharp version (if present), webpack version, node memory (straight dump of
process.memoryUsage()
), size of alljs
files in root folder ofpublic
, and the number of images found recursively inpublic
or.cache
.It also groups the versions and counts into a struct.
The plugin now reports the text of a 500 error, if any.
Need to update big query schema before pulling this.@jamo has updated the schema accordingly.This change is not windows friendly due to the execSyncs using
find
,wc
,bc
, and forward slashes. There are probably one or two more subtleties that I can't test for. For the time being this plugin is intended to run with benchmarking service and some CI's, which all run on linux. If we need windows support, we can iterate (most-if-not-all of this stuff can be done with just JS, just more work).Additionally the execs are sync because that just makes it easier. There are only a handful, and there is no performance concern because this runs at the end of a run. Changing it to async would not make a relevant difference but more annoying code to maintain.