From b0b1d6f02f4ad3669876e7652ac2830f4c3839c8 Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Wed, 11 Sep 2024 20:34:58 +0000 Subject: [PATCH 1/7] Introduce RED metrics for ws-proxy Originally from https://github.com/gitpod-io/gitpod/pull/17294 Co-authored-by: Anton Kosyakov --- .../ws-proxy/pkg/common/infoprovider.go | 4 + components/ws-proxy/pkg/proxy/metrics.go | 162 ++++++++++++++++++ components/ws-proxy/pkg/proxy/pass.go | 11 +- components/ws-proxy/pkg/proxy/routes.go | 65 ++++--- .../ws-proxy/pkg/proxy/workspacerouter.go | 15 +- 5 files changed, 223 insertions(+), 34 deletions(-) create mode 100644 components/ws-proxy/pkg/proxy/metrics.go diff --git a/components/ws-proxy/pkg/common/infoprovider.go b/components/ws-proxy/pkg/common/infoprovider.go index cf078d82c6392d..9a681a31bfb649 100644 --- a/components/ws-proxy/pkg/common/infoprovider.go +++ b/components/ws-proxy/pkg/common/infoprovider.go @@ -23,6 +23,8 @@ const ( WorkspacePathPrefixIdentifier = "workspacePathPrefix" WorkspaceInfoIdentifier = "workspaceInfo" + + ForeignContentIdentifier = "foreignContent" ) // WorkspaceCoords represents the coordinates of a workspace (port). @@ -33,6 +35,8 @@ type WorkspaceCoords struct { Port string // Debug workspace Debug bool + // Foreign content + Foreign bool } // WorkspaceInfoProvider is an entity that is able to provide workspaces related information. diff --git a/components/ws-proxy/pkg/proxy/metrics.go b/components/ws-proxy/pkg/proxy/metrics.go new file mode 100644 index 00000000000000..0cee30acc315d8 --- /dev/null +++ b/components/ws-proxy/pkg/proxy/metrics.go @@ -0,0 +1,162 @@ +// Copyright (c) 2024 Gitpod GmbH. All rights reserved. +// Licensed under the GNU Affero General Public License (AGPL). +// See License.AGPL.txt in the project root for license information. + +package proxy + +import ( + "context" + "net/http" + "strings" + + "github.com/gitpod-io/gitpod/common-go/log" + "github.com/gorilla/mux" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +type httpMetrics struct { + requestsTotal *prometheus.CounterVec + requestsDuration *prometheus.HistogramVec +} + +func (m *httpMetrics) Describe(ch chan<- *prometheus.Desc) { + m.requestsTotal.Describe(ch) + m.requestsDuration.Describe(ch) +} + +func (m *httpMetrics) Collect(ch chan<- prometheus.Metric) { + m.requestsTotal.Collect(ch) + m.requestsDuration.Collect(ch) +} + +var ( + serverMetrics = &httpMetrics{ + requestsTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "http_server_requests_total", + Help: "Total number of incoming HTTP requests", + }, []string{"method", "resource", "code", "http_version"}), + requestsDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Name: "http_server_requests_duration_seconds", + Help: "Duration of incoming HTTP requests in seconds", + Buckets: []float64{.005, .025, .05, .1, .5, 1, 2.5, 5, 30, 60, 120, 240, 600}, + }, []string{"method", "resource", "code", "http_version"}), + } + clientMetrics = &httpMetrics{ + requestsTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "http_client_requests_total", + Help: "Total number of outgoing HTTP requests", + }, []string{"method", "resource", "code", "http_version"}), + requestsDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Name: "http_client_requests_duration_seconds", + Help: "Duration of outgoing HTTP requests in seconds", + Buckets: []float64{.005, .025, .05, .1, .5, 1, 2.5, 5, 30, 60, 120, 240, 600}, + }, []string{"method", "resource", "code", "http_version"}), + } +) + +func init() { + metrics.Registry.MustRegister(serverMetrics, clientMetrics) +} + +type contextKey int + +var ( + resourceKey = contextKey(0) +) + +func withResource(r *http.Request, resource string) *http.Request { + ctx := context.WithValue(r.Context(), resourceKey, []string{resource}) + return r.WithContext(ctx) +} + +func withResourceLabel() promhttp.Option { + return promhttp.WithLabelFromCtx("resource", func(ctx context.Context) string { + if v := ctx.Value(resourceKey); v != nil { + if resources, ok := v.([]string); ok { + if len(resources) > 0 { + return resources[0] + } + } + } + return "unknown" + }) +} + +func instrumentClientMetrics(transport http.RoundTripper) http.RoundTripper { + return promhttp.InstrumentRoundTripperCounter(clientMetrics.requestsTotal, + promhttp.InstrumentRoundTripperDuration(clientMetrics.requestsDuration, + transport, + withResourceLabel()), + withResourceLabel(), + ) +} + +func instrumentServerMetrics(next http.Handler) http.Handler { + handler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + next.ServeHTTP(w, req) + if v := req.Context().Value(resourceKey); v != nil { + if resources, ok := v.([]string); ok { + if len(resources) > 0 { + resources[0] = getHandlerResource(req) + } + } + } + }) + instrumented := promhttp.InstrumentHandlerCounter(serverMetrics.requestsTotal, + promhttp.InstrumentHandlerDuration(serverMetrics.requestsDuration, + handler, + withResourceLabel()), + withResourceLabel(), + ) + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + ctx := context.WithValue(req.Context(), resourceKey, []string{"unknown"}) + instrumented.ServeHTTP(w, req.WithContext(ctx)) + }) +} + +func getHandlerResource(req *http.Request) string { + hostPart := getResourceHost(req) + if hostPart == "" { + hostPart = "unknown" + log.WithField("URL", req.URL).Warn("client metrics: cannot determine resource host part") + } + + routePart := "" + if route := mux.CurrentRoute(req); route != nil { + routePart = route.GetName() + } + if routePart == "" { + log.WithField("URL", req.URL).Warn("client metrics: cannot determine resource route part") + routePart = "unknown" + } + if routePart == "root" { + routePart = "" + } else { + routePart = "/" + routePart + } + return hostPart + routePart +} + +func getResourceHost(req *http.Request) string { + coords := getWorkspaceCoords(req) + + var parts []string + + if coords.Foreign { + parts = append(parts, "foreign_content") + } + + if coords.ID != "" { + workspacePart := "workspace" + if coords.Debug { + workspacePart = "debug_" + workspacePart + } + if coords.Port != "" { + workspacePart += "_port" + } + parts = append(parts, workspacePart) + } + return strings.Join(parts, "/") +} diff --git a/components/ws-proxy/pkg/proxy/pass.go b/components/ws-proxy/pkg/proxy/pass.go index 596a1fccd92f7b..e2bfac2858b3c6 100644 --- a/components/ws-proxy/pkg/proxy/pass.go +++ b/components/ws-proxy/pkg/proxy/pass.go @@ -41,7 +41,7 @@ type proxyPassOpt func(h *proxyPassConfig) type errorHandler func(http.ResponseWriter, *http.Request, error) // targetResolver is a function that determines to which target to forward the given HTTP request to. -type targetResolver func(*Config, common.WorkspaceInfoProvider, *http.Request) (*url.URL, error) +type targetResolver func(*Config, common.WorkspaceInfoProvider, *http.Request) (*url.URL, string, error) type responseHandler func(*http.Response, *http.Request) error @@ -119,7 +119,7 @@ func proxyPass(config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProv } return func(w http.ResponseWriter, req *http.Request) { - targetURL, err := h.TargetResolver(config.Config, infoProvider, req) + targetURL, targetResource, err := h.TargetResolver(config.Config, infoProvider, req) if err != nil { if h.ErrorHandler != nil { h.ErrorHandler(w, req, err) @@ -128,6 +128,7 @@ func proxyPass(config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProv } return } + req = withResource(req, targetResource) originalURL := *req.URL @@ -216,10 +217,10 @@ func withErrorHandler(h errorHandler) proxyPassOpt { } } -func createDefaultTransport(config *TransportConfig) *http.Transport { +func createDefaultTransport(config *TransportConfig) http.RoundTripper { // TODO equivalent of client_max_body_size 2048m; necessary ??? // this is based on http.DefaultTransport, with some values exposed to config - return &http.Transport{ + return instrumentClientMetrics(&http.Transport{ Proxy: http.ProxyFromEnvironment, DialContext: (&net.Dialer{ Timeout: time.Duration(config.ConnectTimeout), // default: 30s @@ -232,7 +233,7 @@ func createDefaultTransport(config *TransportConfig) *http.Transport { IdleConnTimeout: time.Duration(config.IdleConnTimeout), // default: 90s TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, - } + }) } // tell the browser to cache for 1 year and don't ask the server during this period. diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index d045885298c31c..726fc6cd3a4c00 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -83,6 +83,7 @@ type RouteHandler = func(r *mux.Router, config *RouteHandlerConfig) // installWorkspaceRoutes configures routing of workspace and IDE requests. func installWorkspaceRoutes(r *mux.Router, config *RouteHandlerConfig, ip common.WorkspaceInfoProvider, sshGatewayServer *sshproxy.Server) error { r.Use(logHandler) + r.Use(instrumentServerMetrics) // Note: the order of routes defines their priority. // Routes registered first have priority over those that come afterwards. @@ -189,7 +190,7 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S r.NewRoute().HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.Write(byt) - }) + }).Name("ssh_host_key") } func (ir *ideRoutes) HandleCreateKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { @@ -310,7 +311,7 @@ func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated r.Use(ir.Config.WorkspaceAuthHandler) } - r.NewRoute().HandlerFunc(proxyPass(ir.Config, ir.InfoProvider, workspacePodSupervisorResolver, proxyPassOpts...)) + r.NewRoute().HandlerFunc(proxyPass(ir.Config, ir.InfoProvider, workspacePodSupervisorResolver, proxyPassOpts...)).Name("supervisor") } func (ir *ideRoutes) HandleSupervisorFrontendRoute(route *mux.Route) { @@ -331,7 +332,7 @@ func (ir *ideRoutes) HandleSupervisorFrontendRoute(route *mux.Route) { }) }) // always hit the blobserver to ensure that blob is downloaded - r.NewRoute().HandlerFunc(proxyPass(ir.Config, ir.InfoProvider, func(cfg *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (*url.URL, error) { + r.NewRoute().HandlerFunc(proxyPass(ir.Config, ir.InfoProvider, func(cfg *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (*url.URL, string, error) { info := getWorkspaceInfoFromContext(req.Context()) return resolveSupervisorURL(cfg, info, req) }, func(h *proxyPassConfig) { @@ -359,13 +360,13 @@ func (ir *ideRoutes) HandleSupervisorFrontendRoute(route *mux.Route) { return image }, } - }, withUseTargetHost())) + }, withUseTargetHost())).Name("supervisor_frontend") } -func resolveSupervisorURL(cfg *Config, info *common.WorkspaceInfo, req *http.Request) (*url.URL, error) { +func resolveSupervisorURL(cfg *Config, info *common.WorkspaceInfo, req *http.Request) (*url.URL, string, error) { if info == nil && len(cfg.WorkspacePodConfig.SupervisorImage) == 0 { log.WithFields(log.OWI("", getWorkspaceCoords(req).ID, "")).Warn("no workspace info available - cannot resolve supervisor route") - return nil, xerrors.Errorf("no workspace information available - cannot resolve supervisor route") + return nil, "", xerrors.Errorf("no workspace information available - cannot resolve supervisor route") } // use the config value for backwards compatibility when info.SupervisorImage is not set @@ -378,7 +379,7 @@ func resolveSupervisorURL(cfg *Config, info *common.WorkspaceInfo, req *http.Req dst.Scheme = cfg.BlobServer.Scheme dst.Host = cfg.BlobServer.Host dst.Path = cfg.BlobServer.PathPrefix + "/" + supervisorImage - return &dst, nil + return &dst, "blobserve/supervisor", nil } type BlobserveInlineVars struct { @@ -411,11 +412,11 @@ func (ir *ideRoutes) HandleRoot(route *mux.Route) { if imagePath != "/index.html" && imagePath != "/" { return image } - // blobserve can inline static links in index.html for IDE and supervisor to avoid redirects for each resource + // blobserve can inline static links in index.html for IDE and supervisor to avoid redirects for each supervisor resource // but it has to know exposed URLs in the context of current workspace cluster // so first we ask blobserve to preload the supervisor image // and if it is successful we pass exposed URLs to IDE and supervisor to blobserve for inlining - supervisorURL, err := resolveSupervisorURL(t.Config, info, req) + supervisorURL, supervisorResource, err := resolveSupervisorURL(t.Config, info, req) if err != nil { log.WithError(err).Error("could not preload supervisor") return image @@ -426,6 +427,7 @@ func (ir *ideRoutes) HandleRoot(route *mux.Route) { log.WithField("supervisorURL", supervisorURL).WithError(err).Error("could not preload supervisor") return image } + preloadSupervisorReq = withResource(preloadSupervisorReq, supervisorResource) resp, err := t.DoRoundTrip(preloadSupervisorReq) if err != nil { log.WithField("supervisorURL", supervisorURL).WithError(err).Error("could not preload supervisor") @@ -457,10 +459,12 @@ func (ir *ideRoutes) HandleRoot(route *mux.Route) { return image }, } - }, withHTTPErrorHandler(directIDEPass), withUseTargetHost())) + }, withHTTPErrorHandler(directIDEPass), withUseTargetHost())).Name("root") } func installForeignRoutes(r *mux.Router, config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProvider) error { + r.Use(instrumentServerMetrics) + err := installWorkspacePortRoutes(r.MatcherFunc(func(r *http.Request, rm *mux.RouteMatch) bool { workspacePathPrefix := rm.Vars[common.WorkspacePathPrefixIdentifier] if workspacePathPrefix == "" || rm.Vars[common.WorkspacePortIdentifier] == "" { @@ -483,24 +487,24 @@ func installForeignRoutes(r *mux.Router, config *RouteHandlerConfig, infoProvide if err != nil { return err } - installBlobserveRoutes(r.NewRoute().Subrouter(), config, infoProvider) + installForeignBlobserveRoutes(r.NewRoute().Subrouter(), config, infoProvider) return nil } const imagePathSeparator = "/__files__" -// installBlobserveRoutes implements long-lived caching with versioned URLs, see https://web.dev/http-cache/#versioned-urls -func installBlobserveRoutes(r *mux.Router, config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProvider) { +// installForeignBlobserveRoutes implements long-lived caching with versioned URLs, see https://web.dev/http-cache/#versioned-urls +func installForeignBlobserveRoutes(r *mux.Router, config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProvider) { r.Use(logHandler) r.Use(logRouteHandlerHandler("BlobserveRootHandler")) // filter all session cookies r.Use(sensitiveCookieHandler(config.Config.GitpodInstallation.HostName)) - targetResolver := func(cfg *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (tgt *url.URL, err error) { + targetResolver := func(cfg *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (tgt *url.URL, str string, err error) { segments := strings.SplitN(req.URL.Path, imagePathSeparator, 2) if len(segments) < 2 { - return nil, xerrors.Errorf("invalid URL") + return nil, "", xerrors.Errorf("invalid URL") } image, path := segments[0], segments[1] @@ -510,9 +514,9 @@ func installBlobserveRoutes(r *mux.Router, config *RouteHandlerConfig, infoProvi dst.Scheme = cfg.BlobServer.Scheme dst.Host = cfg.BlobServer.Host dst.Path = cfg.BlobServer.PathPrefix + "/" + strings.TrimPrefix(image, "/") - return &dst, nil + return &dst, "blobserve/foreign_content", nil } - r.NewRoute().Handler(proxyPass(config, infoProvider, targetResolver, withLongTermCaching(), withUseTargetHost())) + r.NewRoute().Handler(proxyPass(config, infoProvider, targetResolver, withLongTermCaching(), withUseTargetHost())).Name("blobserve") } // installDebugWorkspaceRoutes configures for debug workspace. @@ -583,27 +587,32 @@ func installWorkspacePortRoutes(r *mux.Router, config *RouteHandlerConfig, infoP } // workspacePodResolver resolves to the workspace pod's url from the given request. -func workspacePodResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (url *url.URL, err error) { +func workspacePodResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (url *url.URL, resource string, err error) { coords := getWorkspaceCoords(req) var port string if coords.Debug { + resource = "debug_workspace" port = fmt.Sprint(config.WorkspacePodConfig.IDEDebugPort) } else { + resource = "workspace" port = fmt.Sprint(config.WorkspacePodConfig.TheiaPort) } workspaceInfo := infoProvider.WorkspaceInfo(coords.ID) - return buildWorkspacePodURL(api.PortProtocol_PORT_PROTOCOL_HTTP, workspaceInfo.IPAddress, port) + url, err = buildWorkspacePodURL(api.PortProtocol_PORT_PROTOCOL_HTTP, workspaceInfo.IPAddress, port) + return } // workspacePodPortResolver resolves to the workspace pods ports. -func workspacePodPortResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (url *url.URL, err error) { +func workspacePodPortResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (url *url.URL, resource string, err error) { coords := getWorkspaceCoords(req) workspaceInfo := infoProvider.WorkspaceInfo(coords.ID) var port string protocol := api.PortProtocol_PORT_PROTOCOL_HTTP if coords.Debug { + resource = "debug_workspace_port" port = fmt.Sprint(config.WorkspacePodConfig.DebugWorkspaceProxyPort) } else { + resource = "workspace_port" port = coords.Port prt, err := strconv.ParseUint(port, 10, 16) if err != nil { @@ -617,27 +626,31 @@ func workspacePodPortResolver(config *Config, infoProvider common.WorkspaceInfoP } } } - return buildWorkspacePodURL(protocol, workspaceInfo.IPAddress, port) + url, err = buildWorkspacePodURL(protocol, workspaceInfo.IPAddress, port) + return } // workspacePodSupervisorResolver resolves to the workspace pods Supervisor url from the given request. -func workspacePodSupervisorResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (url *url.URL, err error) { +func workspacePodSupervisorResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (url *url.URL, resource string, err error) { coords := getWorkspaceCoords(req) var port string if coords.Debug { + resource = "debug_workspace/supervisor" port = fmt.Sprint(config.WorkspacePodConfig.SupervisorDebugPort) } else { + resource = "workspace/supervisor" port = fmt.Sprint(config.WorkspacePodConfig.SupervisorPort) } workspaceInfo := infoProvider.WorkspaceInfo(coords.ID) - return buildWorkspacePodURL(api.PortProtocol_PORT_PROTOCOL_HTTP, workspaceInfo.IPAddress, port) + url, err = buildWorkspacePodURL(api.PortProtocol_PORT_PROTOCOL_HTTP, workspaceInfo.IPAddress, port) + return } -func dynamicIDEResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (res *url.URL, err error) { +func dynamicIDEResolver(config *Config, infoProvider common.WorkspaceInfoProvider, req *http.Request) (res *url.URL, resource string, err error) { info := getWorkspaceInfoFromContext(req.Context()) if info == nil { log.WithFields(log.OWI("", getWorkspaceCoords(req).ID, "")).Warn("no workspace info available - cannot resolve Theia route") - return nil, xerrors.Errorf("no workspace information available - cannot resolve Theia route") + return nil, "", xerrors.Errorf("no workspace information available - cannot resolve Theia route") } var dst url.URL @@ -645,7 +658,7 @@ func dynamicIDEResolver(config *Config, infoProvider common.WorkspaceInfoProvide dst.Host = config.BlobServer.Host dst.Path = config.BlobServer.PathPrefix + "/" + info.IDEImage - return &dst, nil + return &dst, "blobserve/ide", nil } func buildWorkspacePodURL(protocol api.PortProtocol, ipAddress string, port string) (*url.URL, error) { diff --git a/components/ws-proxy/pkg/proxy/workspacerouter.go b/components/ws-proxy/pkg/proxy/workspacerouter.go index 0c95d62a465b0c..8770ba1cd5569f 100644 --- a/components/ws-proxy/pkg/proxy/workspacerouter.go +++ b/components/ws-proxy/pkg/proxy/workspacerouter.go @@ -26,6 +26,8 @@ const ( workspacePortRegex = "(?P<" + common.WorkspacePortIdentifier + ">[0-9]+)-" debugWorkspaceRegex = "(?P<" + common.DebugWorkspaceIdentifier + ">debug-)?" + + foreignContentRegex = "(?P<" + common.ForeignContentIdentifier + ">foreign-)?" ) // This pattern matches v4 UUIDs as well as the new generated workspace ids (e.g. pink-panda-ns35kd21). @@ -172,6 +174,12 @@ func matchForeignHostHeader(wsHostSuffix string, headerProvider hostHeaderProvid result = true + if m.Vars == nil { + m.Vars = make(map[string]string) + } + + m.Vars[common.ForeignContentIdentifier] = "true" + var pathPrefix, workspaceID, workspacePort, debugWorkspace string matches = pathPortRegex.FindStringSubmatch(req.URL.Path) if len(matches) < 4 { @@ -220,9 +228,10 @@ func matchForeignHostHeader(wsHostSuffix string, headerProvider hostHeaderProvid func getWorkspaceCoords(req *http.Request) common.WorkspaceCoords { vars := mux.Vars(req) return common.WorkspaceCoords{ - ID: vars[common.WorkspaceIDIdentifier], - Port: vars[common.WorkspacePortIdentifier], - Debug: vars[common.DebugWorkspaceIdentifier] == "true", + ID: vars[common.WorkspaceIDIdentifier], + Port: vars[common.WorkspacePortIdentifier], + Debug: vars[common.DebugWorkspaceIdentifier] == "true", + Foreign: vars[common.ForeignContentIdentifier] == "true", } } From a0330f0a92ebbafbed96fbd3687c468d64c91751 Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Wed, 11 Sep 2024 20:43:08 +0000 Subject: [PATCH 2/7] Remove unused var --- components/ws-proxy/pkg/proxy/workspacerouter.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/workspacerouter.go b/components/ws-proxy/pkg/proxy/workspacerouter.go index 8770ba1cd5569f..c03019fdb8c087 100644 --- a/components/ws-proxy/pkg/proxy/workspacerouter.go +++ b/components/ws-proxy/pkg/proxy/workspacerouter.go @@ -26,8 +26,6 @@ const ( workspacePortRegex = "(?P<" + common.WorkspacePortIdentifier + ">[0-9]+)-" debugWorkspaceRegex = "(?P<" + common.DebugWorkspaceIdentifier + ">debug-)?" - - foreignContentRegex = "(?P<" + common.ForeignContentIdentifier + ">foreign-)?" ) // This pattern matches v4 UUIDs as well as the new generated workspace ids (e.g. pink-panda-ns35kd21). From 3608c55cc00a3373a6e82ffa6287091734512700 Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Wed, 11 Sep 2024 21:25:37 +0000 Subject: [PATCH 3/7] [ws-proxy] fix crash loop backoff (WIP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think for this value to be populated, we'll need to "bubble up" httpVersion (like what was done with many methods and resource) 🤔 Think of a better way. --- components/ws-proxy/pkg/proxy/metrics.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/metrics.go b/components/ws-proxy/pkg/proxy/metrics.go index 0cee30acc315d8..71d9ce50458820 100644 --- a/components/ws-proxy/pkg/proxy/metrics.go +++ b/components/ws-proxy/pkg/proxy/metrics.go @@ -84,12 +84,28 @@ func withResourceLabel() promhttp.Option { }) } +func withHttpVersionLabel() promhttp.Option { + return promhttp.WithLabelFromCtx("http_version", func(ctx context.Context) string { + if v := ctx.Value(resourceKey); v != nil { + if resources, ok := v.([]string); ok { + if len(resources) > 0 { + return resources[0] + } + } + } + return "unknown" + }) +} + func instrumentClientMetrics(transport http.RoundTripper) http.RoundTripper { return promhttp.InstrumentRoundTripperCounter(clientMetrics.requestsTotal, promhttp.InstrumentRoundTripperDuration(clientMetrics.requestsDuration, transport, - withResourceLabel()), + withResourceLabel(), + withHttpVersionLabel(), + ), withResourceLabel(), + withHttpVersionLabel(), ) } @@ -107,8 +123,11 @@ func instrumentServerMetrics(next http.Handler) http.Handler { instrumented := promhttp.InstrumentHandlerCounter(serverMetrics.requestsTotal, promhttp.InstrumentHandlerDuration(serverMetrics.requestsDuration, handler, - withResourceLabel()), + withResourceLabel(), + withHttpVersionLabel(), + ), withResourceLabel(), + withHttpVersionLabel(), ) return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { ctx := context.WithValue(req.Context(), resourceKey, []string{"unknown"}) From 02a52621d3f6a190b73c7489e729f4898d6db50c Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Thu, 12 Sep 2024 18:29:31 +0000 Subject: [PATCH 4/7] Add namespace and subsystem to metrics --- components/ws-proxy/pkg/proxy/metrics.go | 33 +++++++++++++++++------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/metrics.go b/components/ws-proxy/pkg/proxy/metrics.go index 71d9ce50458820..3cac5bc6ea59a5 100644 --- a/components/ws-proxy/pkg/proxy/metrics.go +++ b/components/ws-proxy/pkg/proxy/metrics.go @@ -16,6 +16,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics" ) +const ( + metricsNamespace = "gitpod" + metricsSubsystem = "ws_proxy" +) + type httpMetrics struct { requestsTotal *prometheus.CounterVec requestsDuration *prometheus.HistogramVec @@ -34,24 +39,32 @@ func (m *httpMetrics) Collect(ch chan<- prometheus.Metric) { var ( serverMetrics = &httpMetrics{ requestsTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ - Name: "http_server_requests_total", - Help: "Total number of incoming HTTP requests", + Namespace: metricsNamespace, + Subsystem: metricsSubsystem, + Name: "http_server_requests_total", + Help: "Total number of incoming HTTP requests", }, []string{"method", "resource", "code", "http_version"}), requestsDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Name: "http_server_requests_duration_seconds", - Help: "Duration of incoming HTTP requests in seconds", - Buckets: []float64{.005, .025, .05, .1, .5, 1, 2.5, 5, 30, 60, 120, 240, 600}, + Namespace: metricsNamespace, + Subsystem: metricsSubsystem, + Name: "http_server_requests_duration_seconds", + Help: "Duration of incoming HTTP requests in seconds", + Buckets: []float64{.005, .025, .05, .1, .5, 1, 2.5, 5, 30, 60, 120, 240, 600}, }, []string{"method", "resource", "code", "http_version"}), } clientMetrics = &httpMetrics{ requestsTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ - Name: "http_client_requests_total", - Help: "Total number of outgoing HTTP requests", + Namespace: metricsNamespace, + Subsystem: metricsSubsystem, + Name: "http_client_requests_total", + Help: "Total number of outgoing HTTP requests", }, []string{"method", "resource", "code", "http_version"}), requestsDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Name: "http_client_requests_duration_seconds", - Help: "Duration of outgoing HTTP requests in seconds", - Buckets: []float64{.005, .025, .05, .1, .5, 1, 2.5, 5, 30, 60, 120, 240, 600}, + Namespace: metricsNamespace, + Subsystem: metricsSubsystem, + Name: "http_client_requests_duration_seconds", + Help: "Duration of outgoing HTTP requests in seconds", + Buckets: []float64{.005, .025, .05, .1, .5, 1, 2.5, 5, 30, 60, 120, 240, 600}, }, []string{"method", "resource", "code", "http_version"}), } ) From 667cbbb624a4ca916015da3aec3e12cb8549363b Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Thu, 12 Sep 2024 19:32:30 +0000 Subject: [PATCH 5/7] Set a value for http_version label --- components/ws-proxy/pkg/proxy/metrics.go | 16 +++++++++++----- components/ws-proxy/pkg/proxy/pass.go | 1 + components/ws-proxy/pkg/proxy/routes.go | 1 + 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/metrics.go b/components/ws-proxy/pkg/proxy/metrics.go index 3cac5bc6ea59a5..da8ce76903dced 100644 --- a/components/ws-proxy/pkg/proxy/metrics.go +++ b/components/ws-proxy/pkg/proxy/metrics.go @@ -76,7 +76,8 @@ func init() { type contextKey int var ( - resourceKey = contextKey(0) + resourceKey = contextKey(0) + httpVersionKey = contextKey(1) ) func withResource(r *http.Request, resource string) *http.Request { @@ -97,12 +98,17 @@ func withResourceLabel() promhttp.Option { }) } +func withHttpVersion(r *http.Request) *http.Request { + ctx := context.WithValue(r.Context(), httpVersionKey, []string{r.Proto}) + return r.WithContext(ctx) +} + func withHttpVersionLabel() promhttp.Option { return promhttp.WithLabelFromCtx("http_version", func(ctx context.Context) string { - if v := ctx.Value(resourceKey); v != nil { - if resources, ok := v.([]string); ok { - if len(resources) > 0 { - return resources[0] + if v := ctx.Value(httpVersionKey); v != nil { + if versions, ok := v.([]string); ok { + if len(versions) > 0 { + return versions[0] } } } diff --git a/components/ws-proxy/pkg/proxy/pass.go b/components/ws-proxy/pkg/proxy/pass.go index e2bfac2858b3c6..b3a16c4cdbe88c 100644 --- a/components/ws-proxy/pkg/proxy/pass.go +++ b/components/ws-proxy/pkg/proxy/pass.go @@ -129,6 +129,7 @@ func proxyPass(config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProv return } req = withResource(req, targetResource) + req = withHttpVersion(req) originalURL := *req.URL diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index 726fc6cd3a4c00..40a91414b7433a 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -428,6 +428,7 @@ func (ir *ideRoutes) HandleRoot(route *mux.Route) { return image } preloadSupervisorReq = withResource(preloadSupervisorReq, supervisorResource) + preloadSupervisorReq = withHttpVersion(preloadSupervisorReq) resp, err := t.DoRoundTrip(preloadSupervisorReq) if err != nil { log.WithField("supervisorURL", supervisorURL).WithError(err).Error("could not preload supervisor") From 1465575e6d800bbb0f72d17cd8a87c4a7894c456 Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Thu, 12 Sep 2024 20:04:02 +0000 Subject: [PATCH 6/7] Persist http_version for server metrics --- components/ws-proxy/pkg/proxy/metrics.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/ws-proxy/pkg/proxy/metrics.go b/components/ws-proxy/pkg/proxy/metrics.go index da8ce76903dced..870d3778a1a9bd 100644 --- a/components/ws-proxy/pkg/proxy/metrics.go +++ b/components/ws-proxy/pkg/proxy/metrics.go @@ -138,6 +138,13 @@ func instrumentServerMetrics(next http.Handler) http.Handler { } } } + if v := req.Context().Value(httpVersionKey); v != nil { + if versions, ok := v.([]string); ok { + if len(versions) > 0 { + versions[0] = req.Proto + } + } + } }) instrumented := promhttp.InstrumentHandlerCounter(serverMetrics.requestsTotal, promhttp.InstrumentHandlerDuration(serverMetrics.requestsDuration, @@ -150,6 +157,7 @@ func instrumentServerMetrics(next http.Handler) http.Handler { ) return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { ctx := context.WithValue(req.Context(), resourceKey, []string{"unknown"}) + ctx = context.WithValue(ctx, httpVersionKey, []string{"unknown"}) instrumented.ServeHTTP(w, req.WithContext(ctx)) }) } From 785377d532fa858ef74453a2e9bbd009aeb0b7e4 Mon Sep 17 00:00:00 2001 From: Kyle Brennan Date: Mon, 16 Sep 2024 15:31:37 +0000 Subject: [PATCH 7/7] Code review feedback --- components/ws-proxy/pkg/proxy/metrics.go | 4 ++-- components/ws-proxy/pkg/proxy/pass.go | 4 ++-- components/ws-proxy/pkg/proxy/routes.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/metrics.go b/components/ws-proxy/pkg/proxy/metrics.go index 870d3778a1a9bd..3b8f93ca7ac7f8 100644 --- a/components/ws-proxy/pkg/proxy/metrics.go +++ b/components/ws-proxy/pkg/proxy/metrics.go @@ -80,7 +80,7 @@ var ( httpVersionKey = contextKey(1) ) -func withResource(r *http.Request, resource string) *http.Request { +func withResourceMetricsLabel(r *http.Request, resource string) *http.Request { ctx := context.WithValue(r.Context(), resourceKey, []string{resource}) return r.WithContext(ctx) } @@ -98,7 +98,7 @@ func withResourceLabel() promhttp.Option { }) } -func withHttpVersion(r *http.Request) *http.Request { +func withHttpVersionMetricsLabel(r *http.Request) *http.Request { ctx := context.WithValue(r.Context(), httpVersionKey, []string{r.Proto}) return r.WithContext(ctx) } diff --git a/components/ws-proxy/pkg/proxy/pass.go b/components/ws-proxy/pkg/proxy/pass.go index b3a16c4cdbe88c..14899559675ef9 100644 --- a/components/ws-proxy/pkg/proxy/pass.go +++ b/components/ws-proxy/pkg/proxy/pass.go @@ -128,8 +128,8 @@ func proxyPass(config *RouteHandlerConfig, infoProvider common.WorkspaceInfoProv } return } - req = withResource(req, targetResource) - req = withHttpVersion(req) + req = withResourceMetricsLabel(req, targetResource) + req = withHttpVersionMetricsLabel(req) originalURL := *req.URL diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index 40a91414b7433a..78a681a1f47937 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -427,8 +427,8 @@ func (ir *ideRoutes) HandleRoot(route *mux.Route) { log.WithField("supervisorURL", supervisorURL).WithError(err).Error("could not preload supervisor") return image } - preloadSupervisorReq = withResource(preloadSupervisorReq, supervisorResource) - preloadSupervisorReq = withHttpVersion(preloadSupervisorReq) + preloadSupervisorReq = withResourceMetricsLabel(preloadSupervisorReq, supervisorResource) + preloadSupervisorReq = withHttpVersionMetricsLabel(preloadSupervisorReq) resp, err := t.DoRoundTrip(preloadSupervisorReq) if err != nil { log.WithField("supervisorURL", supervisorURL).WithError(err).Error("could not preload supervisor")