Skip to content
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

Add SSH connection analytics #10074

Merged
merged 4 commits into from
May 19, 2022
Merged

Add SSH connection analytics #10074

merged 4 commits into from
May 19, 2022

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented May 17, 2022

Description

This PR introduce SSH connection analytics in ws-proxy

Related Issue(s)

Fixes #

How to test

  1. open a workspace
  2. use copy/paste SSH command to connect
  3. watch segment event, you will see ssh_connection event

image
image

Release Notes

Add SSH connection analytics

Documentation

  • /werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe

@akosyakov
Copy link
Member

akosyakov commented May 18, 2022

@iQQBot Could you add a new event in https://www.notion.so/gitpod/Tracking-Plan-ed27d1492da745b9b31711dd73dcbe57 please, explain properties, phases and possible values? 🙏

@iQQBot
Copy link
Contributor Author

iQQBot commented May 18, 2022

Added https://www.notion.so/gitpod/ssh_connection-60f7d649667b4f8ead093daaf7ac619e

@iQQBot iQQBot marked this pull request as ready for review May 18, 2022 08:19
@iQQBot iQQBot requested review from a team May 18, 2022 08:19
@github-actions github-actions bot added team: delivery Issue belongs to the self-hosted team team: workspace Issue belongs to the Workspace team labels May 18, 2022
@akosyakov
Copy link
Member

@iQQBot Could we extract an improvement in another PR? I think the improvement is questionable and need to be discussed. Analytics though looks good already. I would like analytics parts get merged.

@iQQBot
Copy link
Contributor Author

iQQBot commented May 18, 2022

@akosyakov @andreafalzetti Already do some cleanup, now this PR only include analytics for ssh connection

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

lgtm, thank you a lot!

@akosyakov
Copy link
Member

akosyakov commented May 18, 2022

Could you also add a metric for prometheus with another PR? I don't think we need to track duration of ssh connections, but having a counter of auth/connection attempts and success rate will be helpful. We don't need to label workspaces or collect any error information but only use it for monitoring and alerts. If we see a spike then we should go to logs and check them.

@andreafalzetti
Copy link
Contributor

andreafalzetti commented May 18, 2022

watch segment event, you will see ssh_connection event

Where do I verify the data? From segment.com? 🤔

@iQQBot
Copy link
Contributor Author

iQQBot commented May 18, 2022

watch segment event, you will see ssh_connection event

Where do I verify the data? From segment.com? 🤔

Yes, login your account then find source staging untrust -> debugger https://app.segment.com/gitpod/sources/staging_untrusted/debugger

@iQQBot
Copy link
Contributor Author

iQQBot commented May 18, 2022

Could you also add a metric for prometheus with another PR? I don't think we need to track duration of ssh connections, but having a counter of auth/connection attempts and success rate will be helpful. We don't need to label workspaces or collect any error information but only use it for monitoring and alerts. If we see a spike then we should go to logs and check them.

Ok, no problem, just one point needs to be clarified, as a public network to expose the IP of port 22, is bound to be a variety of hackers violent crack every day, I suggest that the success rate of the statistics here should ignore the user name length of less than 6 digits

@akosyakov
Copy link
Member

Ok, no problem, just one point needs to be clarified, as a public network to expose the IP of port 22, is bound to be a variety of hackers violent crack every day, I suggest that the success rate of the statistics here should ignore the user name length of less than 6 digits

Let's try without first?

@iQQBot
Copy link
Contributor Author

iQQBot commented May 18, 2022

Ok, no problem, just one point needs to be clarified, as a public network to expose the IP of port 22, is bound to be a variety of hackers violent crack every day, I suggest that the success rate of the statistics here should ignore the user name length of less than 6 digits

Let's try without first?

OK

Copy link
Contributor

@nandajavarma nandajavarma left a comment

Choose a reason for hiding this comment

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

LGTM from the installer side. Also, noting that this does not affect the self hosted user.

@roboquat roboquat merged commit 3c21eb6 into main May 19, 2022
@roboquat roboquat deleted the pd/ssh-analyze branch May 19, 2022 06:36
@roboquat roboquat added the deployed: workspace Workspace team change is running in production label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production release-note size/L team: delivery Issue belongs to the self-hosted team team: workspace Issue belongs to the Workspace team
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants