-
Notifications
You must be signed in to change notification settings - Fork 3
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: inbrowser.link support #14
base: main
Are you sure you want to change the base?
Conversation
508ca85
to
037ba69
Compare
I was hoping go-rod would allow us to listen for a service worker registration event, but I had to run some javascript in the page to get that sort of access. We should be able to wait until the service worker is registered.
I think measuring from the time the site is first hit to the time the site is fully rendered is ideal. That is what we want to measure for sure... and the closest comparison to all other gateways. The only way I can think of to make the measurement of inbrowser.link accurate is to accumulate web-vitals from the initial landing, the redirect, and the final reload. But that feels really hacky. I think we may need to make some changes to the service-worker-gateway in order to facilitate tiros' testing, but I'm not a big fan of that idea either. |
Just to clarify, that's what I'm doing in this PR in probe.go#L268. So it is possible 👍
We could measure the time until the redirect to |
Can we listen for the page's "context destroyed" state? Because that's likely when the reload happens.. It would be nice to listen more directly for that reload though. |
@SgtPooki I think I found an event that we can listen for. If you check out the latest commit here, this will be the output:
I'm listening for these events: go p.page.EachEvent(func(e *proto.NetworkRequestWillBeSent) {
if e.Request.URL == "https://specs-ipfs-tech.ipns.inbrowser.link/" {
fmt.Printf("Redirect detected: %s\n", e.Request.URL)
p.navigated <- struct{}{}
}
})() The second time I see a redirect to Unfortunately, I only had like half an hour to look into this. I'll continue throughout the week 👍 |
40c2dca
to
971ad41
Compare
// mustNavigateServiceWorker listens to navigation and request events to | ||
// validate the navigation process. Events are matched against an expected | ||
// pattern to confirm a successful operation. | ||
func (p *probe) mustNavigateServiceWorker() { |
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.
@SgtPooki check out this method. It's a bit convoluted but it works 👍
@SgtPooki I just updated this PR with a working version of the service worker measurement. I promoted the service worker configuration on the same level as HTTP and IPFS measurements. This made the most sense to me. Then there's a new method called
So what I'm tracking in this method is the navigation event to https://inbrowser.link/ and then the requestWillBeSent for the final https://specs-ipfs-tech.ipns.inbrowser.link/. The latter event is also the offset that I add to the final measurements because I presume that this is the timestamp that web-vitals will measure against. The time from the first navigation to this timestamp (which I called Please have a look if this makes sense and I could work on a deployment perhaps next week. |
This is because the SW/main thread on the first hit to It may be beneficial for us to take another look at how we're registering the SW on the subdomain here so that there's not so much bouncing, but that was how we decided to get things working initially. |
@SgtPooki
I also had a look, took your commits and debugged quite a bit. First of all, when access
http://inbrowser.link:443/ipns/filecoin.io
I get a400 Bad Request
in Safari and withcurl
while it works just fine with Chrome. However, I noticed that the headless chrome seems to also have trouble resolving thehttp
version of the site. I temporarily changed the URL construction code to always usehttps
. We will need to be smarter though becausehttps
won't work with the local kubo instance. Maybe we can just hard-codeinbrowser.link
and usehttps
in this case 🤷♂️Then the page gets loaded successfully and also the service workers get registered 👍 However, now I have trouble running the measurement javascript because I get:
Execution context was destroyed.
I suppose this is due to the redirection from the service worker. I might evaluate some JS before the redirection and some after? If I add a sleep in there it works. I would like to wait for an event though.
Lastly, what will we eventually measure? Is it the page load time after the service workers have redirected? I would have thought we want to measure the timing from accessing
http://inbrowser.link:443/ipns/filecoin.io
until we end uphttps://filecoin-io.ipns.inbrowser.link/
and have rendered the page. Because of the execution context interruption I'm not sure that this is possible.I'll stop here for now and let you chime in 👍