Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: added latency tooltips on workspaces #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 107 additions & 19 deletions App/ViewModels/AgentViewModel.cs
View file
Open in desktop
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;
Expand All @@ -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);
Expand All @@ -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)
{
Expand All @@ -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,
};
}

Expand All @@ -73,10 +82,10 @@ public AgentViewModel CreateDummy(IAgentExpanderHost expanderHost, Uuid id,

public enum AgentConnectionStatus
{
Green,
Yellow,
Red,
Gray,
Healthy,
Unhealthy,
NoRecentHandshake,
Offline,
}
Comment on lines +85 to +88
Copy link
Member

Choose a reason for hiding this comment

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

On mac, I also have a yellow connecting status when the LastHandshake is missing or negative, otherwise the status dot goes immediately red, and I think that's misleading/needlessly scary.


public partial class AgentViewModel : ObservableObject, IModelUpdateable<AgentViewModel>
Expand Down Expand Up @@ -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.
Comment on lines +221 to +230
Copy link
Member

@ethanndickson ethanndickson Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

On Mac, we include a (High latency) after both first sentences if the ping is unacceptable, otherwise the status icon turns yellow with no indication as to why.


Total latency: {PreferredDerpLatency.Value.Milliseconds} ms
"""
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be the total latency, not just the preferred derp latency?

);

if (PreferredDerp != null && Latency != null)
{
Copy link
Member

@ethanndickson ethanndickson Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't this second null check be against PreferredDerpLatency?

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"}");

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description.Append($"\n\nLast handshake: {LastHandshake?.ToString() ?? "Unknown"}");
description.Append($"\n\nLast handshake: {LastHandshake.ToString()}");

var tooltip = description.ToString().TrimEnd('\n', ' ');

if (string.IsNullOrEmpty(tooltip))
return "No connection information available.";
Copy link
Member

@ethanndickson ethanndickson Jun 24, 2025 •
edited
Loading

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:

    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
    }


Comment on lines +255 to +256
Copy link
Member

Choose a reason for hiding this comment

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

Could this just be an else on the if (p2p) {} else if (derp) {} at the top?

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;
Expand All @@ -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
Expand Down Expand Up @@ -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.

Expand All @@ -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()
Expand All @@ -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();
Expand Down
35 changes: 28 additions & 7 deletions App/ViewModels/TrayWindowViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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

View workflow job for this annotation

GitHub Actions / test 

Dereference of a possibly null reference.

Check warning on line 228 in App/ViewModels/TrayWindowViewModel.cs

View workflow job for this annotation

GitHub Actions / test 

Dereference of a possibly null reference.

Check warning on line 228 in App/ViewModels/TrayWindowViewModel.cs

View workflow job for this annotation

GitHub Actions / build 

Dereference of a possibly null reference.

Check warning on line 228 in App/ViewModels/TrayWindowViewModel.cs

View workflow job for this annotation

GitHub Actions / build 

Dereference of a possibly null reference.
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;
Copy link
Member

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

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,
Expand All @@ -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
Expand All @@ -253,7 +274,7 @@
// conflict with any agent IDs.
uuid,
_hostnameSuffixGetter.GetCachedSuffix(),
AgentConnectionStatus.Gray,
AgentConnectionStatus.Offline,
credentialModel.CoderUrl,
workspace.Name));
}
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions App/Views/Pages/TrayWindowMainPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,22 @@
x:Key="StatusColor"
SelectedKey="{x:Bind Path=ConnectionStatus, Mode=OneWay}">

<converters:StringToBrushSelectorItem>
<converters:StringToBrushSelectorItem Key="Offline">
<converters:StringToBrushSelectorItem.Value>
<SolidColorBrush Color="#8e8e93" />
</converters:StringToBrushSelectorItem.Value>
</converters:StringToBrushSelectorItem>
<converters:StringToBrushSelectorItem Key="Red">
<converters:StringToBrushSelectorItem Key="NoRecentHandshake">
<converters:StringToBrushSelectorItem.Value>
<SolidColorBrush Color="#ff3b30" />
</converters:StringToBrushSelectorItem.Value>
</converters:StringToBrushSelectorItem>
<converters:StringToBrushSelectorItem Key="Yellow">
<converters:StringToBrushSelectorItem Key="Unhealthy">
<converters:StringToBrushSelectorItem.Value>
<SolidColorBrush Color="#ffcc01" />
</converters:StringToBrushSelectorItem.Value>
</converters:StringToBrushSelectorItem>
<converters:StringToBrushSelectorItem Key="Green">
<converters:StringToBrushSelectorItem Key="Healthy">
<converters:StringToBrushSelectorItem.Value>
<SolidColorBrush Color="#34c759" />
</converters:StringToBrushSelectorItem.Value>
Expand Down Expand Up @@ -189,6 +189,7 @@
HorizontalAlignment="Right"
VerticalAlignment="Center"
Height="14" Width="14"
ToolTipService.ToolTip="{x:Bind ConnectionTooltip, Mode=OneWay}"
Margin="0,1,0,0">

<Ellipse
Expand Down
24 changes: 20 additions & 4 deletions Vpn.Proto/vpn.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ option go_package = "github.com/coder/coder/v2/vpn";
option csharp_namespace = "Coder.Desktop.Vpn.Proto";

import "google/protobuf/timestamp.proto";
import "google/protobuf/duration.proto";

package vpn;

Expand Down Expand Up @@ -48,10 +49,10 @@ message TunnelMessage {
message ClientMessage {
RPC rpc = 1;
oneof msg {
StartRequest start = 2;
StopRequest stop = 3;
StatusRequest status = 4;
}
StartRequest start = 2;
StopRequest stop = 3;
StatusRequest status = 4;
}
}

// ServiceMessage is a message from the service (to the client). Windows only.
Expand Down Expand Up @@ -131,6 +132,21 @@ message Agent {
// last_handshake is the primary indicator of whether we are connected to a peer. Zero value or
// anything longer than 5 minutes ago means there is a problem.
google.protobuf.Timestamp last_handshake = 6;
// If unset, a successful ping has not yet been made.
optional LastPing last_ping = 7;
}

message LastPing {
// latency is the RTT of the ping to the agent.
google.protobuf.Duration latency = 1;
// did_p2p indicates whether the ping was sent P2P, or over DERP.
bool did_p2p = 2;
// preferred_derp is the human readable name of the preferred DERP region,
// or the region used for the last ping, if it was sent over DERP.
string preferred_derp = 3;
// preferred_derp_latency is the last known latency to the preferred DERP
// region. Unset if the region does not appear in the DERP map.
optional google.protobuf.Duration preferred_derp_latency = 4;
}

// NetworkSettingsRequest is based on
Expand Down
Loading