pass default period to define what means skipper to be slow

Signed-off-by: Sandor Szücs <[email protected]>
zalando · Sep 6, 2024 · fadd0fe · fadd0fe
1 parent 6e7c62a
commit fadd0fe
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 13 deletions.
diff --git a/go.mod b/go.mod
@@ -169,18 +169,10 @@ require (
 	go.opentelemetry.io/proto/otlp v1.3.1 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/automaxprocs v1.5.3 // indirect
-<<<<<<< HEAD
 	golang.org/x/mod v0.19.0 // indirect
 	golang.org/x/sys v0.23.0 // indirect
 	golang.org/x/text v0.17.0 // indirect
 	golang.org/x/tools v0.23.0 // indirect
-	golang.org/x/exp v0.0.0-20240314144324-c7f7c6466f7f // indirect
-=======
-	golang.org/x/mod v0.18.0 // indirect
-	golang.org/x/sys v0.21.0 // indirect
-	golang.org/x/text v0.16.0 // indirect
-	golang.org/x/tools v0.22.0 // indirect
->>>>>>> baa60cfa (feature: allow configuration for Go x/trace.FlightRecorder)
 	gonum.org/v1/gonum v0.8.2 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect

diff --git a/proxy/proxy.go b/proxy/proxy.go
@@ -368,6 +368,11 @@ type Params struct {
 	// FlightRecorderTargetURL is the target to write the trace
 	// to. Supported targets are http URL and file URL.
 	FlightRecorderTargetURL string
+
+	// FlightRecorderPeriod is the time.Duration that is used for
+	// a slow skipper. If skipper is detected to be slow it tries
+	// to write out a trace as configured by the FlightRecorderTargetURL.
+	FlightRecorderPeriod time.Duration
 }
 
 type (
@@ -464,6 +469,7 @@ type Proxy struct {
 	onPanicSometimes         rate.Sometimes
 	flightRecorder           *trace.FlightRecorder
 	flightRecorderURL        *url.URL
+	flightRecorderPeriod     time.Duration
 }
 
 // proxyError is used to wrap errors during proxying and to indicate
@@ -896,6 +902,7 @@ func WithParams(p Params) *Proxy {
 		onPanicSometimes:         rate.Sometimes{First: 3, Interval: 1 * time.Minute},
 		flightRecorder:           p.FlightRecorder,
 		flightRecorderURL:        frURL,
+		flightRecorderPeriod:     p.FlightRecorderPeriod,
 	}
 }
 
@@ -904,7 +911,7 @@ func (p *Proxy) writeTraceIfTooSlow(ctx *context) {
 		return
 	}
 
-	var d time.Duration
+	d := p.flightRecorderPeriod
 	if e, ok := ctx.StateBag()[filters.TraceName]; ok {
 		d = e.(time.Duration)
 	}

diff --git a/skipper.go b/skipper.go
@@ -2050,18 +2050,20 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
 	routing := routing.New(ro)
 	defer routing.Close()
 
+	frPeriod := defaultFlightRecorderPeriod
 	var fr *trace.FlightRecorder
 	if o.FlightRecorderTargetURL != "" {
 		fr = trace.NewFlightRecorder()
 
 		if o.FlightRecorderPeriod != 0 {
-			fr.SetPeriod(o.FlightRecorderPeriod)
-		} else {
-			fr.SetPeriod(defaultFlightRecorderPeriod)
+			frPeriod = o.FlightRecorderPeriod
 		}
+		fr.SetPeriod(frPeriod)
 
+		frSize := defaultFlightRecorderSize
 		if o.FlightRecorderSize != 0 {
 			fr.SetSize(o.FlightRecorderSize)
+			frSize = o.FlightRecorderSize
 		} else {
 			fr.SetSize(defaultFlightRecorderSize)
 		}
@@ -2071,9 +2073,10 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
 			log.Errorf("Failed to start FlightRecorder: %v", err)
 			fr.Stop()
 			fr = nil
+		} else {
+			log.Infof("FlightRecorder started with config (%s, %d) target: %s", frPeriod, frSize, o.FlightRecorderTargetURL)
 		}
 	}
-	log.Infof("FlightRecorder: %v", fr)
 
 	proxyFlags := proxy.Flags(o.ProxyOptions) | o.ProxyFlags
 	proxyParams := proxy.Params{
@@ -2105,6 +2108,7 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
 		PassiveHealthCheck:         passiveHealthCheck,
 		FlightRecorder:             fr,
 		FlightRecorderTargetURL:    o.FlightRecorderTargetURL,
+		FlightRecorderPeriod:       frPeriod,
 	}
 
 	if o.EnableBreakers || len(o.BreakerSettings) > 0 {