From 4474f5f81327b3e75da5a54a729d93c84179bfb9 Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Tue, 12 Nov 2019 00:13:17 +0800 Subject: [PATCH] Skip trace for router healthz endpoint (#1400) Jaeger preserves the trace data of router healthz endpoints which is not so helpful for monitoring function metrics and consumes storage to store such trace data. This PR uses `GetStartOptions` to examine the request's path and check if it's a request to healthz endpoint. If yes, skip and not to trace it. --- charts/fission-all/templates/router.yaml | 2 ++ charts/fission-all/values.yaml | 6 +++++ charts/fission-core/templates/router.yaml | 2 ++ charts/fission-core/values.yaml | 6 +++++ pkg/router/router.go | 32 ++++++++++++++++++++--- pkg/router/router_test.go | 2 +- 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/charts/fission-all/templates/router.yaml b/charts/fission-all/templates/router.yaml index 6ba16ea2..28d4a929 100644 --- a/charts/fission-all/templates/router.yaml +++ b/charts/fission-all/templates/router.yaml @@ -61,6 +61,8 @@ spec: value: {{ .Values.router.useEncodedPath | default false | quote }} - name: DEBUG_ENV value: {{ .Values.debugEnv | quote }} + - name: DISPLAY_ACCESS_LOG + value: {{ .Values.router.displayAccessLog | default false | quote }} {{- if .Values.analytics }} - name: ANALYTICS_URL value: "https://g.fission.sh/metrics" diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml index bf2c84d6..0fd8a716 100644 --- a/charts/fission-all/values.yaml +++ b/charts/fission-all/values.yaml @@ -63,6 +63,12 @@ router: deployAsDaemonSet: false svcAddressMaxRetries: 5 svcAddressUpdateTimeout: 30s + + ## Display endpoint access logs + ## To be aware of enabling logging endpoint access log, it increases + ## router resource utilization when under heavy workloads. + displayAccessLog: false + ## Add annotations for router # svcAnnotations: # cloud.google.com/load-balancer-type: Internal diff --git a/charts/fission-core/templates/router.yaml b/charts/fission-core/templates/router.yaml index 6ba16ea2..28d4a929 100644 --- a/charts/fission-core/templates/router.yaml +++ b/charts/fission-core/templates/router.yaml @@ -61,6 +61,8 @@ spec: value: {{ .Values.router.useEncodedPath | default false | quote }} - name: DEBUG_ENV value: {{ .Values.debugEnv | quote }} + - name: DISPLAY_ACCESS_LOG + value: {{ .Values.router.displayAccessLog | default false | quote }} {{- if .Values.analytics }} - name: ANALYTICS_URL value: "https://g.fission.sh/metrics" diff --git a/charts/fission-core/values.yaml b/charts/fission-core/values.yaml index 68591e2f..2e5bfbad 100644 --- a/charts/fission-core/values.yaml +++ b/charts/fission-core/values.yaml @@ -50,6 +50,12 @@ router: deployAsDaemonSet: false svcAddressMaxRetries: 5 svcAddressUpdateTimeout: 30s + + ## Display endpoint access logs + ## To be aware of enabling logging endpoint access log, it increases + ## router resource utilization when under heavy workloads. + displayAccessLog: false + ## Add annotations for router # svcAnnotations: # cloud.google.com/load-balancer-type: Internal diff --git a/pkg/router/router.go b/pkg/router/router.go index f5bbf7bc..5b70138c 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -45,6 +45,7 @@ import ( "net/http" "os" "strconv" + "strings" "time" "github.com/gorilla/mux" @@ -77,14 +78,27 @@ func router(ctx context.Context, logger *zap.Logger, httpTriggerSet *HTTPTrigger return mr } -func serve(ctx context.Context, logger *zap.Logger, port int, tracingSamplingRate float64, httpTriggerSet *HTTPTriggerSet, resolver *functionReferenceResolver) { +func serve(ctx context.Context, logger *zap.Logger, port int, tracingSamplingRate float64, + httpTriggerSet *HTTPTriggerSet, resolver *functionReferenceResolver, displayAccessLog bool) { mr := router(ctx, logger, httpTriggerSet, resolver) url := fmt.Sprintf(":%v", port) http.ListenAndServe(url, &ochttp.Handler{ Handler: mr, - StartOptions: trace.StartOptions{ - Sampler: trace.ProbabilitySampler(tracingSamplingRate), + GetStartOptions: func(r *http.Request) trace.StartOptions { + // do not trace router healthz endpoint + if strings.Compare(r.URL.Path, "/router-healthz") == 0 { + return trace.StartOptions{ + Sampler: trace.NeverSample(), + } + } + if displayAccessLog { + logger.Info("path", zap.String("path", r.URL.Path), + zap.String("method", r.Method), zap.Any("header", r.Header)) + } + return trace.StartOptions{ + Sampler: trace.ProbabilitySampler(tracingSamplingRate), + } }, }) } @@ -202,6 +216,16 @@ func Start(logger *zap.Logger, port int, executorUrl string) { zap.Float64("default", tracingSamplingRate)) } + displayAccessLogStr := os.Getenv("DISPLAY_ACCESS_LOG") + displayAccessLog, err := strconv.ParseBool(displayAccessLogStr) + if err != nil { + displayAccessLog = false + logger.Error("failed to parse 'DISPLAY_ACCESS_LOG' - set to the default value", + zap.Error(err), + zap.String("value", displayAccessLogStr), + zap.Bool("default", displayAccessLog)) + } + triggers, _, fnStore := makeHTTPTriggerSet(logger.Named("triggerset"), fmap, frmap, trmap, fissionClient, kubeClient, executor, restClient, &tsRoundTripperParams{ timeout: timeout, timeoutExponent: timeoutExponent, @@ -218,5 +242,5 @@ func Start(logger *zap.Logger, port int, executorUrl string) { logger.Info("starting router", zap.Int("port", port)) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - serve(ctx, logger, port, tracingSamplingRate, triggers, resolver) + serve(ctx, logger, port, tracingSamplingRate, triggers, resolver, displayAccessLog) } diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index b844d7a6..fca5c57e 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -99,7 +99,7 @@ func TestRouter(t *testing.T) { tracingSamplingRate := .5 ctx, cancel := context.WithCancel(context.Background()) defer cancel() - go serve(ctx, logger, port, tracingSamplingRate, triggers, frr) + go serve(ctx, logger, port, tracingSamplingRate, triggers, frr, false) time.Sleep(100 * time.Millisecond) // hit the router