pkg/bugtool: avoid pprof logs without gops - #5329
Conversation
Skip pprof collection when no gops address is configured. This prevents duplicate skip logs and false success messages. Fixes: cilium#5308 Signed-off-by: Ehtasham-Yasin <ehtasham.yasin.dev@gmail.com>
There was a problem hiding this comment.
This isn't needed anymore since this is checked in the caller
There was a problem hiding this comment.
Done — removed the redundant check since it’s now handled by the caller. Thanks!
There was a problem hiding this comment.
What do you think about adding check for si.info.GopsAddr == "" here before calling si.addGopsInfo() and si.addPProfInfo()? This is the only place those functions are called and we can remove any extra checks for si.info.GopsAddr == "" within them (such as in addGopsInfo and getPProf)
You could also print the error message here too if the gops addr wasn't provided
There was a problem hiding this comment.
Yes, that makes sense. I moved the GopsAddr == "" check to the caller before addGopsInfo() and addPProfInfo(), log the skip message there, and removed the redundant checks from the callees. Thanks for the suggestion.
Signed-off-by: Ehtasham-Yasin <ehtasham.yasin.dev@gmail.com>
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
will-isovalent
left a comment
will-isovalent
left a comment
There was a problem hiding this comment.
Hey, thanks for the PR. I really don't think we need a regression test for this. That seems like overkill. Could you drop the test? Then I think it should be good to merge.
bar234
left a comment
bar234
left a comment
There was a problem hiding this comment.
Happy with the changes for my comments. I agree with @will-isovalent, we don't need the extra test case
Signed-off-by: Ehtasham-Yasin <ehtasham.yasin.dev@gmail.com>
|
Thanks @will-isovalent and @bar234 — I removed TestGopsDisabledLogsOnce and its unused imports in 4371116. The production code is unchanged. |
3 participants
Fixes #5308
Description
Skip pprof collection when Tetragon has no gops address. This leaves the
single skip message from the existing gops collection path and prevents false
success messages for CPU and heap profiles.
Add a focused regression test for both reported log problems.
Checks:
go test ./pkg/bugtool -run '^TestGopsDisabledLogsOnce$' -count=1golangci-lint run ./pkg/bugtool/...gofmt -d pkg/bugtool/bugtool.go pkg/bugtool/bugtool_test.gogit diff --checkThe full
go test ./pkg/bugtoolcommand could not complete in the freshLinux container because the unrelated
TestFindMaps/BaseSensorMemlocktestrequires the generated
bpf/objs/bpf_exit_v511.oobject, which was notpresent.
AI assistance: Codex was used for code-path analysis, the guard and regression
test, running checks, and PR preparation. The final diff and test results were
shown to the contributor for review before submission.
Changelog