-
Notifications
You must be signed in to change notification settings - Fork 0
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
Port lighthouse-docker to TS in New Repo #1
Conversation
This class will be used to run launch Chrome for Lighthouse. It is a very simple class that wraps around the "chrome-launcher" package.
This class will be used to execute the a lighthouse runner using the port of the launched chrome instance. This is mainly a wrapper around the lighthouse node module.
This class will be a wrapper around the datadog-api-client package. For the moment, it will only have a method to send metrics to Datadog. That said, we will require some api keys to be able to send metrics, so let's create an .env file and import it via dotenv.
The logic in the index file is similar to the logic in the main.py file in the lighthouse-docker repo, https://github.com/iFixit/lighthouse-docker/blob/master/lib/main.py. The only difference is we will use the timestamp of the lighthouse report to timestamp the metrics data sent to Datadog.
In lighthouse-docker we run both form factors, desktop and mobile. To do this we need to import the associated config files for each form factor. We cannot just set the "formFactor" lh option to "desktop" or "mobile" as there are other options that need to be set. For example, the "throttling" option. These additional options were automatically set when we ran lighthouse through the CLI tool in lighthouse-docker. However, when we run lighthouse programmatically, we need to set these options ourselves, or use the config files directly.
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.
CR 📱 with some comments. deploy_block 📱
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.
CR 📱
Checks out to me!
un_deploy_block ⚡ |
CR 🚗 carry-over just fixed a miss-wording in the README |
dev_block 🌵 @ardelato, it sounds like you and @jordycosta have been troubleshooting this already today. I'm getting the same connection error as he's getting. |
To confirm, @erinemay are you also testing this locally on a Windows machine? |
Yes. Locally on Windows 10 in my Ubuntu terminal via WSL |
I debugged the problem and it has to do with the GoogleChrome/lighthouse#12131 I found a workaround by using |
We need to use puppeteer instead of chrome-launcher because at the moment some of the current Devs are having issues with chrome-launcher when running in WSL. We don't have a docker image for vigilo yet, so we need to use puppeteer for now. We will not be passing a "Page" object to the lighthouse runner instead of a "port" option. Aside from that, everything else should be the same.
I accidentally left this in here when I was debugging. Let's make sure the browser runs in a headless mode
un_dev_block ⚡ I think the issue has been fixed. I also added some additional setup steps in the |
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.
CR 📱 with a couple of questions/comments. deploy_block 📱
Co-authored-by: Michael Lahargou <[email protected]>
un_deploy_block ⚡ I think I addressed all the comments |
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.
CR 📱
QA 👍 On e3f1df1, I was able to successfully run the script. deploy_block 🌵 where do these go in Datadog? |
un_deploy_block ⚡ It looks like it worked Datadog Link The data can be seen in |
Description
The metrics from
lighthouse-docker
are being significantly affected by theubreakit
load. https://github.com/iFixit/ifixit/issues/49196.To ensure we have stable metrics, we need to run
lighthouse-docker
on a dedicated machine. However, the changes that will be needed for this will deviate from the original purpose of thelighthouse-docker
repo. Therefore, it was discussed that a new repo would be created specifically for this use case. In this new repo, we will be using Typescript as opposed to Python because interacting withlighthouse
will be easier if we use thenpm
package as opposed to the CLI tool.CR
The majority of the changes were simply setting up a Typescript environment and integrating the lighthouse package into the core logic. The logic itself is mainly a Typescript port of the Python logic seen in https://github.com/iFixit/lighthouse-docker/blob/master/lib/main.py.
QA
Very similar to prior
lighthouse-docker
pulls with a few changes to the setup steps (iFixit/lighthouse-docker#23)README.md
pnpm run start
Closes: https://github.com/iFixit/ifixit/issues/49454