-
Notifications
You must be signed in to change notification settings - Fork 5
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 our terms of service and privacy 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?
Changes from 1 commit
c77369b
b05f081
419dcb2
d1b2c7d
8fbbff0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,3 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Collections.ObjectModel; | ||
using System.ComponentModel; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Windows.ApplicationModel.DataTransfer; | ||
using Coder.Desktop.App.Services; | ||
using Coder.Desktop.App.Utils; | ||
using Coder.Desktop.CoderSdk; | ||
|
@@ -18,15 +10,24 @@ | |
using Microsoft.UI.Xaml; | ||
using Microsoft.UI.Xaml.Controls; | ||
using Microsoft.UI.Xaml.Controls.Primitives; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Collections.ObjectModel; | ||
using System.ComponentModel; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using System.Xml.Linq; | ||
using Windows.ApplicationModel.DataTransfer; | ||
|
||
namespace Coder.Desktop.App.ViewModels; | ||
|
||
public interface IAgentViewModelFactory | ||
{ | ||
public AgentViewModel Create(IAgentExpanderHost expanderHost, Uuid id, string fullyQualifiedDomainName, | ||
string hostnameSuffix, | ||
AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string? workspaceName); | ||
|
||
string hostnameSuffix, AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, | ||
string? workspaceName, bool? didP2p, string? preferredDerp, TimeSpan? latency, TimeSpan? preferredDerpLatency, DateTime? lastHandshake); | ||
public AgentViewModel CreateDummy(IAgentExpanderHost expanderHost, Uuid id, | ||
string hostnameSuffix, | ||
AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string workspaceName); | ||
|
@@ -40,8 +41,11 @@ public class AgentViewModelFactory( | |
{ | ||
public AgentViewModel Create(IAgentExpanderHost expanderHost, Uuid id, string fullyQualifiedDomainName, | ||
string hostnameSuffix, | ||
AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string? workspaceName) | ||
AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, | ||
string? workspaceName, bool? didP2p, string? preferredDerp, TimeSpan? latency, TimeSpan? preferredDerpLatency, | ||
DateTime? lastHandshake) | ||
{ | ||
System.Diagnostics.Debug.WriteLine($"Creating agent: {didP2p} {preferredDerp} {latency} {lastHandshake}"); | ||
return new AgentViewModel(childLogger, coderApiClientFactory, credentialManager, agentAppViewModelFactory, | ||
expanderHost, id) | ||
{ | ||
|
@@ -51,6 +55,11 @@ public AgentViewModel Create(IAgentExpanderHost expanderHost, Uuid id, string fu | |
ConnectionStatus = connectionStatus, | ||
DashboardBaseUrl = dashboardBaseUrl, | ||
WorkspaceName = workspaceName, | ||
DidP2p = didP2p, | ||
PreferredDerp = preferredDerp, | ||
Latency = latency, | ||
PreferredDerpLatency = preferredDerpLatency, | ||
LastHandshake = lastHandshake, | ||
}; | ||
} | ||
|
||
|
@@ -73,10 +82,10 @@ public AgentViewModel CreateDummy(IAgentExpanderHost expanderHost, Uuid id, | |
|
||
public enum AgentConnectionStatus | ||
{ | ||
Green, | ||
Yellow, | ||
Red, | ||
Gray, | ||
Healthy, | ||
Unhealthy, | ||
NoRecentHandshake, | ||
Offline, | ||
} | ||
|
||
public partial class AgentViewModel : ObservableObject, IModelUpdateable<AgentViewModel> | ||
|
@@ -182,6 +191,75 @@ public string FullyQualifiedDomainName | |
[NotifyPropertyChangedFor(nameof(ExpandAppsMessage))] | ||
public partial bool AppFetchErrored { get; set; } = false; | ||
|
||
[ObservableProperty] | ||
[NotifyPropertyChangedFor(nameof(ConnectionTooltip))] | ||
public partial bool? DidP2p { get; set; } = false; | ||
|
||
[ObservableProperty] | ||
[NotifyPropertyChangedFor(nameof(ConnectionTooltip))] | ||
public partial string? PreferredDerp { get; set; } = null; | ||
|
||
[ObservableProperty] | ||
[NotifyPropertyChangedFor(nameof(ConnectionTooltip))] | ||
public partial TimeSpan? Latency { get; set; } = null; | ||
|
||
[ObservableProperty] | ||
[NotifyPropertyChangedFor(nameof(ConnectionTooltip))] | ||
public partial TimeSpan? PreferredDerpLatency { get; set; } = null; | ||
|
||
[ObservableProperty] | ||
[NotifyPropertyChangedFor(nameof(ConnectionTooltip))] | ||
public partial DateTime? LastHandshake { get; set; } = null; | ||
|
||
public string ConnectionTooltip { get | ||
{ | ||
var description = new StringBuilder(); | ||
|
||
if (DidP2p != null && DidP2p.Value && Latency != null) | ||
{ | ||
description.Append($""" | ||
You're connected peer-to-peer. | ||
|
||
You ↔ {Latency.Value.Milliseconds} ms ↔ {WorkspaceName} | ||
""" | ||
); | ||
} | ||
else if (PreferredDerpLatency != null) | ||
{ | ||
description.Append($""" | ||
You're connected through a DERP relay. | ||
We'll switch over to peer-to-peer when available. | ||
ibetitsmike marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Total latency: {PreferredDerpLatency.Value.Milliseconds} ms | ||
""" | ||
ibetitsmike marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
|
||
if (PreferredDerp != null && Latency != null) | ||
{ | ||
ibetitsmike marked this conversation as resolved.
Show resolved
Hide resolved
|
||
description.Append($"\nYou ↔ {PreferredDerp}: {PreferredDerpLatency.Value.Milliseconds} ms"); | ||
|
||
var derpToWorkspaceEstimatedLatency = Latency - PreferredDerpLatency; | ||
|
||
// Guard against negative values if the two readings were taken at different times | ||
if (derpToWorkspaceEstimatedLatency > TimeSpan.Zero) | ||
{ | ||
description.Append($"\n{PreferredDerp} ms ↔ {WorkspaceName}: {derpToWorkspaceEstimatedLatency.Value.Milliseconds} ms"); | ||
} | ||
} | ||
} | ||
if (LastHandshake != null) | ||
description.Append($"\n\nLast handshake: {LastHandshake?.ToString() ?? "Unknown"}"); | ||
|
||
ibetitsmike marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var tooltip = description.ToString().TrimEnd('\n', ' '); | ||
|
||
if (string.IsNullOrEmpty(tooltip)) | ||
return "No connection information available."; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: var statusString: String {
switch status {
case .okay, .high_latency:
break
default:
return status.description
}
guard let lastPing else {
// Either:
// - Old coder deployment
// - We haven't received any pings yet
return status.description
}
let highLatencyWarning = status == .high_latency ? "(High latency)" : ""
var str: String
if 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 latency
if let preferredDerpLatency = lastPing.preferredDerpLatency {
str += "\nYou ↔ \(lastPing.preferredDerp): \(preferredDerpLatency.prettyPrintMs)"
let derpToWorkspaceEstLatency = 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
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe you handle it here:
I have now replicated this logic by printing the status. |
||
|
||
ibetitsmike marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return tooltip; | ||
} | ||
} | ||
|
||
|
||
// We only show 6 apps max, which fills the entire width of the tray | ||
// window. | ||
public IEnumerable<AgentAppViewModel> VisibleApps => Apps.Count > MaxAppsPerRow ? Apps.Take(MaxAppsPerRow) : Apps; | ||
|
@@ -192,7 +270,7 @@ public string? ExpandAppsMessage | |
{ | ||
get | ||
{ | ||
if (ConnectionStatus == AgentConnectionStatus.Gray) | ||
if (ConnectionStatus == AgentConnectionStatus.Offline) | ||
return "Your workspace is offline."; | ||
if (FetchingApps && Apps.Count == 0) | ||
// Don't show this message if we have any apps already. When | ||
|
@@ -285,6 +363,16 @@ public bool TryApplyChanges(AgentViewModel model) | |
DashboardBaseUrl = model.DashboardBaseUrl; | ||
if (WorkspaceName != model.WorkspaceName) | ||
WorkspaceName = model.WorkspaceName; | ||
if (DidP2p != model.DidP2p) | ||
DidP2p = model.DidP2p; | ||
if (PreferredDerp != model.PreferredDerp) | ||
PreferredDerp = model.PreferredDerp; | ||
if (Latency != model.Latency) | ||
Latency = model.Latency; | ||
if (PreferredDerpLatency != model.PreferredDerpLatency) | ||
PreferredDerpLatency = model.PreferredDerpLatency; | ||
if (LastHandshake != model.LastHandshake) | ||
LastHandshake = model.LastHandshake; | ||
|
||
// Apps are not set externally. | ||
|
||
|
@@ -307,7 +395,7 @@ public void SetExpanded(bool expanded) | |
|
||
partial void OnConnectionStatusChanged(AgentConnectionStatus oldValue, AgentConnectionStatus newValue) | ||
{ | ||
if (IsExpanded && newValue is not AgentConnectionStatus.Gray) FetchApps(); | ||
if (IsExpanded && newValue is not AgentConnectionStatus.Offline) FetchApps(); | ||
} | ||
|
||
private void FetchApps() | ||
|
@@ -316,7 +404,7 @@ private void FetchApps() | |
FetchingApps = true; | ||
|
||
// If the workspace is off, then there's no agent and there's no apps. | ||
if (ConnectionStatus == AgentConnectionStatus.Gray) | ||
if (ConnectionStatus == AgentConnectionStatus.Offline) | ||
{ | ||
FetchingApps = false; | ||
Apps.Clear(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
using System.Collections.ObjectModel; | ||
using System.ComponentModel; | ||
using System.Linq; | ||
using System.Security.Principal; | ||
using System.Threading.Tasks; | ||
using Coder.Desktop.App.Models; | ||
using Coder.Desktop.App.Services; | ||
|
@@ -29,6 +30,7 @@ | |
{ | ||
private const int MaxAgents = 5; | ||
private const string DefaultDashboardUrl = "https://coder.com"; | ||
private readonly TimeSpan HealthyPingThreshold = TimeSpan.FromMilliseconds(150); | ||
|
||
private readonly IServiceProvider _services; | ||
private readonly IRpcController _rpcController; | ||
|
@@ -222,12 +224,26 @@ | |
if (string.IsNullOrWhiteSpace(fqdn)) | ||
continue; | ||
|
||
|
||
var lastHandshakeAgo = DateTime.UtcNow.Subtract(agent.LastHandshake.ToDateTime()); | ||
Check warning on line 228 in App/ViewModels/TrayWindowViewModel.cs
|
||
var connectionStatus = lastHandshakeAgo < TimeSpan.FromMinutes(5) | ||
? AgentConnectionStatus.Green | ||
: AgentConnectionStatus.Yellow; | ||
|
||
// 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Yes, for reference types it will be null. |
||
var connectionStatus = AgentConnectionStatus.Healthy; | ||
if (lastHandshakeAgo > TimeSpan.FromMinutes(5)) | ||
{ | ||
connectionStatus = AgentConnectionStatus.NoRecentHandshake; | ||
} | ||
else if (!isLatencyAcceptable) | ||
{ | ||
connectionStatus = AgentConnectionStatus.Unhealthy; | ||
} | ||
|
||
|
||
workspacesWithAgents.Add(agent.WorkspaceId); | ||
var workspace = rpcModel.Workspaces.FirstOrDefault(w => w.Id == agent.WorkspaceId); | ||
System.Diagnostics.Debug.WriteLine($"Agent {uuid} LastHandshakeAgo: {lastHandshakeAgo} ConnectionStatus: {connectionStatus} FQDN: {fqdn} Last ping: {agent.LastPing} Last handshake: {agent.LastHandshake}"); | ||
|
||
agents.Add(_agentViewModelFactory.Create( | ||
this, | ||
|
@@ -236,7 +252,12 @@ | |
_hostnameSuffixGetter.GetCachedSuffix(), | ||
connectionStatus, | ||
credentialModel.CoderUrl, | ||
workspace?.Name)); | ||
workspace?.Name, | ||
agent.LastPing?.DidP2P, | ||
agent.LastPing?.PreferredDerp, | ||
agent.LastPing?.Latency?.ToTimeSpan(), | ||
agent.LastPing?.PreferredDerpLatency?.ToTimeSpan(), | ||
agent.LastHandshake?.ToDateTime())); | ||
} | ||
|
||
// For every stopped workspace that doesn't have any agents, add a | ||
|
@@ -253,7 +274,7 @@ | |
// conflict with any agent IDs. | ||
uuid, | ||
_hostnameSuffixGetter.GetCachedSuffix(), | ||
AgentConnectionStatus.Gray, | ||
AgentConnectionStatus.Offline, | ||
credentialModel.CoderUrl, | ||
workspace.Name)); | ||
} | ||
|
@@ -268,7 +289,7 @@ | |
|
||
if (Agents.Count < MaxAgents) ShowAllAgents = false; | ||
|
||
var firstOnlineAgent = agents.FirstOrDefault(a => a.ConnectionStatus != AgentConnectionStatus.Gray); | ||
var firstOnlineAgent = agents.FirstOrDefault(a => a.ConnectionStatus != AgentConnectionStatus.Offline); | ||
if (firstOnlineAgent is null) | ||
_hasExpandedAgent = false; | ||
if (!_hasExpandedAgent && firstOnlineAgent is not null) | ||
|
@@ -433,7 +454,7 @@ | |
case Workspace.Types.Status.Stopping: | ||
case Workspace.Types.Status.Stopped: | ||
return true; | ||
// TODO: should we include and show a different color than Gray for workspaces that are | ||
// TODO: should we include and show a different color than Offline for workspaces that are | ||
// failed, canceled or deleting? | ||
default: | ||
return false; | ||
|
Uh oh!
There was an error while loading. Please reload this page.