Skip to content

[vnet] feat: SSH usage reporting#56537

Merged
nklaassen merged 5 commits intomasterfrom
nklaassen/vnet-ssh-metrics
Jul 18, 2025
Merged

[vnet] feat: SSH usage reporting#56537
nklaassen merged 5 commits intomasterfrom
nklaassen/vnet-ssh-metrics

Conversation

@nklaassen
Copy link
Copy Markdown
Contributor

This PR adds usage reporting for VNet SSH sessions used via Connect.

@nklaassen nklaassen requested a review from ravicious July 8, 2025 01:02
@nklaassen nklaassen added backport-required no-changelog Indicates that a PR does not require a changelog entry vnet labels Jul 8, 2025
@github-actions github-actions Bot added size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Jul 8, 2025
Comment thread lib/teleterm/vnet/service.go
Comment thread lib/teleterm/vnet/service.go Outdated
Comment thread lib/teleterm/vnet/service.go Outdated
Comment thread lib/teleterm/vnet/service.go
@nklaassen nklaassen requested a review from ravicious July 8, 2025 16:49
func (p *vnetClientApplication) OnNewSSHSession(ctx context.Context, profileName, rootClusterName string) {
}

// OnNewConnection gets called before each VNet connection. It's a noop as tsh doesn't need to do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that I think about it, OnNewConnection ceased to be a good name and the comment makes it sound like it's being called for each new connection.

This name is used in many places so renaming it might be a bit of a pain, if you don't feel like taking care of this right now, then could you update the comments at least?

$ rg -B 2 "func.*OnNewConnection" -g 'lib/vnet/**' -g 'lib/teleterm/vnet/**' -g 'tool/tsh/**'
tool/tsh/common/vnet_client_application.go
142-// OnNewConnection gets called before each VNet connection. It's a noop as tsh doesn't need to do
143-// anything extra here.
144:func (p *vnetClientApplication) OnNewConnection(_ context.Context, _ *vnetv1.AppKey) error {

lib/vnet/client_application_service_client.go
136-
137-// OnNewConnection reports a new TCP connection to the target app.
138:func (c *clientApplicationServiceClient) OnNewConnection(ctx context.Context, appKey *vnetv1.AppKey) error {

lib/vnet/client_application_service.go
220-// OnNewConnection gets called whenever a new connection is about to be
221-// established through VNet for observability.
222:func (s *clientApplicationService) OnNewConnection(ctx context.Context, req *vnetv1.OnNewConnectionRequest) (*vnetv1.OnNewConnectionResponse, error) {

lib/vnet/app_provider.go
76-
77-// OnNewConnection reports a new TCP connection to the target app.
78:func (p *appProvider) OnNewConnection(ctx context.Context, appKey *vnetv1.AppKey) error {

lib/vnet/vnet_test.go
571-}
572-
573:func (p *fakeClientApp) OnNewConnection(_ context.Context, _ *vnetv1.AppKey) error {
--
1045-// TestOnNewConnection tests that the client applications OnNewConnection method
1046-// is called when a user connects to a valid TCP app.
1047:func TestOnNewConnection(t *testing.T) {

lib/vnet/app_handler.go
173-}
174-
175:func (m *localProxyMiddleware) OnNewConnection(ctx context.Context, lp *alpnproxy.LocalProxy) error {

lib/teleterm/vnet/service.go
549-// event. This is to mimic how Connect submits events for its app gateways. This lets us compare
550-// popularity of VNet and app gateways.
551:func (p *clientApplication) OnNewConnection(ctx context.Context, appKey *vnetv1.AppKey) error {

// OnNewConnection gets called whenever a new connection is about to be
// established through VNet for observability.
rpc OnNewConnection(OnNewConnectionRequest) returns (OnNewConnectionResponse);

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.

i did the rename

@nklaassen
Copy link
Copy Markdown
Contributor Author

friendly ping @capnspacehook or @probakowski

Comment on lines +538 to +540
// Not passing ctx to ReportSSHSession since ctx is tied to the
// lifetime of a short-lived API call, inheriting the context could
// interrupt reporting.
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.

Not sure that it matters much since the context only appears to be used for logging in the ReportSSHSession implementation, but you could use context.WithoutCancel to ensure any contextual information is forwarded without risking the request context interrupting the usage reporting.

@nklaassen nklaassen added this pull request to the merge queue Jul 18, 2025
Merged via the queue into master with commit 0c62706 Jul 18, 2025
41 checks passed
@nklaassen nklaassen deleted the nklaassen/vnet-ssh-metrics branch July 18, 2025 21:54
nklaassen added a commit that referenced this pull request Jul 18, 2025
nklaassen added a commit that referenced this pull request Jul 18, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Jul 22, 2025
* [v17][vnet] feat: TCP dial to SSH targets

Backport #55087 to branch/v17

* [v17][vnet] feat: accept incoming SSH connections

Backport #55155 to branch/v17

* [v17][vnet] feat: forward SSH connections to target

Backport #55156 to branch/v17

* [v17][vnet] feat: write VNet SSH keys to TELEPORT_HOME

Backport #55228 to branch/v17

* [v17][vnet] feat: write OpenSSH-compatible config file for VNet SSH

Backport #55239 to branch/v17

* [v17][vnet] fix: support <hostname>.<leaf-cluster> for VNet SSH

Backport #55688 to branch/v17

* fix BlockUntil API for backport

* [v17][vnet] feat: add "Connect with VNet" button to SSH servers

Backport #55623 to branch/v17

* [v17][vnet] feat: support VNet SSH when cluster name does not match proxy public addr

Backport #55655 to branch/v17

* [v17][vnet] feat: add SSH configuration diagnostic

Backport #55594 to branch/v17

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

* [v17][vnet] feat: show SSH status in VNet slider

Backport #55755 to branch/v17

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

* [v17][vnet] feat: support proxy recording mode with VNet SSH

Backport #55788 to branch/v17

* [v17][vnet] feat: support diag checks on windows

Backport #55856 to branch/v17

* [v17] fix: data race in vnet.TestSSH

Backport #55980 to branch/v17

* [v17][vnet] feat: mention SSH on VNet info page

Backport #55973 to branch/v17

* [v17][vnet] feat: serve DNS on IPv4

Backport #55539 to branch/v17

* [v17][vnet] fix: close proxied channel only after data and requests are complete

Backport #56020 to branch/v17

* [v17][vnet] feat: automatic SSH client configuration

Backport #55923 to branch/v17

* VNet docs: Provide clear instructions for getting debug logs (#56068)

* VNet diag notification: Do not show button to open report if there's no workspace selected (#56067)

* VNet diag report: Don't show button in notification if there's no workspace

* Replace deprecated MutableRefObject with RefObject

* Make openReport not depend on value of rootClusterUri

Otherwise the effect that uses setInterval re-runs whenever the user
switches to another workspace.

* [v17][vnet] feat: automatic SSH client configuration in Connect

Backport #55924 to branch/v17

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>

* [v17][vnet] fix: avoid empty host matchers in generated SSH config

Backport #56103 to branch/v17

* avoid t.Context() pre go1.24

* fix cspell lint

* [v17][docs] VNet SSH

Backport #56147 to branch/v17

* [v17][vnet] feat: SSH usage reporting

Backport #56537 to branch/v17

* [v17][vnet] fix: mask default IP route on windows

Backport #56957 to branch/v17

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Jul 22, 2025
* [v18][vnet] feat: TCP dial to SSH targets

Backport #55087 to branch/v18

* [v18][vnet] feat: accept incoming SSH connections

Backport #55155 to branch/v18

* [v18][vnet] feat: forward SSH connections to target

Backport #55156 to branch/v18

* [v18][vnet] feat: write VNet SSH keys to TELEPORT_HOME

Backport #55228 to branch/v18

* [v18][vnet] feat: write OpenSSH-compatible config file for VNet SSH

Backport #55239 to branch/v18

* [v18][vnet] fix: support <hostname>.<leaf-cluster> for VNet SSH

Backport #55688 to branch/v18

* [v18][vnet] feat: add "Connect with VNet" button to SSH servers

Backport #55623 to branch/v18

* fix test in backport

* [v18][vnet] feat: support VNet SSH when cluster name does not match proxy public addr

Backport #55655 to branch/v18

* [v18][vnet] feat: add SSH configuration diagnostic

Backport #55594 to branch/v18

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

* [v18][vnet] feat: show SSH status in VNet slider

Backport #55755 to branch/v18

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

* [v18][vnet] feat: support proxy recording mode with VNet SSH

Backport #55788 to branch/v18

* [v18][vnet] feat: support diag checks on windows

Backport #55856 to branch/v18

* [v18] fix: data race in vnet.TestSSH

Backport #55980 to branch/v18

* [v18][vnet] feat: mention SSH on VNet info page

Backport #55973 to branch/v18

* [v18][vnet] feat: serve DNS on IPv4

Backport #55539 to branch/v18

* [v18][vnet] fix: close proxied channel only after data and requests are complete

Backport #56020 to branch/v18

* [v18][vnet] feat: automatic SSH client configuration

Backport #55923 to branch/v18

* VNet diag notification: Do not show button to open report if there's no workspace selected (#56067)

* VNet diag report: Don't show button in notification if there's no workspace

* Replace deprecated MutableRefObject with RefObject

* Make openReport not depend on value of rootClusterUri

Otherwise the effect that uses setInterval re-runs whenever the user
switches to another workspace.

* [v18][vnet] feat: automatic SSH client configuration in Connect

Backport #55924 to branch/v18

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>

* [v18][vnet] fix: avoid empty host matchers in generated SSH config

Backport #56103 to branch/v18

* [v18][docs] VNet SSH

Backport #56147 to branch/v18

* [v18][docs] add VNet warnings

Backport #56601 to branch/v18

* [v18][vnet] feat: SSH usage reporting

Backport #56537 to branch/v18

* [v18][vnet] fix: mask default IP route on windows

Backport #56957 to branch/v18

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport. vnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants