- Notifications
You must be signed in to change notification settings - Fork5
chore: added latency tooltips on workspaces#134
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
// For compatibility with older deployments, we assume that if the | ||
// last ping is null, the agent is healthy. | ||
var isLatencyAcceptable = agent.LastPing != null ? agent.LastPing.Latency.ToTimeSpan() < HealthyPingThreshold : true; |
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.
Did you check that the behavior of the C# protobuf library does this? They usually just use a zero value when it's not set to be consistent with other protobuf implementations
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.
Yes, for reference types it will be null.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
App/ViewModels/AgentViewModel.cs Outdated
var tooltip = description.ToString().TrimEnd('\n', ' '); | ||
if (string.IsNullOrEmpty(tooltip)) |
ethanndicksonJun 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Is this necessary? When would this happen?
I'm not handling it on mac:
varstatusString:String{switch status{case.okay,.high_latency:breakdefault:return status.description}guardlet lastPingelse{ // Either: // - Old coder deployment // - We haven't received any pings yetreturn status.description}lethighLatencyWarning= status==.high_latency?"(High latency)":""varstr:Stringif lastPing.didP2p{ str=""" You're connected peer-to-peer.\(highLatencyWarning) You ↔\(lastPing.latency.prettyPrintMs) ↔\(wsName)"""}else{ str=""" You're connected through a DERP relay.\(highLatencyWarning) We'll switch over to peer-to-peer when available. Total latency:\(lastPing.latency.prettyPrintMs)""" // We're not guranteed to have the preferred DERP latencyiflet preferredDerpLatency= lastPing.preferredDerpLatency{ str+="\nYou ↔\(lastPing.preferredDerp):\(preferredDerpLatency.prettyPrintMs)"letderpToWorkspaceEstLatency= lastPing.latency- preferredDerpLatency // We're not guaranteed the preferred derp latency is less than // the total, as they might have been recorded at slightly // different times, and we don't want to show a negative value.if derpToWorkspaceEstLatency>0{ str+="\n\(lastPing.preferredDerp) ↔\(wsName):\(derpToWorkspaceEstLatency.prettyPrintMs)"}}} str+="\n\nLast handshake:\(lastHandshake?.relativeTimeString??"Unknown")"return str}
ibetitsmikeJun 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
I believe you handle it here:
guard let lastPing else { // Either: // - Old coder deployment // - We haven't received any pings yet return status.description }
I have now replicated this logic by printing the status.
Fixes:#106