-
Notifications
You must be signed in to change notification settings - Fork 888
feat: Workspace Proxy picker show latency to each proxy #7486
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
Conversation
Use performance API timings. - Fix cors and timing headers - Accept custom headers
I tested it locally and it works 👏 One minor thing tho, when I hover over the table row it looks like I can click on it, is that intentional? If yes, we maybe want to improve the UI feedback a bit because I clicked a few times and didn't receive any "action feedback" so I was not sure if it should be supposed to do something or not. Screen.Recording.2023-05-11.at.11.06.18.mov |
@BrunoQuaresma would love to have help redesigning the UI haha. Yes you can click it to select a proxy. Peek.2023-05-11.09-16.webm |
Ah, I see. I'm more than happy to help with that, could you please file a ticket for the "improve UI" part and assign it to me please? |
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.
FE looks good
Future work is to auto select based on this latency. That will be a follow up PR
What this does
This adds latency checks to workspace proxies in the dashboard. Fixes: #7381
(Obviously this looks terrible, but it works!)
Cloudflare uses the Performance API in their metrics.
https://blog.cloudflare.com/browser-performance-api/
New
latency-check
routeThis new route does not use any of our middleware. This is intentional to keep the request overhead as minimal as possible. This route is both on the proxy and coderd.
This route also adds a
Timing-Allow-Origin
header which is required to make this performance api in the browser work.https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/Resource_timing
Cors to Workspace Proxy
Include the CORs middleware to new
latency-check
route on the workspace proxy.Add custom headers that we attach to all requests.
Custom
X-LATENCY-CHECK
headerA
GET
request is asimple web request
which does not trigger CORs preflight on some browsers. Adding a custom header forces it to do a preflight.useProxyLatency.ts
hook to calculate proxy latenciesUses the performance API to measure latency requests. We measure from
requestStart
toresponseStart
. Which gives us as close toping
type measuring as we can get.These values are only allowed if
Timing-Allow-Origin
is set correctly. If the performance API cannot access these values, it falls back toduration
which is the total time of the request. I use aconsole.log
to log this case. We might want to display something in the ui about this.My development process if anyone was curious.
Approach 1: Axios timing
My first idea was to time the axios requests to measure latency. This turned out to be flawed. Not only are we measure the overhead of things like TLS, but the axios timing is just imprecise. Even in the interceptors. The values were off my 10ms to the network panel for the total request time.
Approach 2: Performance API
The better approach is to use: https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/Resource_timing
Issue 1: Cors + CSP
Setup CORs and CSP to allow external requests to the proxy.
Fixed #7484
Issue 2: The performance api is missing the timing values
requestStart
andresponseStart
The requestStart value is not coming through.
Currently trying to use
Timing-Allow-Origin
header, but not getting any results.https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/requestStart#cross-origin_timing_information
Notes:
On chrome I was running with
--disable-web-security
to ignore some CORs issues, but that was removingrequestStart
even from local requests which I was using to test the logic. So it looks like that was causing therequestStart
to always be0
even if the Timing header was present.