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.

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 5 commits into
base: main
Choose a base branch
from

Conversation

ibetitsmike
Copy link
Contributor

Fixes: #106


// 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

Copy link
Contributor Author

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.


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

if (string.IsNullOrEmpty(tooltip))
Copy link
Member

@ethanndickson ethanndickson Jun 24, 2025

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
    }

Copy link
Contributor Author

@ibetitsmike ibetitsmike Jun 24, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show latency on workspace view
3 participants