Skip to content

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

Merged
merged 23 commits into from
May 11, 2023

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented May 10, 2023

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!)

Screenshot from 2023-05-11 09-45-52

Cloudflare uses the Performance API in their metrics.

https://blog.cloudflare.com/browser-performance-api/

New latency-check route

This 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 header

A GET request is a simple 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 latencies

Uses the performance API to measure latency requests. We measure from requestStart to responseStart. Which gives us as close to ping type measuring as we can get.

timestamp-diagram

These values are only allowed if Timing-Allow-Origin is set correctly. If the performance API cannot access these values, it falls back to duration which is the total time of the request. I use a console.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 and responseStart

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 removing requestStart even from local requests which I was using to test the logic. So it looks like that was causing the requestStart to always be 0 even if the Timing header was present.

Emyrk added 4 commits May 10, 2023 14:40
Use performance API timings.
- Fix cors and timing headers
- Accept custom headers
@Emyrk Emyrk marked this pull request as ready for review May 10, 2023 23:33
@Emyrk Emyrk changed the title WIP: Proxy picker show latency feat: Workspace Proxy picker show latency to each proxy May 10, 2023
@BrunoQuaresma
Copy link
Collaborator

The stories are showing a ? in the "latency" column. I think it is because we are not passing the latency value in the mock. Also, could be better to display a híphen instead of a question mark or a helpful message if that is possible.

Screen Shot 2023-05-11 at 10 55 48

@BrunoQuaresma
Copy link
Collaborator

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

@Emyrk
Copy link
Member Author

Emyrk commented May 11, 2023

@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

@BrunoQuaresma
Copy link
Collaborator

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?

Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FE looks good

@Emyrk Emyrk merged commit 8f768f8 into main May 11, 2023
@Emyrk Emyrk deleted the stevenmasley/proxy_picker branch May 11, 2023 20:42
@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

workspace proxies: show latency indicator on dashboard
2 participants