From 830996ecdcbe41b29d0741670929b9576b76c94c Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Wed, 16 Jul 2025 14:25:01 +0100 Subject: [PATCH 01/15] [tctl] Add support for debug sock endpoint with fallback in top With this change `tctl top` will now attempt to connect to the local socket debug endpoint before defaulting to the default diag addr. If the user specifies the endpoint explicitly then the command will always attempt the given address regardless of success. This matches previous behavior. The implementation is not concurrent as only two options exist, both of which are local and not expected to have long round trips. Closes: #56673 changelog: `tctl top` now supports the local unix sock debug endpoint. --- docs/pages/reference/cli/tctl.mdx | 16 +++++++--- tool/tctl/common/top/client.go | 53 +++++++++++++++++++++++++++++++ tool/tctl/common/top/command.go | 36 +++++++++++++++++++-- 3 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 tool/tctl/common/top/client.go diff --git a/docs/pages/reference/cli/tctl.mdx b/docs/pages/reference/cli/tctl.mdx index dd9002a0ae9ae..44ab519bf1796 100644 --- a/docs/pages/reference/cli/tctl.mdx +++ b/docs/pages/reference/cli/tctl.mdx @@ -1356,7 +1356,7 @@ $ tctl rm saml/okta # Delete a local user called "admin": $ tctl rm users/admin -# Delete a lock +# Delete a lock $ tctl rm lock/ed7038cb-a3cc-4f59-8063-09553665b773 ``` @@ -1850,12 +1850,18 @@ $ tctl tokens rm [] Reports diagnostic information. -The diagnostic metrics endpoint must be enabled with `teleport start --diag-addr=` for `tctl top` to work. +`tctl top` can consume metrics from a HTTP diagnostic endpoint or a local UNIX socket. ```code $ tctl top [] [] ``` +When a specific endpoint is provided, `tctl top` will always attempt to connect to it, in this case +the diagnostic metrics endpoint must be enabled with `teleport start --diag-addr=`. + +When no endpoint is specified, `tctl top` will attempt to connect via the debug UNIX socket endpoint, falling back to +localhost. + ### Argument - `[]` Diagnostic HTTP URL (HTTPS not supported) @@ -1867,6 +1873,8 @@ $ tctl top [] [] $ sudo teleport start --diag-addr=127.0.0.1:3000 # View stats with a refresh period of 5 seconds $ tctl top http://127.0.0.1:3000 5s +# View stats using the debug service socket +$ tctl top ``` ## tctl users add @@ -2159,7 +2167,7 @@ the two user filter flags imply that if a user - was provisioned into Teleport by Okta (from `--user-origin okta`), OR - has the label values `role=aws-admin` AND `dept=engineering` (from `--user-label "role=aws-admin,dept=engineering"`) -then they will be provisioned into AWS Identity Center by Teleport. +then they will be provisioned into AWS Identity Center by Teleport. ## tctl plugins install okta @@ -2176,7 +2184,7 @@ Install the Okta integration. | `--api-token` | none | string | Optional. Okta API token for the plugin to use. | | `--[no-]scim` | `--no-scim` | boolean | Optional. Enable SCIM Okta integration. | | `--[no-]users-sync` | `--users-sync` | none | Optional. Enable user synchronization. | -| `-o`, `--owner` | none | string | Optional. Add a default owner for synced Access Lists. | +| `-o`, `--owner` | none | string | Optional. Add a default owner for synced Access Lists. | | `--[no-]accesslist-sync` | `--accesslist-sync` | none | Optional. Enable or disable group to Access List synchronization. | | `--[no-]appgroup-sync` | `--appgroup-sync` | none | Optional. Enable or disable Okta Applications and Groups sync. | | `-g`, `--group-filter` | none | string | Optional. Add a group filter. Supports globbing by default. Enclose in `^pattern$` for full regex support. | diff --git a/tool/tctl/common/top/client.go b/tool/tctl/common/top/client.go new file mode 100644 index 0000000000000..d5d1b6337e612 --- /dev/null +++ b/tool/tctl/common/top/client.go @@ -0,0 +1,53 @@ +// Teleport +// Copyright (C) 2025 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package top + +import ( + "context" + "errors" + "net/url" + + "github.com/gravitational/roundtrip" + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/lib/httplib" +) + +type ClientConfig struct { + addr string + opts []roundtrip.ClientParam +} + +func tryNewClient(ctx context.Context, cfgs ...ClientConfig) (*roundtrip.Client, error) { + var errs []error + for _, config := range cfgs { + client, err := roundtrip.NewClient(config.addr, "", config.opts...) + if err != nil { + errs = append(errs, trace.Wrap(err, "failed create client for %v", config.addr)) + continue + } + + if _, err = httplib.ConvertResponse(client.Get(ctx, client.Endpoint("metrics"), url.Values{})); err != nil { + errs = append(errs, trace.Wrap(err, "failed to fetch metrics from addr %v", config.addr)) + continue + } + + return client, nil + } + + return nil, errors.Join(errs...) +} diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index b917ff5fb9fcb..0bae6d3d39aa2 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -18,6 +18,9 @@ package top import ( "context" + "net" + "net/http" + "path/filepath" "time" "github.com/alecthomas/kingpin/v2" @@ -38,23 +41,52 @@ type Command struct { top *kingpin.CmdClause diagURL string refreshPeriod time.Duration + forceURL bool } // Initialize sets up the "tctl top" command. func (c *Command) Initialize(app *kingpin.Application, _ *tctlcfg.GlobalCLIFlags, config *servicecfg.Config) { c.config = config c.top = app.Command("top", "Report diagnostic information.") - c.top.Arg("diag-addr", "Diagnostic HTTP URL").Default("http://127.0.0.1:3000").StringVar(&c.diagURL) + c.top.Arg("diag-addr", "Diagnostic HTTP URL").Default("http://127.0.0.1:3000").Action(func(*kingpin.ParseContext) error { + // kingpin does not support checking if a positional argument was set explicitly, that is limited to flags. + c.forceURL = true + return nil + }).StringVar(&c.diagURL) c.top.Arg("refresh", "Refresh period").Default("5s").DurationVar(&c.refreshPeriod) } +func (c *Command) newDiagClient(ctx context.Context) (*roundtrip.Client, error) { + if c.forceURL { + // Note that explicitly passing a socket path URI here is not supported. + return roundtrip.NewClient(c.diagURL, "") + } + + clientCfgs := []ClientConfig{ + { + addr: "http://unix", + opts: []roundtrip.ClientParam{roundtrip.HTTPClient(&http.Client{ + // A custom transport is required for unix socket connection. + Transport: &http.Transport{ + DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, "unix", filepath.Join(c.config.DataDir, "debug.sock")) + }}})}, + }, + { + addr: c.diagURL, + }, + } + + return tryNewClient(ctx, clientCfgs...) +} + // TryRun attempts to run subcommands. func (c *Command) TryRun(ctx context.Context, cmd string, _ commonclient.InitFunc) (match bool, err error) { if cmd != c.top.FullCommand() { return false, nil } - diagClient, err := roundtrip.NewClient(c.diagURL, "") + diagClient, err := c.newDiagClient(ctx) if err != nil { return true, trace.Wrap(err) } From 54712f2fe46c07699ab3b835a683f8132ad0433e Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Fri, 18 Jul 2025 09:39:55 +0100 Subject: [PATCH 02/15] refactor: Simplify top address behaviour based on PR feedback --- tool/tctl/common/top/command.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index 0bae6d3d39aa2..861428149b5fe 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -41,23 +41,20 @@ type Command struct { top *kingpin.CmdClause diagURL string refreshPeriod time.Duration - forceURL bool } +const fallbackHTTPAddr = "http://127.0.0.1:3000" + // Initialize sets up the "tctl top" command. func (c *Command) Initialize(app *kingpin.Application, _ *tctlcfg.GlobalCLIFlags, config *servicecfg.Config) { c.config = config c.top = app.Command("top", "Report diagnostic information.") - c.top.Arg("diag-addr", "Diagnostic HTTP URL").Default("http://127.0.0.1:3000").Action(func(*kingpin.ParseContext) error { - // kingpin does not support checking if a positional argument was set explicitly, that is limited to flags. - c.forceURL = true - return nil - }).StringVar(&c.diagURL) + c.top.Arg("diag-addr", "Diagnostic HTTP URL").StringVar(&c.diagURL) c.top.Arg("refresh", "Refresh period").Default("5s").DurationVar(&c.refreshPeriod) } func (c *Command) newDiagClient(ctx context.Context) (*roundtrip.Client, error) { - if c.forceURL { + if c.diagURL != "" { // Note that explicitly passing a socket path URI here is not supported. return roundtrip.NewClient(c.diagURL, "") } @@ -73,7 +70,7 @@ func (c *Command) newDiagClient(ctx context.Context) (*roundtrip.Client, error) }}})}, }, { - addr: c.diagURL, + addr: fallbackHTTPAddr, }, } From fa9c2a8f26528dacdc812a62668b1a9abef55d2d Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Fri, 18 Jul 2025 11:16:18 +0100 Subject: [PATCH 03/15] Refactor top to reuse debug client for metrics. --- lib/client/debug/debug.go | 18 ++++++ tool/tctl/common/top/client/client.go | 49 ++++++++++++++++ .../common/top/{ => client/http}/client.go | 52 +++++++++-------- tool/tctl/common/top/command.go | 57 ++++++++++++------- tool/tctl/common/top/model.go | 10 ++-- tool/tctl/common/top/report.go | 14 +---- 6 files changed, 140 insertions(+), 60 deletions(-) create mode 100644 tool/tctl/common/top/client/client.go rename tool/tctl/common/top/{ => client/http}/client.go (53%) diff --git a/lib/client/debug/debug.go b/lib/client/debug/debug.go index 368a4eadc3ab1..bb017cd23b83c 100644 --- a/lib/client/debug/debug.go +++ b/lib/client/debug/debug.go @@ -26,6 +26,9 @@ import ( "net/url" "strconv" + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" + "github.com/gravitational/trace" apidefaults "github.com/gravitational/teleport/api/defaults" @@ -173,6 +176,21 @@ func (c *Client) GetReadiness(ctx context.Context) (Readiness, error) { return ready, nil } +func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { + resp, err := c.do(ctx, http.MethodGet, url.URL{Path: "/metrics"}, nil) + if err != nil { + return nil, trace.Wrap(err) + } + defer resp.Body.Close() + var parser expfmt.TextParser + metrics, err := parser.TextToMetricFamilies(resp.Body) + if err != nil { + return nil, trace.Wrap(err) + } + + return metrics, nil +} + func (c *Client) do(ctx context.Context, method string, u url.URL, body []byte) (*http.Response, error) { u.Scheme = "http" u.Host = "debug" diff --git a/tool/tctl/common/top/client/client.go b/tool/tctl/common/top/client/client.go new file mode 100644 index 0000000000000..7c951ce494b8a --- /dev/null +++ b/tool/tctl/common/top/client/client.go @@ -0,0 +1,49 @@ +// Teleport +// Copyright (C) 2025 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package client + +import ( + "context" + "net/url" + + dto "github.com/prometheus/client_model/go" + + "github.com/gravitational/teleport/lib/client/debug" + "github.com/gravitational/teleport/tool/tctl/common/top/client/http" + "github.com/gravitational/trace" +) + +type MetricCient interface { + GetMetrics(context.Context) (map[string]*dto.MetricFamily, error) +} + +// Create metrics client based on address +func NewMetricCient(addr string) (MetricCient, error) { + u, err := url.Parse(addr) + if err != nil { + return nil, trace.Wrap(err) + } + + switch u.Scheme { + case "unix": + // For unix, expect: "unix:///var/lib/d.sock" + return debug.NewClient(u.Path), nil + default: + // For anything else try the http client + return http.NewClient(addr) + } +} diff --git a/tool/tctl/common/top/client.go b/tool/tctl/common/top/client/http/client.go similarity index 53% rename from tool/tctl/common/top/client.go rename to tool/tctl/common/top/client/http/client.go index d5d1b6337e612..a44395e3d20ff 100644 --- a/tool/tctl/common/top/client.go +++ b/tool/tctl/common/top/client/http/client.go @@ -14,40 +14,46 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -package top +package http import ( "context" - "errors" "net/url" + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" + "github.com/gravitational/roundtrip" "github.com/gravitational/trace" - - "github.com/gravitational/teleport/lib/httplib" ) -type ClientConfig struct { - addr string - opts []roundtrip.ClientParam +type Client struct { + clt *roundtrip.Client } -func tryNewClient(ctx context.Context, cfgs ...ClientConfig) (*roundtrip.Client, error) { - var errs []error - for _, config := range cfgs { - client, err := roundtrip.NewClient(config.addr, "", config.opts...) - if err != nil { - errs = append(errs, trace.Wrap(err, "failed create client for %v", config.addr)) - continue - } - - if _, err = httplib.ConvertResponse(client.Get(ctx, client.Endpoint("metrics"), url.Values{})); err != nil { - errs = append(errs, trace.Wrap(err, "failed to fetch metrics from addr %v", config.addr)) - continue - } - - return client, nil +func NewClient(addr string) (*Client, error) { + clt, err := roundtrip.NewClient(addr, "") + if err != nil { + return nil, trace.Wrap(err) + } + + return &Client{ + clt: clt, + }, nil +} + +func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { + + re, err := c.clt.Get(ctx, c.clt.Endpoint("metrics"), url.Values{}) + if err != nil { + return nil, trace.Wrap(trace.ConvertSystemError(err)) + } + + var parser expfmt.TextParser + metrics, err := parser.TextToMetricFamilies(re.Reader()) + if err != nil { + return nil, trace.Wrap(err) } - return nil, errors.Join(errs...) + return metrics, nil } diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index 861428149b5fe..7f3ca8783e60f 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -18,19 +18,19 @@ package top import ( "context" - "net" - "net/http" + "errors" + "net/url" "path/filepath" "time" "github.com/alecthomas/kingpin/v2" tea "github.com/charmbracelet/bubbletea" - "github.com/gravitational/roundtrip" "github.com/gravitational/trace" "github.com/gravitational/teleport/lib/service/servicecfg" commonclient "github.com/gravitational/teleport/tool/tctl/common/client" tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" + "github.com/gravitational/teleport/tool/tctl/common/top/client" ) // Command is a debug command that consumes the @@ -53,28 +53,41 @@ func (c *Command) Initialize(app *kingpin.Application, _ *tctlcfg.GlobalCLIFlags c.top.Arg("refresh", "Refresh period").Default("5s").DurationVar(&c.refreshPeriod) } -func (c *Command) newDiagClient(ctx context.Context) (*roundtrip.Client, error) { +func (c *Command) newDiagClient(ctx context.Context) (string, client.MetricCient, error) { if c.diagURL != "" { - // Note that explicitly passing a socket path URI here is not supported. - return roundtrip.NewClient(c.diagURL, "") + clt, err := client.NewMetricCient(c.diagURL) + return c.diagURL, clt, err } - clientCfgs := []ClientConfig{ - { - addr: "http://unix", - opts: []roundtrip.ClientParam{roundtrip.HTTPClient(&http.Client{ - // A custom transport is required for unix socket connection. - Transport: &http.Transport{ - DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { - return (&net.Dialer{}).DialContext(ctx, "unix", filepath.Join(c.config.DataDir, "debug.sock")) - }}})}, - }, - { - addr: fallbackHTTPAddr, - }, + sockURL := url.URL{ + Scheme: "unix", + Path: filepath.Join(c.config.DataDir, "debug.sock"), } - return tryNewClient(ctx, clientCfgs...) + var errs []error + + sockClient, err := client.NewMetricCient(sockURL.String()) + if err != nil { + errs = append(errs, trace.Wrap(err, "failed instanciate local debug client")) + } + if _, err = sockClient.GetMetrics(ctx); err != nil { + errs = append(errs, trace.Wrap(err, "failed connect to local debug service, is Teleport running?")) + } else { + return sockURL.String(), sockClient, nil + } + + fallbackClient, err := client.NewMetricCient(fallbackHTTPAddr) + if err != nil { + errs = append(errs, trace.Wrap(err, "failed instanciate http metric client")) + } + + if _, err = fallbackClient.GetMetrics(ctx); err != nil { + errs = append(errs, trace.Wrap(err, "failed connect to http metric service, restart Teleport with --diag-addr")) + } else { + return fallbackHTTPAddr, fallbackClient, nil + } + + return "", nil, errors.Join(errs...) } // TryRun attempts to run subcommands. @@ -83,13 +96,13 @@ func (c *Command) TryRun(ctx context.Context, cmd string, _ commonclient.InitFun return false, nil } - diagClient, err := c.newDiagClient(ctx) + addr, diagClient, err := c.newDiagClient(ctx) if err != nil { return true, trace.Wrap(err) } p := tea.NewProgram( - newTopModel(c.refreshPeriod, diagClient), + newTopModel(c.refreshPeriod, diagClient, addr), tea.WithAltScreen(), tea.WithContext(ctx), ) diff --git a/tool/tctl/common/top/model.go b/tool/tctl/common/top/model.go index 4a2d06f5162b9..80f2699061eba 100644 --- a/tool/tctl/common/top/model.go +++ b/tool/tctl/common/top/model.go @@ -30,11 +30,11 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" "github.com/dustin/go-humanize" - "github.com/gravitational/roundtrip" "github.com/gravitational/trace" "github.com/guptarohit/asciigraph" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/tool/tctl/common/top/client" ) // topModel is a [tea.Model] implementation which @@ -46,16 +46,18 @@ type topModel struct { selected int help help.Model refreshInterval time.Duration - clt *roundtrip.Client + clt client.MetricCient report *Report reportError error + addr string } -func newTopModel(refreshInterval time.Duration, clt *roundtrip.Client) *topModel { +func newTopModel(refreshInterval time.Duration, clt client.MetricCient, addr string) *topModel { return &topModel{ help: help.New(), clt: clt, refreshInterval: refreshInterval, + addr: addr, } } @@ -171,7 +173,7 @@ func (m *topModel) footerView() string { if m.reportError != nil { if trace.IsConnectionProblem(m.reportError) { - leftContent = fmt.Sprintf("Could not connect to metrics service: %v", m.clt.Endpoint()) + leftContent = fmt.Sprintf("Could not connect to metrics service: %v", m.addr) } else { leftContent = fmt.Sprintf("Failed to generate report: %v", m.reportError) } diff --git a/tool/tctl/common/top/report.go b/tool/tctl/common/top/report.go index ce7ef1f7850c4..9560500d8ec7f 100644 --- a/tool/tctl/common/top/report.go +++ b/tool/tctl/common/top/report.go @@ -22,21 +22,19 @@ import ( "fmt" "iter" "math" - "net/url" "os" "slices" "strings" "time" - "github.com/gravitational/roundtrip" "github.com/gravitational/trace" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" - "github.com/prometheus/common/expfmt" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/utils" + mclient "github.com/gravitational/teleport/tool/tctl/common/top/client" ) // Report is a report rendered over the data @@ -377,14 +375,8 @@ type Bucket struct { UpperBound float64 } -func fetchAndGenerateReport(ctx context.Context, client *roundtrip.Client, prev *Report, period time.Duration) (*Report, error) { - re, err := client.Get(ctx, client.Endpoint("metrics"), url.Values{}) - if err != nil { - return nil, trace.Wrap(trace.ConvertSystemError(err)) - } - - var parser expfmt.TextParser - metrics, err := parser.TextToMetricFamilies(re.Reader()) +func fetchAndGenerateReport(ctx context.Context, client mclient.MetricCient, prev *Report, period time.Duration) (*Report, error) { + metrics, err := client.GetMetrics(ctx) if err != nil { return nil, trace.Wrap(err) } From bc6dc5a9450e597611184344ee39c2853e2ca5c3 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Fri, 18 Jul 2025 11:48:16 +0100 Subject: [PATCH 04/15] fix linting issues --- lib/client/debug/debug.go | 3 +-- tool/tctl/common/top/client/client.go | 2 +- tool/tctl/common/top/command.go | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/client/debug/debug.go b/lib/client/debug/debug.go index bb017cd23b83c..8827695d95fcf 100644 --- a/lib/client/debug/debug.go +++ b/lib/client/debug/debug.go @@ -26,11 +26,10 @@ import ( "net/url" "strconv" + "github.com/gravitational/trace" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" - "github.com/gravitational/trace" - apidefaults "github.com/gravitational/teleport/api/defaults" ) diff --git a/tool/tctl/common/top/client/client.go b/tool/tctl/common/top/client/client.go index 7c951ce494b8a..346b351828d3b 100644 --- a/tool/tctl/common/top/client/client.go +++ b/tool/tctl/common/top/client/client.go @@ -20,11 +20,11 @@ import ( "context" "net/url" + "github.com/gravitational/trace" dto "github.com/prometheus/client_model/go" "github.com/gravitational/teleport/lib/client/debug" "github.com/gravitational/teleport/tool/tctl/common/top/client/http" - "github.com/gravitational/trace" ) type MetricCient interface { diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index 7f3ca8783e60f..d8def04706af4 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -68,7 +68,7 @@ func (c *Command) newDiagClient(ctx context.Context) (string, client.MetricCient sockClient, err := client.NewMetricCient(sockURL.String()) if err != nil { - errs = append(errs, trace.Wrap(err, "failed instanciate local debug client")) + errs = append(errs, trace.Wrap(err, "failed instantiate local debug client")) } if _, err = sockClient.GetMetrics(ctx); err != nil { errs = append(errs, trace.Wrap(err, "failed connect to local debug service, is Teleport running?")) @@ -78,7 +78,7 @@ func (c *Command) newDiagClient(ctx context.Context) (string, client.MetricCient fallbackClient, err := client.NewMetricCient(fallbackHTTPAddr) if err != nil { - errs = append(errs, trace.Wrap(err, "failed instanciate http metric client")) + errs = append(errs, trace.Wrap(err, "failed instantiate http metric client")) } if _, err = fallbackClient.GetMetrics(ctx); err != nil { From 467344a1e8aa3f25a15858513fc2cf9d7c667d63 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Fri, 18 Jul 2025 11:56:34 +0100 Subject: [PATCH 05/15] Update docs to match support for unix sockets --- docs/pages/reference/cli/tctl.mdx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/pages/reference/cli/tctl.mdx b/docs/pages/reference/cli/tctl.mdx index 44ab519bf1796..24648b15ccd2e 100644 --- a/docs/pages/reference/cli/tctl.mdx +++ b/docs/pages/reference/cli/tctl.mdx @@ -1856,15 +1856,17 @@ Reports diagnostic information. $ tctl top [] [] ``` -When a specific endpoint is provided, `tctl top` will always attempt to connect to it, in this case -the diagnostic metrics endpoint must be enabled with `teleport start --diag-addr=`. +When a specific endpoint is provided, `tctl top` will always attempt to connect to it. The endpoint can be a HTTP URL or +a valid local UNIX socket path in a URI format (such as `unix:///var/lib/teleport/debug.sock`). + +In the case of a HTTP endpoint the diagnostic metrics service must be enabled with `teleport start --diag-addr=`. When no endpoint is specified, `tctl top` will attempt to connect via the debug UNIX socket endpoint, falling back to localhost. ### Argument -- `[]` Diagnostic HTTP URL (HTTPS not supported) +- `[]` Diagnostic HTTP URL (HTTPS not supported) or UNIX socket. - `[]` Refresh period e.g. `5s`, `2m`, or `3h` ### Example @@ -1874,6 +1876,8 @@ $ sudo teleport start --diag-addr=127.0.0.1:3000 # View stats with a refresh period of 5 seconds $ tctl top http://127.0.0.1:3000 5s # View stats using the debug service socket +$ tctl top unix:///var/lib/teleport/debug.sock 5s +# Use configured defaults for local node $ tctl top ``` From afb40e46858ea6acd15636f9371128c8882122c5 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Fri, 18 Jul 2025 12:11:13 +0100 Subject: [PATCH 06/15] Fix imports --- tool/tctl/common/top/client/http/client.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tool/tctl/common/top/client/http/client.go b/tool/tctl/common/top/client/http/client.go index a44395e3d20ff..042210fa37359 100644 --- a/tool/tctl/common/top/client/http/client.go +++ b/tool/tctl/common/top/client/http/client.go @@ -20,11 +20,10 @@ import ( "context" "net/url" - dto "github.com/prometheus/client_model/go" - "github.com/prometheus/common/expfmt" - "github.com/gravitational/roundtrip" "github.com/gravitational/trace" + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" ) type Client struct { From 7776890f4ee6f50fa0277de06e73152b5537a893 Mon Sep 17 00:00:00 2001 From: Tim Ross Date: Fri, 18 Jul 2025 10:58:57 -0400 Subject: [PATCH 07/15] refactor debug service --- lib/autoupdate/agent/updater.go | 2 +- lib/client/debug/debug.go | 13 ++++- lib/client/debug/debug_test.go | 2 +- tool/tctl/common/top/client/client.go | 49 ---------------- .../top/client/{http => diag}/client.go | 32 ++++++++--- tool/tctl/common/top/command.go | 57 ++++++++----------- tool/tctl/common/top/model.go | 5 +- tool/tctl/common/top/report.go | 3 +- tool/teleport/common/debug.go | 22 ++++--- 9 files changed, 75 insertions(+), 110 deletions(-) delete mode 100644 tool/tctl/common/top/client/client.go rename tool/tctl/common/top/client/{http => diag}/client.go (68%) diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 3f4e967e6d6f2..9a2d3696c118a 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -125,7 +125,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig, ns *Namespace) (*Updater, error) { cfg.SystemDir = packageSystemDir } validator := Validator{Log: cfg.Log} - debugClient := debug.NewClient(filepath.Join(ns.dataDir, debugSocketFileName)) + debugClient := debug.NewClient(ns.dataDir) return &Updater{ Log: cfg.Log, Pool: certPool, diff --git a/lib/client/debug/debug.go b/lib/client/debug/debug.go index 8827695d95fcf..6ad702cbadf6d 100644 --- a/lib/client/debug/debug.go +++ b/lib/client/debug/debug.go @@ -24,12 +24,14 @@ import ( "net" "net/http" "net/url" + "path/filepath" "strconv" "github.com/gravitational/trace" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" + "github.com/gravitational/teleport" apidefaults "github.com/gravitational/teleport/api/defaults" ) @@ -49,11 +51,13 @@ var SupportedProfiles = map[string]struct{}{ // Client represents the debug service client. type Client struct { - clt *http.Client + clt *http.Client + socketPath string } // NewClient generates a new debug service client. -func NewClient(socketPath string) *Client { +func NewClient(dataDir string) *Client { + socketPath := filepath.Join(dataDir, teleport.DebugServiceSocketName) return &Client{ clt: &http.Client{ Timeout: apidefaults.DefaultIOTimeout, @@ -68,9 +72,14 @@ func NewClient(socketPath string) *Client { return trace.Errorf("redirect via socket not allowed") }, }, + socketPath: socketPath, } } +func (c *Client) SocketPath() string { + return c.socketPath +} + // SetLogLevel changes the application's log level and a change status message. func (c *Client) SetLogLevel(ctx context.Context, level string) (string, error) { resp, err := c.do(ctx, http.MethodPut, url.URL{Path: "/log-level"}, []byte(level)) diff --git a/lib/client/debug/debug_test.go b/lib/client/debug/debug_test.go index 52ced041ca0d6..c82b945a57fe9 100644 --- a/lib/client/debug/debug_test.go +++ b/lib/client/debug/debug_test.go @@ -206,7 +206,7 @@ func newSocketMockService(t *testing.T, status int, contents []byte) (string, fu }() t.Cleanup(func() { srv.Shutdown(context.Background()) }) - return socketPath, func() []string { + return socketDir, func() []string { srv.Shutdown(context.Background()) return requests } diff --git a/tool/tctl/common/top/client/client.go b/tool/tctl/common/top/client/client.go deleted file mode 100644 index 346b351828d3b..0000000000000 --- a/tool/tctl/common/top/client/client.go +++ /dev/null @@ -1,49 +0,0 @@ -// Teleport -// Copyright (C) 2025 Gravitational, Inc. -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package client - -import ( - "context" - "net/url" - - "github.com/gravitational/trace" - dto "github.com/prometheus/client_model/go" - - "github.com/gravitational/teleport/lib/client/debug" - "github.com/gravitational/teleport/tool/tctl/common/top/client/http" -) - -type MetricCient interface { - GetMetrics(context.Context) (map[string]*dto.MetricFamily, error) -} - -// Create metrics client based on address -func NewMetricCient(addr string) (MetricCient, error) { - u, err := url.Parse(addr) - if err != nil { - return nil, trace.Wrap(err) - } - - switch u.Scheme { - case "unix": - // For unix, expect: "unix:///var/lib/d.sock" - return debug.NewClient(u.Path), nil - default: - // For anything else try the http client - return http.NewClient(addr) - } -} diff --git a/tool/tctl/common/top/client/http/client.go b/tool/tctl/common/top/client/diag/client.go similarity index 68% rename from tool/tctl/common/top/client/http/client.go rename to tool/tctl/common/top/client/diag/client.go index 042210fa37359..53ee575b9700d 100644 --- a/tool/tctl/common/top/client/http/client.go +++ b/tool/tctl/common/top/client/diag/client.go @@ -14,42 +14,58 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -package http +package diag import ( "context" + "net/http" "net/url" - "github.com/gravitational/roundtrip" "github.com/gravitational/trace" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" + + "github.com/gravitational/teleport/lib/defaults" ) type Client struct { - clt *roundtrip.Client + endpoint string + clt *http.Client } func NewClient(addr string) (*Client, error) { - clt, err := roundtrip.NewClient(addr, "") + clt, err := defaults.HTTPClient() + if err != nil { + return nil, trace.Wrap(err) + } + + u, err := url.Parse(addr) if err != nil { return nil, trace.Wrap(err) } + u.Path = "metrics" + return &Client{ - clt: clt, + endpoint: u.String(), + clt: clt, }, nil } func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint, http.NoBody) + if err != nil { + return nil, trace.Wrap(err) + } - re, err := c.clt.Get(ctx, c.clt.Endpoint("metrics"), url.Values{}) + resp, err := c.clt.Do(req) if err != nil { - return nil, trace.Wrap(trace.ConvertSystemError(err)) + return nil, trace.Wrap(err) } + defer resp.Body.Close() var parser expfmt.TextParser - metrics, err := parser.TextToMetricFamilies(re.Reader()) + metrics, err := parser.TextToMetricFamilies(resp.Body) if err != nil { return nil, trace.Wrap(err) } diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index d8def04706af4..9ae4229e294d6 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -18,19 +18,18 @@ package top import ( "context" - "errors" - "net/url" - "path/filepath" "time" "github.com/alecthomas/kingpin/v2" tea "github.com/charmbracelet/bubbletea" "github.com/gravitational/trace" + dto "github.com/prometheus/client_model/go" + "github.com/gravitational/teleport/lib/client/debug" "github.com/gravitational/teleport/lib/service/servicecfg" commonclient "github.com/gravitational/teleport/tool/tctl/common/client" tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" - "github.com/gravitational/teleport/tool/tctl/common/top/client" + "github.com/gravitational/teleport/tool/tctl/common/top/client/diag" ) // Command is a debug command that consumes the @@ -43,7 +42,7 @@ type Command struct { refreshPeriod time.Duration } -const fallbackHTTPAddr = "http://127.0.0.1:3000" +const defaultDiagAddr = "http://127.0.0.1:3000" // Initialize sets up the "tctl top" command. func (c *Command) Initialize(app *kingpin.Application, _ *tctlcfg.GlobalCLIFlags, config *servicecfg.Config) { @@ -53,41 +52,35 @@ func (c *Command) Initialize(app *kingpin.Application, _ *tctlcfg.GlobalCLIFlags c.top.Arg("refresh", "Refresh period").Default("5s").DurationVar(&c.refreshPeriod) } -func (c *Command) newDiagClient(ctx context.Context) (string, client.MetricCient, error) { - if c.diagURL != "" { - clt, err := client.NewMetricCient(c.diagURL) - return c.diagURL, clt, err - } +type MetricsClient interface { + GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) +} - sockURL := url.URL{ - Scheme: "unix", - Path: filepath.Join(c.config.DataDir, "debug.sock"), +func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, error) { + if c.diagURL != "" { + clt, err := diag.NewClient(c.diagURL) + return c.diagURL, clt, trace.Wrap(err) } - var errs []error - - sockClient, err := client.NewMetricCient(sockURL.String()) - if err != nil { - errs = append(errs, trace.Wrap(err, "failed instantiate local debug client")) - } - if _, err = sockClient.GetMetrics(ctx); err != nil { - errs = append(errs, trace.Wrap(err, "failed connect to local debug service, is Teleport running?")) - } else { - return sockURL.String(), sockClient, nil + debugClient := debug.NewClient(c.config.DataDir) + _, debugErr := debugClient.GetMetrics(ctx) + if debugErr == nil { + return c.config.DataDir, debugClient, nil } - fallbackClient, err := client.NewMetricCient(fallbackHTTPAddr) + diagClient, err := diag.NewClient(defaultDiagAddr) if err != nil { - errs = append(errs, trace.Wrap(err, "failed instantiate http metric client")) + // replace with better error below + return "", nil, trace.NewAggregate(trace.Wrap(err, "creating diag addr client"), trace.Wrap(debugErr, "creating debug client")) } - if _, err = fallbackClient.GetMetrics(ctx); err != nil { - errs = append(errs, trace.Wrap(err, "failed connect to http metric service, restart Teleport with --diag-addr")) - } else { - return fallbackHTTPAddr, fallbackClient, nil + _, err = diagClient.GetMetrics(ctx) + if err == nil { + return defaultDiagAddr, diagClient, nil } - return "", nil, errors.Join(errs...) + // replace with better error below + return "", nil, trace.NewAggregate(trace.Wrap(err, "creating diag addr client"), trace.Wrap(debugErr, "creating debug client")) } // TryRun attempts to run subcommands. @@ -96,13 +89,13 @@ func (c *Command) TryRun(ctx context.Context, cmd string, _ commonclient.InitFun return false, nil } - addr, diagClient, err := c.newDiagClient(ctx) + addr, metricsClient, err := c.newMetricsClient(ctx) if err != nil { return true, trace.Wrap(err) } p := tea.NewProgram( - newTopModel(c.refreshPeriod, diagClient, addr), + newTopModel(c.refreshPeriod, metricsClient, addr), tea.WithAltScreen(), tea.WithContext(ctx), ) diff --git a/tool/tctl/common/top/model.go b/tool/tctl/common/top/model.go index 80f2699061eba..7c1ea1ad8139e 100644 --- a/tool/tctl/common/top/model.go +++ b/tool/tctl/common/top/model.go @@ -34,7 +34,6 @@ import ( "github.com/guptarohit/asciigraph" "github.com/gravitational/teleport/api/constants" - "github.com/gravitational/teleport/tool/tctl/common/top/client" ) // topModel is a [tea.Model] implementation which @@ -46,13 +45,13 @@ type topModel struct { selected int help help.Model refreshInterval time.Duration - clt client.MetricCient + clt MetricsClient report *Report reportError error addr string } -func newTopModel(refreshInterval time.Duration, clt client.MetricCient, addr string) *topModel { +func newTopModel(refreshInterval time.Duration, clt MetricsClient, addr string) *topModel { return &topModel{ help: help.New(), clt: clt, diff --git a/tool/tctl/common/top/report.go b/tool/tctl/common/top/report.go index 9560500d8ec7f..f9af7a604de32 100644 --- a/tool/tctl/common/top/report.go +++ b/tool/tctl/common/top/report.go @@ -34,7 +34,6 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/utils" - mclient "github.com/gravitational/teleport/tool/tctl/common/top/client" ) // Report is a report rendered over the data @@ -375,7 +374,7 @@ type Bucket struct { UpperBound float64 } -func fetchAndGenerateReport(ctx context.Context, client mclient.MetricCient, prev *Report, period time.Duration) (*Report, error) { +func fetchAndGenerateReport(ctx context.Context, client MetricsClient, prev *Report, period time.Duration) (*Report, error) { metrics, err := client.GetMetrics(ctx) if err != nil { return nil, trace.Wrap(err) diff --git a/tool/teleport/common/debug.go b/tool/teleport/common/debug.go index c993b78742f6a..2d1d8d59a11e9 100644 --- a/tool/teleport/common/debug.go +++ b/tool/teleport/common/debug.go @@ -24,14 +24,12 @@ import ( "errors" "fmt" "io" - "path/filepath" "slices" "strings" "time" "github.com/gravitational/trace" - "github.com/gravitational/teleport" debugclient "github.com/gravitational/teleport/lib/client/debug" "github.com/gravitational/teleport/lib/config" "github.com/gravitational/teleport/lib/defaults" @@ -46,18 +44,19 @@ type DebugClient interface { GetLogLevel(context.Context) (string, error) // CollectProfile collects a pprof profile. CollectProfile(context.Context, string, int) ([]byte, error) + SocketPath() string } func onSetLogLevel(configPath string, level string) error { ctx := context.Background() - clt, dataDir, socketPath, err := newDebugClient(configPath) + clt, dataDir, err := newDebugClient(configPath) if err != nil { return trace.Wrap(err) } setMessage, err := setLogLevel(ctx, clt, level) if err != nil { - return convertToReadableErr(err, dataDir, socketPath) + return convertToReadableErr(err, dataDir, clt.SocketPath()) } fmt.Println(setMessage) @@ -74,14 +73,14 @@ func setLogLevel(ctx context.Context, clt DebugClient, level string) (string, er func onGetLogLevel(configPath string) error { ctx := context.Background() - clt, dataDir, socketPath, err := newDebugClient(configPath) + clt, dataDir, err := newDebugClient(configPath) if err != nil { return trace.Wrap(err) } currentLogLevel, err := getLogLevel(ctx, clt) if err != nil { - return convertToReadableErr(err, dataDir, socketPath) + return convertToReadableErr(err, dataDir, clt.SocketPath()) } fmt.Printf("Current log level %q\n", currentLogLevel) @@ -111,14 +110,14 @@ func onCollectProfiles(configPath string, rawProfiles string, seconds int) error ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - clt, dataDir, socketPath, err := newDebugClient(configPath) + clt, dataDir, err := newDebugClient(configPath) if err != nil { return trace.Wrap(err) } var output bytes.Buffer if err := collectProfiles(ctx, clt, &output, rawProfiles, seconds); err != nil { - return convertToReadableErr(err, dataDir, socketPath) + return convertToReadableErr(err, dataDir, clt.SocketPath()) } fmt.Print(output.String()) @@ -169,10 +168,10 @@ func collectProfiles(ctx context.Context, clt DebugClient, buf io.Writer, rawPro // newDebugClient initializes the debug client based on the Teleport // configuration. It also returns the data dir and socket path used. -func newDebugClient(configPath string) (DebugClient, string, string, error) { +func newDebugClient(configPath string) (DebugClient, string, error) { cfg, err := config.ReadConfigFile(configPath) if err != nil { - return nil, "", "", trace.Wrap(err) + return nil, "", trace.Wrap(err) } // ReadConfigFile returns nil configuration if the file doesn't exists. @@ -183,8 +182,7 @@ func newDebugClient(configPath string) (DebugClient, string, string, error) { dataDir = cfg.DataDir } - socketPath := filepath.Join(dataDir, teleport.DebugServiceSocketName) - return debugclient.NewClient(socketPath), dataDir, socketPath, nil + return debugclient.NewClient(dataDir), dataDir, nil } // convertToReadableErr converts debug service client error into a more friendly From 8b5fd15f6a60bb93140e801e6547f67d453b5b65 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 11:16:47 +0100 Subject: [PATCH 08/15] clean up error handling in top command --- tool/tctl/common/top/command.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index 9ae4229e294d6..287c244b8ed68 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -62,25 +62,33 @@ func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, return c.diagURL, clt, trace.Wrap(err) } + // Try the local UNIX debug service client first. debugClient := debug.NewClient(c.config.DataDir) _, debugErr := debugClient.GetMetrics(ctx) if debugErr == nil { return c.config.DataDir, debugClient, nil } - - diagClient, err := diag.NewClient(defaultDiagAddr) - if err != nil { - // replace with better error below - return "", nil, trace.NewAggregate(trace.Wrap(err, "creating diag addr client"), trace.Wrap(debugErr, "creating debug client")) + debugErr = trace.Wrap(debugErr, "failed to get metrics from debug service") + + // Try default diagnostic address + diagClient, defErr := diag.NewClient(defaultDiagAddr) + if defErr != nil { + return "", nil, trace.NewAggregate( + trace.Errorf("unable to connect to Teleport service"), + trace.Wrap(defErr, "failed to create diagnostics client for default address %q", defaultDiagAddr), + debugErr) } - _, err = diagClient.GetMetrics(ctx) - if err == nil { + _, defErr = diagClient.GetMetrics(ctx) + if defErr == nil { return defaultDiagAddr, diagClient, nil } - // replace with better error below - return "", nil, trace.NewAggregate(trace.Wrap(err, "creating diag addr client"), trace.Wrap(debugErr, "creating debug client")) + return "", nil, trace.NewAggregate( + trace.Errorf("unable to connect to Teleport service"), + trace.Wrap(defErr, "failed to get metrics from diagnostics client at default address %q", defaultDiagAddr), + debugErr, + ) } // TryRun attempts to run subcommands. From b0cfc0eed0c1698b2fda565c95295cc3118b09c7 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 11:22:14 +0100 Subject: [PATCH 09/15] Add footer display for addr --- tool/tctl/common/top/command.go | 2 +- tool/tctl/common/top/model.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index 287c244b8ed68..16c6eb795208f 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -66,7 +66,7 @@ func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, debugClient := debug.NewClient(c.config.DataDir) _, debugErr := debugClient.GetMetrics(ctx) if debugErr == nil { - return c.config.DataDir, debugClient, nil + return debugClient.SocketPath(), debugClient, nil } debugErr = trace.Wrap(debugErr, "failed to get metrics from debug service") diff --git a/tool/tctl/common/top/model.go b/tool/tctl/common/top/model.go index 7c1ea1ad8139e..f6a443e94e5a0 100644 --- a/tool/tctl/common/top/model.go +++ b/tool/tctl/common/top/model.go @@ -178,9 +178,10 @@ func (m *topModel) footerView() string { } } if leftContent == "" && m.report != nil { - leftContent = fmt.Sprintf("Report generated at %s for host %s", + leftContent = fmt.Sprintf("Report generated at %s for host %s (%s)", m.report.Timestamp.Format(constants.HumanDateFormatSeconds), m.report.Hostname, + m.addr, ) } left := lipgloss.NewStyle(). From c5fcaed90a26a6cfb75579f645f13e91a0ada8d8 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 11:26:29 +0100 Subject: [PATCH 10/15] Docs for diag/client --- tool/tctl/common/top/client/diag/client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tool/tctl/common/top/client/diag/client.go b/tool/tctl/common/top/client/diag/client.go index 53ee575b9700d..7f1dbae0e0911 100644 --- a/tool/tctl/common/top/client/diag/client.go +++ b/tool/tctl/common/top/client/diag/client.go @@ -28,11 +28,14 @@ import ( "github.com/gravitational/teleport/lib/defaults" ) +// Client is a wrapper around [*http.Client] that provides +// helpers for fetching metrics from the diagnostic endpoint of Teleport. type Client struct { endpoint string clt *http.Client } +// Creates a new Client for a given address. func NewClient(addr string) (*Client, error) { clt, err := defaults.HTTPClient() if err != nil { @@ -52,6 +55,7 @@ func NewClient(addr string) (*Client, error) { }, nil } +// Fetches metrics from the configured endpoint and returns them as a map keyed by metric name. func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint, http.NoBody) if err != nil { From df22bf7224945542d922e133076831a988fdcfc2 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 13:29:59 +0100 Subject: [PATCH 11/15] update docs and add addr parser tests --- docs/pages/reference/cli/tctl.mdx | 11 +-- tool/tctl/common/top/client/diag/client.go | 30 ++++++- .../common/top/client/diag/client_test.go | 78 +++++++++++++++++++ 3 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 tool/tctl/common/top/client/diag/client_test.go diff --git a/docs/pages/reference/cli/tctl.mdx b/docs/pages/reference/cli/tctl.mdx index 24648b15ccd2e..22933caf8ccad 100644 --- a/docs/pages/reference/cli/tctl.mdx +++ b/docs/pages/reference/cli/tctl.mdx @@ -1850,16 +1850,15 @@ $ tctl tokens rm [] Reports diagnostic information. -`tctl top` can consume metrics from a HTTP diagnostic endpoint or a local UNIX socket. +`tctl top` can consume metrics from a HTTP diagnostic endpoint. ```code $ tctl top [] [] ``` -When a specific endpoint is provided, `tctl top` will always attempt to connect to it. The endpoint can be a HTTP URL or -a valid local UNIX socket path in a URI format (such as `unix:///var/lib/teleport/debug.sock`). - -In the case of a HTTP endpoint the diagnostic metrics service must be enabled with `teleport start --diag-addr=`. +When a specific endpoint is provided, `tctl top` will always attempt to connect to it. +The endpoint should be a valid HTTP URL, corresponding to a matching +diagnostic metrics service configuration such as `teleport start --diag-addr=`. When no endpoint is specified, `tctl top` will attempt to connect via the debug UNIX socket endpoint, falling back to localhost. @@ -1875,8 +1874,6 @@ localhost. $ sudo teleport start --diag-addr=127.0.0.1:3000 # View stats with a refresh period of 5 seconds $ tctl top http://127.0.0.1:3000 5s -# View stats using the debug service socket -$ tctl top unix:///var/lib/teleport/debug.sock 5s # Use configured defaults for local node $ tctl top ``` diff --git a/tool/tctl/common/top/client/diag/client.go b/tool/tctl/common/top/client/diag/client.go index 7f1dbae0e0911..5fb22e1a65c9a 100644 --- a/tool/tctl/common/top/client/diag/client.go +++ b/tool/tctl/common/top/client/diag/client.go @@ -18,6 +18,7 @@ package diag import ( "context" + "net" "net/http" "net/url" @@ -35,6 +36,27 @@ type Client struct { clt *http.Client } +// Takes a string address and attempts to parse it into a valid URL. +// The input can either be a valid string URL or a :. +func parseAddress(addr string) (*url.URL, error) { + u, err := url.Parse(addr) + + if err != nil || u.Scheme == "" || u.Host == "" { + // It's possible the address is hostport format + _, _, err = net.SplitHostPort(addr) + if err != nil { + return nil, trace.Errorf("address %s is neither a valid URL nor :", addr) + } + + u = &url.URL{ + Scheme: "http", + Host: addr, + } + } + + return u, nil +} + // Creates a new Client for a given address. func NewClient(addr string) (*Client, error) { clt, err := defaults.HTTPClient() @@ -42,15 +64,17 @@ func NewClient(addr string) (*Client, error) { return nil, trace.Wrap(err) } - u, err := url.Parse(addr) + u, err := parseAddress(addr) if err != nil { return nil, trace.Wrap(err) } - u.Path = "metrics" + if u.Scheme != "http" { + return nil, trace.Errorf("unsupported scheme: %s, please provide a http address", u.Scheme) + } return &Client{ - endpoint: u.String(), + endpoint: u.JoinPath("metrics").String(), clt: clt, }, nil } diff --git a/tool/tctl/common/top/client/diag/client_test.go b/tool/tctl/common/top/client/diag/client_test.go new file mode 100644 index 0000000000000..bd80f7cd32b8a --- /dev/null +++ b/tool/tctl/common/top/client/diag/client_test.go @@ -0,0 +1,78 @@ +/* + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package diag + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestClientAddressParsing(t *testing.T) { + t.Parallel() + + testCases := []struct { + addr string + url *url.URL + }{ + { + addr: "http://127.0.0.1:3000", + url: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:3000", + }, + }, + { + addr: "http://localhost:3000", + url: &url.URL{ + Scheme: "http", + Host: "localhost:3000", + }, + }, + { + addr: "localhost:3000", + url: &url.URL{ + Scheme: "http", + Host: "localhost:3000", + }, + }, + { + addr: "127.0.0.1:3000", + url: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:3000", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.addr, func(t *testing.T) { + u, err := parseAddress(tc.addr) + if tc.url == nil { + require.Error(t, err) + require.Nil(t, u) + } else { + require.NoError(t, err) + require.Equal(t, tc.url.Host, u.Host) + require.Equal(t, tc.url.Scheme, u.Scheme) + } + }) + } +} From dd62fff1e38b1aa145b0d1395243d3cecb1f14de Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 13:34:31 +0100 Subject: [PATCH 12/15] More docs fixes --- docs/pages/reference/cli/tctl.mdx | 5 ++--- lib/client/debug/debug.go | 1 + tool/tctl/common/top/client/diag/client.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/pages/reference/cli/tctl.mdx b/docs/pages/reference/cli/tctl.mdx index 22933caf8ccad..2f886b538c9ee 100644 --- a/docs/pages/reference/cli/tctl.mdx +++ b/docs/pages/reference/cli/tctl.mdx @@ -1860,12 +1860,11 @@ When a specific endpoint is provided, `tctl top` will always attempt to connect The endpoint should be a valid HTTP URL, corresponding to a matching diagnostic metrics service configuration such as `teleport start --diag-addr=`. -When no endpoint is specified, `tctl top` will attempt to connect via the debug UNIX socket endpoint, falling back to -localhost. +When no endpoint is specified, `tctl top` will attempt to connect via the debug UNIX socket endpoint, falling back to localhost. ### Argument -- `[]` Diagnostic HTTP URL (HTTPS not supported) or UNIX socket. +- `[]` Diagnostic HTTP URL (HTTPS not supported) - `[]` Refresh period e.g. `5s`, `2m`, or `3h` ### Example diff --git a/lib/client/debug/debug.go b/lib/client/debug/debug.go index 6ad702cbadf6d..b6fd984430d85 100644 --- a/lib/client/debug/debug.go +++ b/lib/client/debug/debug.go @@ -184,6 +184,7 @@ func (c *Client) GetReadiness(ctx context.Context) (Readiness, error) { return ready, nil } +// GetMetrics returns metrics as a map keyed by metric name. func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { resp, err := c.do(ctx, http.MethodGet, url.URL{Path: "/metrics"}, nil) if err != nil { diff --git a/tool/tctl/common/top/client/diag/client.go b/tool/tctl/common/top/client/diag/client.go index 5fb22e1a65c9a..922a7c4c4917c 100644 --- a/tool/tctl/common/top/client/diag/client.go +++ b/tool/tctl/common/top/client/diag/client.go @@ -37,12 +37,12 @@ type Client struct { } // Takes a string address and attempts to parse it into a valid URL. -// The input can either be a valid string URL or a :. +// The input can either be a valid string URL or a : pair. func parseAddress(addr string) (*url.URL, error) { u, err := url.Parse(addr) if err != nil || u.Scheme == "" || u.Host == "" { - // It's possible the address is hostport format + // Attempt to parse the input as a host:port tuple instead. _, _, err = net.SplitHostPort(addr) if err != nil { return nil, trace.Errorf("address %s is neither a valid URL nor :", addr) @@ -79,7 +79,7 @@ func NewClient(addr string) (*Client, error) { }, nil } -// Fetches metrics from the configured endpoint and returns them as a map keyed by metric name. +// GetMetrics returns metrics as a map keyed by metric name. func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint, http.NoBody) if err != nil { From 2d0aca7fc24d4701caf410831c608dae6717098a Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 13:59:43 +0100 Subject: [PATCH 13/15] Fix linting after rebase --- lib/autoupdate/agent/updater.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 9a2d3696c118a..c5f639c77d761 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -67,8 +67,6 @@ const ( // TODO(sclevine): This value is arbitrary and could be replaced by, e.g., min(1%, 200mb) in the future // to account for a range of disk sizes. reservedFreeDisk = 10_000_000 - // debugSocketFileName is the name of Teleport's debug socket in the data dir. - debugSocketFileName = "debug.sock" // 10 MB // requiredUmask must be set before this package can be used. // Use syscall.Umask to set when no other goroutines are running. requiredUmask = 0o022 From 293b27b1a5f8b55850694a734e98b18ae5a27098 Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Mon, 21 Jul 2025 17:04:47 +0100 Subject: [PATCH 14/15] Better godocs, more URL test cases, fixed user facing errors --- docs/pages/reference/cli/tctl.mdx | 2 +- lib/autoupdate/agent/updater.go | 2 +- lib/client/debug/debug.go | 3 ++- tool/tctl/common/top/client/diag/client.go | 6 +++--- tool/tctl/common/top/client/diag/client_test.go | 9 +++++++++ tool/tctl/common/top/command.go | 6 +++--- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/docs/pages/reference/cli/tctl.mdx b/docs/pages/reference/cli/tctl.mdx index 2f886b538c9ee..f8588ab351a81 100644 --- a/docs/pages/reference/cli/tctl.mdx +++ b/docs/pages/reference/cli/tctl.mdx @@ -1873,7 +1873,7 @@ When no endpoint is specified, `tctl top` will attempt to connect via the debug $ sudo teleport start --diag-addr=127.0.0.1:3000 # View stats with a refresh period of 5 seconds $ tctl top http://127.0.0.1:3000 5s -# Use configured defaults for local node +# Use configured defaults $ tctl top ``` diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index c5f639c77d761..cb609fa7136d0 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -66,7 +66,7 @@ const ( // reservedFreeDisk is the minimum required free space left on disk during downloads. // TODO(sclevine): This value is arbitrary and could be replaced by, e.g., min(1%, 200mb) in the future // to account for a range of disk sizes. - reservedFreeDisk = 10_000_000 + reservedFreeDisk = 10_000_000 // 10 MB // requiredUmask must be set before this package can be used. // Use syscall.Umask to set when no other goroutines are running. requiredUmask = 0o022 diff --git a/lib/client/debug/debug.go b/lib/client/debug/debug.go index b6fd984430d85..a8b51a5fb2ef8 100644 --- a/lib/client/debug/debug.go +++ b/lib/client/debug/debug.go @@ -76,6 +76,7 @@ func NewClient(dataDir string) *Client { } } +// SocketPath returns the absolute path to the UNIX socket that the debug service is exposed on. func (c *Client) SocketPath() string { return c.socketPath } @@ -184,7 +185,7 @@ func (c *Client) GetReadiness(ctx context.Context) (Readiness, error) { return ready, nil } -// GetMetrics returns metrics as a map keyed by metric name. +// GetMetrics returns prometheus metrics as a map keyed by metric name. func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { resp, err := c.do(ctx, http.MethodGet, url.URL{Path: "/metrics"}, nil) if err != nil { diff --git a/tool/tctl/common/top/client/diag/client.go b/tool/tctl/common/top/client/diag/client.go index 922a7c4c4917c..feea1e50fffc4 100644 --- a/tool/tctl/common/top/client/diag/client.go +++ b/tool/tctl/common/top/client/diag/client.go @@ -36,7 +36,7 @@ type Client struct { clt *http.Client } -// Takes a string address and attempts to parse it into a valid URL. +// parseAddress takes a string address and attempts to parse it into a valid URL. // The input can either be a valid string URL or a : pair. func parseAddress(addr string) (*url.URL, error) { u, err := url.Parse(addr) @@ -57,7 +57,7 @@ func parseAddress(addr string) (*url.URL, error) { return u, nil } -// Creates a new Client for a given address. +// NewClient creates a new Client for a given address. func NewClient(addr string) (*Client, error) { clt, err := defaults.HTTPClient() if err != nil { @@ -79,7 +79,7 @@ func NewClient(addr string) (*Client, error) { }, nil } -// GetMetrics returns metrics as a map keyed by metric name. +// GetMetrics returns prometheus metrics as a map keyed by metric name. func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint, http.NoBody) if err != nil { diff --git a/tool/tctl/common/top/client/diag/client_test.go b/tool/tctl/common/top/client/diag/client_test.go index bd80f7cd32b8a..047e53b1c9f2e 100644 --- a/tool/tctl/common/top/client/diag/client_test.go +++ b/tool/tctl/common/top/client/diag/client_test.go @@ -60,6 +60,15 @@ func TestClientAddressParsing(t *testing.T) { Host: "127.0.0.1:3000", }, }, + { + addr: "badurl:300:9:1", + }, + { + addr: "http//badurl", + }, + { + addr: "/var/lib/file.sock", + }, } for _, tc := range testCases { diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index 16c6eb795208f..c63374cde17c5 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -68,14 +68,14 @@ func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, if debugErr == nil { return debugClient.SocketPath(), debugClient, nil } - debugErr = trace.Wrap(debugErr, "failed to get metrics from debug service") + debugErr = trace.Wrap(debugErr, "retrieving metrics from debug service") // Try default diagnostic address diagClient, defErr := diag.NewClient(defaultDiagAddr) if defErr != nil { return "", nil, trace.NewAggregate( trace.Errorf("unable to connect to Teleport service"), - trace.Wrap(defErr, "failed to create diagnostics client for default address %q", defaultDiagAddr), + trace.Wrap(defErr, "creating diagnostics client for default address %q", defaultDiagAddr), debugErr) } @@ -86,7 +86,7 @@ func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, return "", nil, trace.NewAggregate( trace.Errorf("unable to connect to Teleport service"), - trace.Wrap(defErr, "failed to get metrics from diagnostics client at default address %q", defaultDiagAddr), + trace.Wrap(defErr, "getting metrics from diagnostics client at default address %q", defaultDiagAddr), debugErr, ) } From 2f9806cc21501fc4f8a8f39775c13c083a6be7af Mon Sep 17 00:00:00 2001 From: Lukasz Okraszewski Date: Tue, 22 Jul 2025 11:55:46 +0100 Subject: [PATCH 15/15] Fix err trace wrapping semantics --- tool/tctl/common/top/command.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tool/tctl/common/top/command.go b/tool/tctl/common/top/command.go index c63374cde17c5..14aeaf20597ea 100644 --- a/tool/tctl/common/top/command.go +++ b/tool/tctl/common/top/command.go @@ -73,10 +73,11 @@ func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, // Try default diagnostic address diagClient, defErr := diag.NewClient(defaultDiagAddr) if defErr != nil { - return "", nil, trace.NewAggregate( - trace.Errorf("unable to connect to Teleport service"), - trace.Wrap(defErr, "creating diagnostics client for default address %q", defaultDiagAddr), - debugErr) + return "", nil, trace.Wrap( + trace.NewAggregate( + trace.Wrap(defErr, "creating diagnostics client for default address %q", defaultDiagAddr), + debugErr), + "unable to connect to Teleport metrics server") } _, defErr = diagClient.GetMetrics(ctx) @@ -84,11 +85,12 @@ func (c *Command) newMetricsClient(ctx context.Context) (string, MetricsClient, return defaultDiagAddr, diagClient, nil } - return "", nil, trace.NewAggregate( - trace.Errorf("unable to connect to Teleport service"), - trace.Wrap(defErr, "getting metrics from diagnostics client at default address %q", defaultDiagAddr), - debugErr, - ) + return "", nil, trace.Wrap( + trace.NewAggregate( + trace.Wrap(defErr, "getting metrics from diagnostics client at default address %q", defaultDiagAddr), + debugErr, + ), + "connecting to Teleport metrics server") } // TryRun attempts to run subcommands.