Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 23 commits intomainfromstevenmasley/proxy_picker
May 11, 2023

Conversation

Emyrk
Copy link
Member

@EmyrkEmyrk commentedMay 10, 2023
edited
Loading

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/

Newlatency-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 aTiming-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 newlatency-check route on the workspace proxy.
Add custom headers that we attach to all requests.

CustomX-LATENCY-CHECK header

AGET 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 latencies

Uses the performance API to measure latency requests. We measure fromrequestStart toresponseStart. Which gives us as close toping type measuring as we can get.

timestamp-diagram

These values are only allowed ifTiming-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 valuesrequestStart andresponseStart

The requestStart value is not coming through.

Currently trying to useTiming-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.

@EmyrkEmyrk marked this pull request as ready for reviewMay 10, 2023 23:33
@EmyrkEmyrk changed the titleWIP: Proxy picker show latencyfeat: Workspace Proxy picker show latency to each proxyMay 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
MemberAuthor

@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?

Emyrk reacted with thumbs up emoji

Copy link
Collaborator

@BrunoQuaresmaBrunoQuaresma 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

@EmyrkEmyrk merged commit8f768f8 intomainMay 11, 2023
@EmyrkEmyrk deleted the stevenmasley/proxy_picker branchMay 11, 2023 20:42
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsMay 11, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@BrunoQuaresmaBrunoQuaresmaBrunoQuaresma approved these changes

@Kira-PilotKira-PilotAwaiting requested review from Kira-Pilot

Assignees

@EmyrkEmyrk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

workspace proxies: show latency indicator on dashboard
2 participants
@Emyrk@BrunoQuaresma

[8]ページ先頭

©2009-2025 Movatter.jp