From f3df6cbc42e66460da6b8037e737cecca4c2024e Mon Sep 17 00:00:00 2001 From: Evan Freed <2314084+evanfreed@users.noreply.github.com> Date: Fri, 11 Jul 2025 09:41:51 -0500 Subject: [PATCH] Pyroscope client timeout and default upload rate (#56584) * add pyroscope timeout and default upload rate Signed-off-by: Evan Freed * fix logging Signed-off-by: Evan Freed * fix lint Signed-off-by: Evan Freed * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Update lib/service/pyroscope.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * review updates Signed-off-by: Evan Freed * fmt Signed-off-by: Evan Freed --------- Signed-off-by: Evan Freed Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 +-- integrations/terraform-mwi/go.mod | 2 +- integrations/terraform-mwi/go.sum | 4 +-- integrations/terraform/go.mod | 2 +- integrations/terraform/go.sum | 4 +-- lib/service/pyroscope.go | 59 +++++++++++++++++++++++++------ 7 files changed, 58 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index 55b3a7271b6..304b21eb7d8 100644 --- a/go.mod +++ b/go.mod @@ -139,7 +139,7 @@ require ( github.com/google/uuid v1.6.0 github.com/googleapis/gax-go/v2 v2.14.2 github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 - github.com/grafana/pyroscope-go v1.2.2 + github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693 github.com/gravitational/license v0.0.0-20250329001817-070456fa8ec1 github.com/gravitational/roundtrip v1.0.2 github.com/gravitational/teleport/api v0.0.0 diff --git a/go.sum b/go.sum index e11102f6fbb..6bfb788fc84 100644 --- a/go.sum +++ b/go.sum @@ -1533,8 +1533,8 @@ github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5T github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= github.com/gosuri/uitable v0.0.4 h1:IG2xLKRvErL3uhY6e1BylFzG+aJiwQviDDTfOKeKTpY= github.com/gosuri/uitable v0.0.4/go.mod h1:tKR86bXuXPZazfOTG1FIzvjIdXzd0mo4Vtn16vt0PJo= -github.com/grafana/pyroscope-go v1.2.2 h1:uvKCyZMD724RkaCEMrSTC38Yn7AnFe8S2wiAIYdDPCE= -github.com/grafana/pyroscope-go v1.2.2/go.mod h1:zzT9QXQAp2Iz2ZdS216UiV8y9uXJYQiGE1q8v1FyhqU= +github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693 h1:oAnuX7YdawNRJN9jAJqs4uWfv11R+9sgBBMalTp2Xz0= +github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693/go.mod h1:zzT9QXQAp2Iz2ZdS216UiV8y9uXJYQiGE1q8v1FyhqU= github.com/grafana/pyroscope-go/godeltaprof v0.1.8 h1:iwOtYXeeVSAeYefJNaxDytgjKtUuKQbJqgAIjlnicKg= github.com/grafana/pyroscope-go/godeltaprof v0.1.8/go.mod h1:2+l7K7twW49Ct4wFluZD3tZ6e0SjanjcUUBPVD/UuGU= github.com/gravitational/go-cassandra-native-protocol v0.0.0-teleport.1 h1:zGsdDzqXSuXI+1t+2TRRzdYiv+B3M4IgOPA8W/raFOA= diff --git a/integrations/terraform-mwi/go.mod b/integrations/terraform-mwi/go.mod index 7c63eb33fe5..acf9f28946f 100644 --- a/integrations/terraform-mwi/go.mod +++ b/integrations/terraform-mwi/go.mod @@ -257,7 +257,7 @@ require ( github.com/gorilla/securecookie v1.1.2 // indirect github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/gosuri/uitable v0.0.4 // indirect - github.com/grafana/pyroscope-go v1.2.2 // indirect + github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693 // indirect github.com/grafana/pyroscope-go/godeltaprof v0.1.8 // indirect github.com/gravitational/license v0.0.0-20250329001817-070456fa8ec1 // indirect github.com/gravitational/roundtrip v1.0.2 // indirect diff --git a/integrations/terraform-mwi/go.sum b/integrations/terraform-mwi/go.sum index 08373258d80..6b71457b7c4 100644 --- a/integrations/terraform-mwi/go.sum +++ b/integrations/terraform-mwi/go.sum @@ -689,8 +689,8 @@ github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5T github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= github.com/gosuri/uitable v0.0.4 h1:IG2xLKRvErL3uhY6e1BylFzG+aJiwQviDDTfOKeKTpY= github.com/gosuri/uitable v0.0.4/go.mod h1:tKR86bXuXPZazfOTG1FIzvjIdXzd0mo4Vtn16vt0PJo= -github.com/grafana/pyroscope-go v1.2.2 h1:uvKCyZMD724RkaCEMrSTC38Yn7AnFe8S2wiAIYdDPCE= -github.com/grafana/pyroscope-go v1.2.2/go.mod h1:zzT9QXQAp2Iz2ZdS216UiV8y9uXJYQiGE1q8v1FyhqU= +github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693 h1:oAnuX7YdawNRJN9jAJqs4uWfv11R+9sgBBMalTp2Xz0= +github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693/go.mod h1:zzT9QXQAp2Iz2ZdS216UiV8y9uXJYQiGE1q8v1FyhqU= github.com/grafana/pyroscope-go/godeltaprof v0.1.8 h1:iwOtYXeeVSAeYefJNaxDytgjKtUuKQbJqgAIjlnicKg= github.com/grafana/pyroscope-go/godeltaprof v0.1.8/go.mod h1:2+l7K7twW49Ct4wFluZD3tZ6e0SjanjcUUBPVD/UuGU= github.com/gravitational/go-cassandra-native-protocol v0.0.0-teleport.1 h1:zGsdDzqXSuXI+1t+2TRRzdYiv+B3M4IgOPA8W/raFOA= diff --git a/integrations/terraform/go.mod b/integrations/terraform/go.mod index 9990dd34b30..dcf0fbc7786 100644 --- a/integrations/terraform/go.mod +++ b/integrations/terraform/go.mod @@ -260,7 +260,7 @@ require ( github.com/gorilla/securecookie v1.1.2 // indirect github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/gosuri/uitable v0.0.4 // indirect - github.com/grafana/pyroscope-go v1.2.2 // indirect + github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693 // indirect github.com/grafana/pyroscope-go/godeltaprof v0.1.8 // indirect github.com/gravitational/license v0.0.0-20250329001817-070456fa8ec1 // indirect github.com/gravitational/roundtrip v1.0.2 // indirect diff --git a/integrations/terraform/go.sum b/integrations/terraform/go.sum index 07efa45187c..e44345c79fc 100644 --- a/integrations/terraform/go.sum +++ b/integrations/terraform/go.sum @@ -790,8 +790,8 @@ github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5T github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= github.com/gosuri/uitable v0.0.4 h1:IG2xLKRvErL3uhY6e1BylFzG+aJiwQviDDTfOKeKTpY= github.com/gosuri/uitable v0.0.4/go.mod h1:tKR86bXuXPZazfOTG1FIzvjIdXzd0mo4Vtn16vt0PJo= -github.com/grafana/pyroscope-go v1.2.2 h1:uvKCyZMD724RkaCEMrSTC38Yn7AnFe8S2wiAIYdDPCE= -github.com/grafana/pyroscope-go v1.2.2/go.mod h1:zzT9QXQAp2Iz2ZdS216UiV8y9uXJYQiGE1q8v1FyhqU= +github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693 h1:oAnuX7YdawNRJN9jAJqs4uWfv11R+9sgBBMalTp2Xz0= +github.com/grafana/pyroscope-go v1.2.3-0.20250624101038-0410c20e9693/go.mod h1:zzT9QXQAp2Iz2ZdS216UiV8y9uXJYQiGE1q8v1FyhqU= github.com/grafana/pyroscope-go/godeltaprof v0.1.8 h1:iwOtYXeeVSAeYefJNaxDytgjKtUuKQbJqgAIjlnicKg= github.com/grafana/pyroscope-go/godeltaprof v0.1.8/go.mod h1:2+l7K7twW49Ct4wFluZD3tZ6e0SjanjcUUBPVD/UuGU= github.com/gravitational/go-cassandra-native-protocol v0.0.0-teleport.1 h1:zGsdDzqXSuXI+1t+2TRRzdYiv+B3M4IgOPA8W/raFOA= diff --git a/lib/service/pyroscope.go b/lib/service/pyroscope.go index d35fd7f2352..c5ec7b8a90e 100644 --- a/lib/service/pyroscope.go +++ b/lib/service/pyroscope.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "log/slog" + "net/http" "os" "time" @@ -27,6 +28,7 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport" + logutils "github.com/gravitational/teleport/lib/utils/log" ) // TODO: Replace logger when pyroscope uses slog @@ -34,6 +36,24 @@ type pyroscopeLogger struct { l *slog.Logger } +type roundTripper struct { + tripper http.RoundTripper + timeout time.Duration + logger *slog.Logger +} + +// CloseIdleConnections ensures idle connections of the wrapped +// [http.RoundTripper] are closed. +func (rt roundTripper) CloseIdleConnections() { + type closeIdler interface { + CloseIdleConnections() + } + + if tr, ok := rt.tripper.(closeIdler); ok { + tr.CloseIdleConnections() + } +} + func (l pyroscopeLogger) Infof(format string, args ...any) { if !l.l.Handler().Enabled(context.Background(), slog.LevelInfo) { return @@ -60,6 +80,19 @@ func (l pyroscopeLogger) Errorf(format string, args ...any) { l.l.Error(fmt.Sprintf(format, args...)) } +func (rt roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + start := time.Now() + resp, err := rt.tripper.RoundTrip(req) + duration := time.Since(start) + + threshold := rt.timeout * 90 / 100 + if duration > threshold { + rt.logger.DebugContext(req.Context(), "Pyroscope upload exceeded threshold", "upload_duration", duration, "upload_threshold", threshold, "upload_url", logutils.StringerAttr(req.URL)) + } + + return resp, err +} + // createPyroscopeConfig generates the Pyroscope configuration for the Teleport process. func createPyroscopeConfig(ctx context.Context, logger *slog.Logger, address string) (pyroscope.Config, error) { if address == "" { @@ -71,6 +104,17 @@ func createPyroscopeConfig(ctx context.Context, logger *slog.Logger, address str hostname = "unknown" } + const httpTimeout = 60 * time.Second + + httpClient := &http.Client{ + Timeout: httpTimeout, + Transport: roundTripper{ + tripper: http.DefaultTransport, + timeout: httpTimeout, + logger: logger, + }, + } + config := pyroscope.Config{ ApplicationName: teleport.ComponentTeleport, ServerAddress: address, @@ -80,6 +124,8 @@ func createPyroscopeConfig(ctx context.Context, logger *slog.Logger, address str "version": teleport.Version, "git_ref": teleport.Gitref, }, + HTTPClient: httpClient, + UploadRate: 60 * time.Second, } // Evaluate if profile configuration is customized @@ -90,21 +136,14 @@ func createPyroscopeConfig(ctx context.Context, logger *slog.Logger, address str logger.InfoContext(ctx, "Pyroscope will configure profiles from env") } - var uploadRate *time.Duration if rate := os.Getenv("TELEPORT_PYROSCOPE_UPLOAD_RATE"); rate != "" { parsedRate, err := time.ParseDuration(rate) if err != nil { logger.InfoContext(ctx, "invalid TELEPORT_PYROSCOPE_UPLOAD_RATE, ignoring value", "provided_value", rate, "error", err) } else { - uploadRate = &parsedRate + logger.InfoContext(ctx, "TELEPORT_PYROSCOPE_UPLOAD_RATE configured", "rate", parsedRate) + config.UploadRate = parsedRate } - } else { - logger.InfoContext(ctx, "TELEPORT_PYROSCOPE_UPLOAD_RATE not specified, using default") - } - - // Set UploadRate or fall back to defaults - if uploadRate != nil { - config.UploadRate = *uploadRate } if value, isSet := os.LookupEnv("TELEPORT_PYROSCOPE_KUBE_COMPONENT"); isSet { @@ -131,13 +170,13 @@ func (process *TeleportProcess) initPyroscope(address string) { if err != nil { logger.ErrorContext(process.ExitContext(), "error starting pyroscope profiler", "address", address, "error", err) } else { + logger.InfoContext(process.ExitContext(), "Pyroscope has successfully started") process.OnExit("pyroscope.profiler", func(payload any) { // Observed rare and inconsistent panics, short term solution is to not wait for flush profiler.Flush(false) _ = profiler.Stop() }) } - logger.InfoContext(process.ExitContext(), "Pyroscope has successfully started") } // getPyroscopeProfileTypesFromEnv sets the profile types based on environment variables.