Skip to content

Detect and report latency measurements for SSH sessions to the UI#34862

Merged
rosstimothy merged 1 commit intomasterfrom
tross/latency
Dec 6, 2023
Merged

Detect and report latency measurements for SSH sessions to the UI#34862
rosstimothy merged 1 commit intomasterfrom
tross/latency

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy commented Nov 21, 2023

Monitors both the UI<->Proxy and Proxy<->SSH host connections in order to provide users with near real time latency data for ssh connections established via the UI. The client portion of the connection is measured by how long it takes to receive a web socket pong in response to sending a web socket ping. The host portion of the connection is measured by how long it takes to receive a reply of a keepalive@openssh.com global SSH request. The statistics are periodically sent via a new envelope type over the web socket where they are consumed and displayed to users.

Changelog: Calculate latency of Web SSH sessions and report it to users.

@rosstimothy rosstimothy requested a review from avatus November 21, 2023 21:14
@rosstimothy rosstimothy force-pushed the tross/latency branch 4 times, most recently from 85c2de0 to 370dcb6 Compare November 21, 2023 22:05
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/Console/ActionBar/ActionBar.tsx Outdated
Comment thread web/packages/teleport/src/lib/useLatency.ts Outdated
Comment thread web/packages/teleport/src/lib/useLatency.ts Outdated
Comment thread web/packages/teleport/src/lib/useLatency.ts Outdated
@rosstimothy rosstimothy force-pushed the tross/latency branch 7 times, most recently from 63d884e to bf2959f Compare November 28, 2023 14:11
@rosstimothy rosstimothy marked this pull request as ready for review November 28, 2023 15:52
Comment thread lib/utils/diagnostics/latency/monitor.go Outdated
Comment thread lib/utils/diagnostics/latency/monitor.go
Comment thread lib/utils/diagnostics/latency/monitor.go Outdated
Comment thread lib/utils/diagnostics/latency/monitor.go
Comment thread lib/utils/diagnostics/latency/monitor.go Outdated
Comment thread lib/utils/diagnostics/latency/monitor.go Outdated
Comment thread lib/web/terminal.go Outdated
@rosstimothy rosstimothy force-pushed the tross/latency branch 2 times, most recently from db17b19 to 16873d2 Compare December 1, 2023 19:36
@rosstimothy
Copy link
Copy Markdown
Contributor Author

@avatus @ibeckermayer I tweaked the frontend portion of this based on some feedback from the design team. Would you mind looking things over once more? I'm particularly interested to know if there is a better way to apply some of the css tweaks that I made.

Comment on lines +79 to +82
if (
(client >= warnThreshold || server >= warnThreshold) &&
(client < warnThreshold || server < warnThreshold)
) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is fine but it seems like (correct me if I'm wrong) that the pattern is basically "show the highest threshold as the total". So if any are red, it'll be red, and if any are yellow, it'll show yellow.

I wonder if instead of doing these comparisons we could just do another small helper function like getColorForTotal and have the logic be

if either are red, return red
if either are yellow, return yellow
return green

thoughts?

Copy link
Copy Markdown
Contributor Author

@rosstimothy rosstimothy Dec 5, 2023

Choose a reason for hiding this comment

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

These were the rules that I was given from the design team regarding total color:

  • green + green = green
  • green + yellow = yellow
  • green + red = red
  • yellow + yellow = yellow
  • yellow + red = red
  • red + red = red

Initially the total color was solely based on the total latency, but that lead to the warning color being used for the total even though both legs of the connection were green. The feedback was this was confusing and somewhat alarming which resulted in the request to just stick with a green total color if both legs are ok.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplified a bit in d688d2cd494a663f2652e1382cbad3d03af5ab09. WDYT?

Copy link
Copy Markdown
Contributor

@avatus avatus left a comment

Choose a reason for hiding this comment

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

small nit comment but css looks fine to me (i'm not a css person really so if it works it works)

@rosstimothy rosstimothy requested a review from gzdunek December 4, 2023 22:25
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from fspmarshall December 6, 2023 12:28
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
@gravitational gravitational deleted a comment from github-actions Bot Dec 6, 2023
Comment thread lib/utils/diagnostics/latency/monitor.go Outdated
Comment thread lib/utils/diagnostics/latency/monitor.go Outdated
Comment thread lib/utils/interval/multi.go Outdated
@rosstimothy rosstimothy force-pushed the tross/latency branch 2 times, most recently from fae999f to b5739d4 Compare December 6, 2023 22:01
Monitors both the UI<->Proxy and Proxy<->SSH host connections in
order to provide users with near real time latency data for ssh
connections established via the UI. The client portion of the
connection is measured by how long it takes to receive a web socket
pong in response to sending a web socket ping. The host portion of
the connection is measured by how long it takes to receive a reply
of a keepalive@openssh.com global SSH request. The statistics are
periodically sent via a new envelope type over the web socket where
they are consumed and displayed to users.
@rosstimothy rosstimothy enabled auto-merge December 6, 2023 22:31
@rosstimothy rosstimothy added this pull request to the merge queue Dec 6, 2023
Merged via the queue into master with commit 26ae87a Dec 6, 2023
@rosstimothy rosstimothy deleted the tross/latency branch December 6, 2023 22:53
@public-teleport-github-review-bot
Copy link
Copy Markdown

@rosstimothy See the table below for backport results.

Branch Result
branch/v12 Failed
branch/v13 Failed
branch/v14 Failed

rosstimothy added a commit that referenced this pull request Dec 7, 2023
…4862)

Monitors both the UI<->Proxy and Proxy<->SSH host connections in
order to provide users with near real time latency data for ssh
connections established via the UI. The client portion of the
connection is measured by how long it takes to receive a web socket
pong in response to sending a web socket ping. The host portion of
the connection is measured by how long it takes to receive a reply
of a keepalive@openssh.com global SSH request. The statistics are
periodically sent via a new envelope type over the web socket where
they are consumed and displayed to users.
github-merge-queue Bot pushed a commit that referenced this pull request Dec 7, 2023
…4862) (#35516)

Monitors both the UI<->Proxy and Proxy<->SSH host connections in
order to provide users with near real time latency data for ssh
connections established via the UI. The client portion of the
connection is measured by how long it takes to receive a web socket
pong in response to sending a web socket ping. The host portion of
the connection is measured by how long it takes to receive a reply
of a keepalive@openssh.com global SSH request. The statistics are
periodically sent via a new envelope type over the web socket where
they are consumed and displayed to users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants