Skip to content

feat: include ping and network stats on status tooltip #181

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 3 additions & 0 deletions Coder-Desktop/Coder-Desktop/Coder_DesktopApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ class AppDelegate: NSObject, NSApplicationDelegate {
}

func applicationDidFinishLaunching(_: Notification) {
// We have important file sync and network info behind tooltips,
// so the default delay is too long.
UserDefaults.standard.setValue(Theme.Animation.tooltipDelay, forKey: "NSInitialToolTipDelay")
// Init SVG loader
SDImageCodersManager.shared.addCoder(SDImageSVGCoder.shared)

Expand Down
1 change: 1 addition & 0 deletions Coder-Desktop/Coder-Desktop/Theme.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum Theme {

enum Animation {
static let collapsibleDuration = 0.2
static let tooltipDelay: Int = 250 // milliseconds
}

static let defaultVisibleAgents = 5
Expand Down
166 changes: 161 additions & 5 deletions Coder-Desktop/Coder-Desktop/VPN/MenuState.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import SwiftProtobuf
import SwiftUI
import VPNLib

Expand All @@ -9,6 +10,29 @@ struct Agent: Identifiable, Equatable, Comparable, Hashable {
let hosts: [String]
let wsName: String
let wsID: UUID
let lastPing: LastPing?
let lastHandshake: Date?

init(id: UUID,
name: String,
status: AgentStatus,
hosts: [String],
wsName: String,
wsID: UUID,
lastPing: LastPing? = nil,
lastHandshake: Date? = nil,
primaryHost: String)
{
self.id = id
self.name = name
self.status = status
self.hosts = hosts
self.wsName = wsName
self.wsID = wsID
self.lastPing = lastPing
self.lastHandshake = lastHandshake
self.primaryHost = primaryHost
}

// Agents are sorted by status, and then by name
static func < (lhs: Agent, rhs: Agent) -> Bool {
Expand All @@ -18,21 +42,89 @@ struct Agent: Identifiable, Equatable, Comparable, Hashable {
return lhs.wsName.localizedCompare(rhs.wsName) == .orderedAscending
}

var statusString: String {
if status == .error {
return status.description
}

guard let lastPing else {
// either:
// - old coder deployment
// - we haven't received any pings yet
return status.description
}

var str: String
if lastPing.didP2p {
str = """
You're connected peer-to-peer.

You ↔ \(lastPing.latency.prettyPrintMs) ↔ \(wsName)
"""
} else {
str = """
You're connected through a DERP relay.
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
}

let primaryHost: String
}

extension TimeInterval {
var prettyPrintMs: String {
let milliseconds = self * 1000
return "\(milliseconds.formatted(.number.precision(.fractionLength(2)))) ms"
}
}

struct LastPing: Equatable, Hashable {
let latency: TimeInterval
let didP2p: Bool
let preferredDerp: String
let preferredDerpLatency: TimeInterval?
}

enum AgentStatus: Int, Equatable, Comparable {
case okay = 0
case warn = 1
case error = 2
case off = 3
case connecting = 1
case warn = 2
case error = 3
case off = 4

public var description: String {
switch self {
case .okay: "Connected"
case .connecting: "Connecting..."
case .warn: "Connected, but with high latency" // Currently unused
case .error: "Could not establish a connection to the agent. Retrying..."
case .off: "Offline"
}
}

public var color: Color {
switch self {
case .okay: .green
case .warn: .yellow
case .error: .red
case .off: .secondary
case .connecting: .yellow
}
}

Expand Down Expand Up @@ -87,14 +179,27 @@ struct VPNMenuState {
workspace.agents.insert(id)
workspaces[wsID] = workspace

var lastPing: LastPing?
if agent.hasLastPing {
lastPing = LastPing(
latency: agent.lastPing.latency.timeInterval,
didP2p: agent.lastPing.didP2P,
preferredDerp: agent.lastPing.preferredDerp,
preferredDerpLatency:
agent.lastPing.hasPreferredDerpLatency
? agent.lastPing.preferredDerpLatency.timeInterval
: nil
)
}
agents[id] = Agent(
id: id,
name: agent.name,
// If last handshake was not within last five minutes, the agent is unhealthy
status: agent.lastHandshake.date > Date.now.addingTimeInterval(-300) ? .okay : .warn,
status: agent.status,
hosts: nonEmptyHosts,
wsName: workspace.name,
wsID: wsID,
lastPing: lastPing,
lastHandshake: agent.lastHandshake.maybeDate,
// Hosts arrive sorted by length, the shortest looks best in the UI.
primaryHost: nonEmptyHosts.first!
)
Expand Down Expand Up @@ -154,3 +259,54 @@ struct VPNMenuState {
workspaces.removeAll()
}
}

extension Date {
var relativeTimeString: String {
let formatter = RelativeDateTimeFormatter()
formatter.unitsStyle = .full
if Date.now.timeIntervalSince(self) < 1.0 {
// Instead of showing "in 0 seconds"
return "Just now"
}
return formatter.localizedString(for: self, relativeTo: Date.now)
}
}

extension SwiftProtobuf.Google_Protobuf_Timestamp {
var maybeDate: Date? {
guard seconds > 0 else { return nil }
return date
}
}

extension Vpn_Agent {
var healthyLastHandshakeMin: Date {
Date.now.addingTimeInterval(-300) // 5 minutes ago
}

var healthyPingMax: TimeInterval { 0.15 } // 150ms

var status: AgentStatus {
guard let lastHandshake = lastHandshake.maybeDate else {
// Initially the handshake is missing
return .connecting
}

return if lastHandshake < healthyLastHandshakeMin {
// If last handshake was not within the last five minutes, the agent
// is potentially unhealthy.
.error
} else if hasLastPing, lastPing.latency.timeInterval < healthyPingMax {
// If latency is less than 150ms
.okay
} else if hasLastPing, lastPing.latency.timeInterval >= healthyPingMax {
// if latency is greater than 150ms
.warn
} else {
// No ping data, but we have a recent handshake.
// We show green for backwards compatibility with old Coder
// deployments.
.okay
}
}
}
8 changes: 8 additions & 0 deletions Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenuItem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ enum VPNMenuItem: Equatable, Comparable, Identifiable {
}
}

var statusString: String {
switch self {
case let .agent(agent): agent.statusString
case .offlineWorkspace: status.description
}
}

var id: UUID {
switch self {
case let .agent(agent): agent.id
Expand Down Expand Up @@ -224,6 +231,7 @@ struct MenuItemIcons: View {
StatusDot(color: item.status.color)
.padding(.trailing, 3)
.padding(.top, 1)
.help(item.statusString)
MenuItemIconButton(systemName: "doc.on.doc", action: copyToClipboard)
.font(.system(size: 9))
.symbolVariant(.fill)
Expand Down
1 change: 1 addition & 0 deletions Coder-Desktop/Coder-DesktopTests/AgentsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ struct AgentsTests {
hosts: ["a\($0).coder"],
wsName: "ws\($0)",
wsID: UUID(),
lastPing: nil,
primaryHost: "a\($0).coder"
)
return (agent.id, agent)
Expand Down
63 changes: 62 additions & 1 deletion Coder-Desktop/Coder-DesktopTests/VPNMenuStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ struct VPNMenuStateTests {
$0.workspaceID = workspaceID.uuidData
$0.name = "dev"
$0.lastHandshake = .init(date: Date.now)
$0.lastPing = .with {
$0.latency = .init(floatLiteral: 0.05)
$0.didP2P = true
}
$0.fqdn = ["foo.coder"]
}

Expand All @@ -29,6 +33,9 @@ struct VPNMenuStateTests {
#expect(storedAgent.wsName == "foo")
#expect(storedAgent.primaryHost == "foo.coder")
#expect(storedAgent.status == .okay)
#expect(storedAgent.statusString.contains("You're connected peer-to-peer."))
#expect(storedAgent.statusString.contains("You ↔ 50.00 ms ↔ foo"))
#expect(storedAgent.statusString.contains("Last handshake: Just now"))
}

@Test
Expand Down Expand Up @@ -72,6 +79,49 @@ struct VPNMenuStateTests {
#expect(state.workspaces[workspaceID] == nil)
}

@Test
mutating func testUpsertAgent_poorConnection() async throws {
let agentID = UUID()
let workspaceID = UUID()
state.upsertWorkspace(Vpn_Workspace.with { $0.id = workspaceID.uuidData; $0.name = "foo" })

let agent = Vpn_Agent.with {
$0.id = agentID.uuidData
$0.workspaceID = workspaceID.uuidData
$0.name = "agent1"
$0.lastHandshake = .init(date: Date.now)
$0.lastPing = .with {
$0.latency = .init(seconds: 1)
}
$0.fqdn = ["foo.coder"]
}

state.upsertAgent(agent)

let storedAgent = try #require(state.agents[agentID])
#expect(storedAgent.status == .warn)
}

@Test
mutating func testUpsertAgent_connecting() async throws {
let agentID = UUID()
let workspaceID = UUID()
state.upsertWorkspace(Vpn_Workspace.with { $0.id = workspaceID.uuidData; $0.name = "foo" })

let agent = Vpn_Agent.with {
$0.id = agentID.uuidData
$0.workspaceID = workspaceID.uuidData
$0.name = "agent1"
$0.lastHandshake = .init()
$0.fqdn = ["foo.coder"]
}

state.upsertAgent(agent)

let storedAgent = try #require(state.agents[agentID])
#expect(storedAgent.status == .connecting)
}

@Test
mutating func testUpsertAgent_unhealthyAgent() async throws {
let agentID = UUID()
Expand All @@ -89,7 +139,7 @@ struct VPNMenuStateTests {
state.upsertAgent(agent)

let storedAgent = try #require(state.agents[agentID])
#expect(storedAgent.status == .warn)
#expect(storedAgent.status == .error)
}

@Test
Expand All @@ -114,6 +164,9 @@ struct VPNMenuStateTests {
$0.workspaceID = workspaceID.uuidData
$0.name = "agent1" // Same name as old agent
$0.lastHandshake = .init(date: Date.now)
$0.lastPing = .with {
$0.latency = .init(floatLiteral: 0.05)
}
$0.fqdn = ["foo.coder"]
}

Expand Down Expand Up @@ -146,6 +199,10 @@ struct VPNMenuStateTests {
$0.workspaceID = workspaceID.uuidData
$0.name = "agent1"
$0.lastHandshake = .init(date: Date.now.addingTimeInterval(-200))
$0.lastPing = .with {
$0.didP2P = false
$0.latency = .init(floatLiteral: 0.05)
}
$0.fqdn = ["foo.coder"]
}
state.upsertAgent(agent)
Expand All @@ -155,6 +212,10 @@ struct VPNMenuStateTests {
#expect(output[0].id == agentID)
#expect(output[0].wsName == "foo")
#expect(output[0].status == .okay)
let storedAgentFromSort = try #require(state.agents[agentID])
#expect(storedAgentFromSort.statusString.contains("You're connected through a DERP relay."))
#expect(storedAgentFromSort.statusString.contains("Total latency: 50.00 ms"))
#expect(storedAgentFromSort.statusString.contains("Last handshake: 3 minutes ago"))
}

@Test
Expand Down
3 changes: 1 addition & 2 deletions Coder-Desktop/VPN/Manager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ actor Manager {
dest: dest,
urlSession: URLSession(configuration: sessionConfig)
) { progress in
// TODO: Debounce, somehow
pushProgress(stage: .downloading, downloadProgress: progress)
}
} catch {
Expand Down Expand Up @@ -322,7 +321,7 @@ func writeVpnLog(_ log: Vpn_Log) {
category: log.loggerNames.joined(separator: ".")
)
let fields = log.fields.map { "\($0.name): \($0.value)" }.joined(separator: ", ")
logger.log(level: level, "\(log.message, privacy: .public): \(fields, privacy: .public)")
logger.log(level: level, "\(log.message, privacy: .public)\(fields.isEmpty ? "" : ": \(fields)", privacy: .public)")
}

private func removeQuarantine(_ dest: URL) async throws(ManagerError) {
Expand Down
Loading
Loading