From bd39b9267ade75880af68e2358bcc583fc471d6b Mon Sep 17 00:00:00 2001 From: Alex Ellis Date: Wed, 3 Oct 2018 13:16:28 +0100 Subject: [PATCH] Update comments - updates comments and adds where missing - updates locks so that unlock is done via defer instead of at the end of the statement - extracts timeout variable in two places - remove makeClient() unused method from metrics package No-harm changes tested via go build. Signed-off-by: Alex Ellis (VMware) --- gateway/handlers/alerthandler.go | 27 +++++++++++++++------------ gateway/handlers/forwarding_proxy.go | 2 +- gateway/handlers/function_cache.go | 9 +++++---- gateway/metrics/add_metrics.go | 8 -------- gateway/metrics/exporter.go | 5 ++++- gateway/plugin/external.go | 4 +++- gateway/requests/requests.go | 2 ++ gateway/types/readconfig.go | 4 ++-- gateway/version/version.go | 13 ++++++++----- 9 files changed, 40 insertions(+), 34 deletions(-) diff --git a/gateway/handlers/alerthandler.go b/gateway/handlers/alerthandler.go index 716dab1d..ce908ced 100644 --- a/gateway/handlers/alerthandler.go +++ b/gateway/handlers/alerthandler.go @@ -14,23 +14,25 @@ import ( "github.com/openfaas/faas/gateway/requests" ) -// DefaultMinReplicas is the minimal amount of replicas for a service. -const DefaultMinReplicas = 1 +const ( + // DefaultMinReplicas is the minimal amount of replicas for a service. + DefaultMinReplicas = 1 -// DefaultMaxReplicas is the amount of replicas a service will auto-scale up to. -const DefaultMaxReplicas = 20 + // DefaultMaxReplicas is the amount of replicas a service will auto-scale up to. + DefaultMaxReplicas = 20 -// DefaultScalingFactor is the defining proportion for the scaling increments. -const DefaultScalingFactor = 20 + // DefaultScalingFactor is the defining proportion for the scaling increments. + DefaultScalingFactor = 20 -// MinScaleLabel label indicating min scale for a function -const MinScaleLabel = "com.openfaas.scale.min" + // MinScaleLabel label indicating min scale for a function + MinScaleLabel = "com.openfaas.scale.min" -// MaxScaleLabel label indicating max scale for a function -const MaxScaleLabel = "com.openfaas.scale.max" + // MaxScaleLabel label indicating max scale for a function + MaxScaleLabel = "com.openfaas.scale.max" -// ScalingFactorLabel label indicates the scaling factor for a function -const ScalingFactorLabel = "com.openfaas.scale.factor" + // ScalingFactorLabel label indicates the scaling factor for a function + ScalingFactorLabel = "com.openfaas.scale.factor" +) // MakeAlertHandler handles alerts from Prometheus Alertmanager func MakeAlertHandler(service ServiceQuery) http.HandlerFunc { @@ -130,5 +132,6 @@ func CalculateReplicas(status string, currentReplicas uint64, maxReplicas uint64 } else { // Resolved event. newReplicas = minReplicas } + return newReplicas } diff --git a/gateway/handlers/forwarding_proxy.go b/gateway/handlers/forwarding_proxy.go index d5f6b1c2..81627f73 100644 --- a/gateway/handlers/forwarding_proxy.go +++ b/gateway/handlers/forwarding_proxy.go @@ -23,7 +23,7 @@ import ( // functionMatcher parses out the service name (group 1) and rest of path (group 2). var functionMatcher = regexp.MustCompile("^/?(?:async-)?function/([^/?]+)([^?]*)") -// Indicies and meta-data for functionMatcher regex parts +// Indices and meta-data for functionMatcher regex parts const ( hasPathCount = 3 routeIndex = 0 // routeIndex corresponds to /function/ or /async-function/ diff --git a/gateway/handlers/function_cache.go b/gateway/handlers/function_cache.go index 58a835e3..1c141f28 100644 --- a/gateway/handlers/function_cache.go +++ b/gateway/handlers/function_cache.go @@ -31,6 +31,7 @@ type FunctionCache struct { // Set replica count for functionName func (fc *FunctionCache) Set(functionName string, serviceQueryResponse ServiceQueryResponse) { fc.Sync.Lock() + defer fc.Sync.Unlock() if _, exists := fc.Cache[functionName]; !exists { fc.Cache[functionName] = &FunctionMeta{} @@ -40,23 +41,23 @@ func (fc *FunctionCache) Set(functionName string, serviceQueryResponse ServiceQu entry.LastRefresh = time.Now() entry.ServiceQueryResponse = serviceQueryResponse - fc.Sync.Unlock() } // Get replica count for functionName func (fc *FunctionCache) Get(functionName string) (ServiceQueryResponse, bool) { + + fc.Sync.Lock() + defer fc.Sync.Unlock() + replicas := ServiceQueryResponse{ AvailableReplicas: 0, } hit := false - fc.Sync.Lock() - if val, exists := fc.Cache[functionName]; exists { replicas = val.ServiceQueryResponse hit = !val.Expired(fc.Expiry) } - fc.Sync.Unlock() return replicas, hit } diff --git a/gateway/metrics/add_metrics.go b/gateway/metrics/add_metrics.go index 02437a94..f143a66c 100644 --- a/gateway/metrics/add_metrics.go +++ b/gateway/metrics/add_metrics.go @@ -13,16 +13,10 @@ import ( "github.com/openfaas/faas/gateway/requests" ) -func makeClient() http.Client { - // Fine-tune the client to fail fast. - return http.Client{} -} - // AddMetricsHandler wraps a http.HandlerFunc with Prometheus metrics func AddMetricsHandler(handler http.HandlerFunc, prometheusQuery PrometheusQueryFetcher) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - // log.Printf("Calling upstream for function info\n") recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, r) @@ -56,7 +50,6 @@ func AddMetricsHandler(handler http.HandlerFunc, prometheusQuery PrometheusQuery return } - // log.Printf("Querying Prometheus API\n") expr := url.QueryEscape(`sum(gateway_function_invocation_total{function_name=~".*", code=~".*"}) by (function_name, code)`) // expr := "sum(gateway_function_invocation_total%7Bfunction_name%3D~%22.*%22%2C+code%3D~%22.*%22%7D)+by+(function_name%2C+code)" results, fetchErr := prometheusQuery.Fetch(expr) @@ -76,7 +69,6 @@ func AddMetricsHandler(handler http.HandlerFunc, prometheusQuery PrometheusQuery return } - // log.Printf("Writing bytesOut: %s\n", bytesOut) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) w.Write(bytesOut) diff --git a/gateway/metrics/exporter.go b/gateway/metrics/exporter.go index 5a627d71..8ceeb88f 100644 --- a/gateway/metrics/exporter.go +++ b/gateway/metrics/exporter.go @@ -60,11 +60,14 @@ func (e *Exporter) Collect(ch chan<- prometheus.Metric) { func (e *Exporter) StartServiceWatcher(endpointURL url.URL, metricsOptions MetricOptions, label string, interval time.Duration) { ticker := time.NewTicker(interval) quit := make(chan struct{}) + + timeout := 3 * time.Second + proxyClient := http.Client{ Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, DialContext: (&net.Dialer{ - Timeout: 3 * time.Second, + Timeout: timeout, KeepAlive: 0, }).DialContext, MaxIdleConns: 1, diff --git a/gateway/plugin/external.go b/gateway/plugin/external.go index b8fcd6ae..58c17f4a 100644 --- a/gateway/plugin/external.go +++ b/gateway/plugin/external.go @@ -24,11 +24,13 @@ import ( // NewExternalServiceQuery proxies service queries to external plugin via HTTP func NewExternalServiceQuery(externalURL url.URL, credentials *auth.BasicAuthCredentials) handlers.ServiceQuery { + timeout := 3 * time.Second + proxyClient := http.Client{ Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, DialContext: (&net.Dialer{ - Timeout: 3 * time.Second, + Timeout: timeout, KeepAlive: 0, }).DialContext, MaxIdleConns: 1, diff --git a/gateway/requests/requests.go b/gateway/requests/requests.go index 048c1ab1..3ca4a8ce 100644 --- a/gateway/requests/requests.go +++ b/gateway/requests/requests.go @@ -1,6 +1,8 @@ // Copyright (c) Alex Ellis 2017. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +// Package requests package provides a client SDK or library for +// the OpenFaaS gateway REST API package requests // CreateFunctionRequest create a function in the swarm. diff --git a/gateway/types/readconfig.go b/gateway/types/readconfig.go index 5f0986e9..cfe5c712 100644 --- a/gateway/types/readconfig.go +++ b/gateway/types/readconfig.go @@ -51,7 +51,7 @@ func parseIntOrDurationValue(val string, fallback time.Duration) time.Duration { return duration } -// Read fetches config from environmental variables. +// Read fetches gateway server configuration from environmental variables func (ReadConfig) Read(hasEnv HasEnv) GatewayConfig { cfg := GatewayConfig{ PrometheusHost: "prometheus", @@ -117,7 +117,7 @@ func (ReadConfig) Read(hasEnv HasEnv) GatewayConfig { return cfg } -// GatewayConfig for the process. +// GatewayConfig provides config for the API Gateway server process type GatewayConfig struct { // HTTP timeout for reading a request from clients. diff --git a/gateway/version/version.go b/gateway/version/version.go index 16093fe4..e1011c27 100644 --- a/gateway/version/version.go +++ b/gateway/version/version.go @@ -1,17 +1,20 @@ package version var ( - //Version release version of the provider + // Version release version of the provider Version string - //GitCommit SHA of the last git commit + + // GitCommitSHA is the Git SHA of the latest tag/release GitCommitSHA string - //GitCommit message of the last commit + + // GitCommitMessage as read from the latest tag/release GitCommitMessage string - //DevVersion string for the development version + + // DevVersion string for the development version DevVersion = "dev" ) -//BuildVersion returns current version of the provider +// BuildVersion returns current version of the provider func BuildVersion() string { if len(Version) == 0 { return DevVersion