Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: flightrecorder to enable Go trace #2985

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

szuecs
Copy link
Member

@szuecs szuecs commented Mar 14, 2024

feature: flightrecorder to enable Go trace

https://go.dev/blog/execution-traces-2024
https://pkg.go.dev/golang.org/x/exp/trace#FlightRecorder

wdyt?

Ideas from discussion

  • enable flightrecorder by filter
  • write to http endpoint PUT (think of cluster local service that collects the trace and make it available or S3)
  • test: separate client and server similar to stdlib to benchmark overhead only in proxy

proxy/proxy.go Outdated
return
}
// Write it to a file.
if err := os.WriteFile("trace.out", b.Bytes(), 0o755); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are we going to fetch this file if needed? Manually copying it from the pod? Or is there some automation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cite "Ideas from discussion":

write to http endpoint PUT (think of cluster local service that collects the trace and make it available or S3)

proxy/proxy.go Outdated Show resolved Hide resolved
config/config.go Outdated
Comment on lines 387 to 382
flag.IntVar(&cfg.FlightRecorderSize, "flight-recorder-size", 0, "max flight-recorder trace data size")
flag.DurationVar(&cfg.FlightRecorderPeriod, "flight-recorder-period", 0, "sets the approximate time duration that the flight recorder's circular buffer represents.")
flag.DurationVar(&cfg.FlightRecorderProxyTookTooLong, "flight-recorder-proxy-took-too-long", 0, "sets the threshold, if proxy took longer than that the flight recorder will write out a trace.")
flag.StringVar(&cfg.FlightRecorderTargetURL, "flight-recorder-target-url", "", "sets the flight recorder target URL that is used to write out the trace to.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better as a filter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think it's better that people have to pass their own URL into the filter?

@szuecs szuecs force-pushed the feature/flight-recorder branch 2 times, most recently from b23465b to c42035a Compare March 20, 2024 18:56
@szuecs
Copy link
Member Author

szuecs commented Mar 20, 2024

I get only errors for Go 1.22 support:

+ compile_go_fuzzer github.com/zalando/skipper/fuzz/fuzz_targets FuzzParseRouteGroupsJSON FuzzParseRouteGroupsJSON gofuzz
/usr/local/bin/compile_go_fuzzer: line 28: cd: /root/go/src/github.com/zalando/skipper/fuzz/fuzz_targets: No such file or directory
go: downloading go1.22 (linux/amd64)
go: download go1.22 for linux/amd64: toolchain not available
go: downloading go1.22 (linux/amd64)
go: download go1.22 for linux/amd64: toolchain not available
/usr/local/bin/compile_go_fuzzer: line 32: cd: github.com/zalando/skipper@*: No such file or directory
/src/skipper
go: downloading go1.22 (linux/amd64)
go: download go1.22 for linux/amd64: toolchain not available
go: downloading go1.22 (linux/amd64)
go: download go1.22 for linux/amd64: toolchain not available
Running go-fuzz -tags gofuzz -func FuzzParseRouteGroupsJSON -o FuzzParseRouteGroupsJSON.a github.com/zalando/skipper/fuzz/fuzz_targets
2024/03/20 18:48:40 failed to load packages:go [-e -json -compiled=false -test=false -export=false -deps=false -find=true -buildmode c-archive -gcflags all=-d=libfuzzer -tags gofuzz,gofuzz_libfuzzer,libfuzzer,gofuzz -trimpath -gcflags syscall=-d=libfuzzer=0 -- github.com/zalando/skipper/fuzz/fuzz_targets]: exit status 1: go: downloading go1.22 (linux/amd64)
go: download go1.22 for linux/amd64: toolchain not available
2024-03-20 18:48:41,038 - root - ERROR - Building fuzzers failed.
2024-03-20 18:48:41,038 - root - ERROR - Error building fuzzers for (commit: 48f73d71fea2fbd879fcec320fae5e35443243d9, pr_ref: refs/pull/2985/merge).

Without forcing Go1.22 I get:

Running go-fuzz -tags gofuzz -func FuzzParseRouteGroupsJSON -o FuzzParseRouteGroupsJSON.a github.com/zalando/skipper/fuzz/fuzz_targets
# github.com/zalando/skipper/proxy
proxy/proxy.go:322:24: undefined: trace.FlightRecorder
proxy/proxy.go:424:40: undefined: trace.FlightRecorder
2024/03/20 18:58:30 failed to build packages:exit status 1
2024-03-20 18:58:33,169 - root - ERROR - Building fuzzers failed.
2024-03-20 18:58:33,169 - root - ERROR - Error building fuzzers for (commit: 368aca29104401b8a16a6e8c791e06026d22b1fb, pr_ref: refs/pull/2985/merge).

I don't really know why: https://pkg.go.dev/golang.org/x/exp/trace#FlightRecorder should not depend on Go version, because x/ is a separate package. Maybe it's really an issue created by go-fuzz. Maybe fixed by google/oss-fuzz#11677

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants