Add IgnoreCreatedBy (#135) #136
Conversation
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
|
|
WalkthroughThe changes introduce the ability to track and retrieve the creator function of a goroutine within the stack parsing logic. The Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #136 +/- ##
==========================================
+ Coverage 98.57% 98.61% +0.03%
==========================================
Files 5 5
Lines 351 360 +9
==========================================
+ Hits 346 355 +9
Misses 4 4
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
options_test.go (1)
74-95: Well-structured test for the new IgnoreCreatedBy option.The test effectively verifies that the
IgnoreCreatedByoption correctly filters out goroutines created by the specified function name. It follows the same pattern as other option tests in the file, which is good for consistency.The test properly manages resources by closing the channel to allow the goroutine to exit, preventing any leaks from the test itself.
Consider adding a brief comment at the beginning of the test explaining its purpose, similar to the comments on lines 67-69 for the
IgnoreAnyFunctiontest.func TestOptionsIgnoreCreatedBy(t *testing.T) { + // Test that IgnoreCreatedBy correctly filters goroutines created by the specified function. stopCh := make(chan struct{}) go func() { <-stopCh }() defer close(stopCh)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
options_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
options_test.go (2)
internal/stack/stacks.go (2)
Current(251-253)All(246-248)options.go (1)
IgnoreCreatedBy(91-95)
|
Bump - FYI the CI failure is golangci-lint complaining about the config. |
|
For the lint failure, we need to run |
sywhang
left a comment
There was a problem hiding this comment.
LGTM, please address the golangci-lint issue and we can merge.
|
The golangci-lint issue is that your workflow is running golangci-lint v2 but your config is v1.
I'd suggest you decide the migration of the golangci-lint issue within the project. |
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com>
f686317 to
cbb740d
Compare
abhinav
left a comment
There was a problem hiding this comment.
This looks good to me.
Will defer to Uber folks to merge.
Someone may need to upgrade to golangci-lint v2 on a separate branch.
I agree that that's out of scope for this PR.
|
ping pong. Just gentle reminder :) |
A golangci-lint failure was raised in #136 $ golangci-lint config verify ``` ⎿ Error: jsonschema: "linters-settings.govet.enable.0" does not validate with "/properties/linters-settings/properties/govet/properties/enable/items/$ref/enum": value must be one of 'appends', 'asmdecl', 'assign', 'atomic', 'atomicalign', 'bools', 'buildtag', 'cgocall', 'composites', 'copylocks', 'deepequalerrors', 'defers', 'directive', 'errorsas', 'fieldalignment', 'findcall', 'framepointer', 'httpresponse', 'ifaceassert', 'loopclosure', 'lostcancel', 'nilfunc', 'nilness', 'printf', 'reflectvaluecompare', 'shadow', 'shift', 'sigchanyzer', 'slog', 'sortslice', 'stdmethods', 'stdversion', 'stringintconv', 'structtag', 'testinggoroutine', 'tests', 'timeformat', 'unmarshal', 'unreachable', 'unsafeptr', 'unusedresult', 'unusedwrite', 'waitgroup' Failed executing command with error: the configuration contains invalid elements ``` After the fix $ golangci-lint config verify yields no errors.
A golangci-lint failure was raised in #136 $ golangci-lint config verify ``` ⎿ Error: jsonschema: "linters-settings.govet.enable.0" does not validate with "/properties/linters-settings/properties/govet/properties/enable/items/$ref/enum": value must be one of 'appends', 'asmdecl', 'assign', 'atomic', 'atomicalign', 'bools', 'buildtag', 'cgocall', 'composites', 'copylocks', 'deepequalerrors', 'defers', 'directive', 'errorsas', 'fieldalignment', 'findcall', 'framepointer', 'httpresponse', 'ifaceassert', 'loopclosure', 'lostcancel', 'nilfunc', 'nilness', 'printf', 'reflectvaluecompare', 'shadow', 'shift', 'sigchanyzer', 'slog', 'sortslice', 'stdmethods', 'stdversion', 'stringintconv', 'structtag', 'testinggoroutine', 'tests', 'timeformat', 'unmarshal', 'unreachable', 'unsafeptr', 'unusedresult', 'unusedwrite', 'waitgroup' Failed executing command with error: the configuration contains invalid elements ``` After the fix $ golangci-lint config verify yields no errors.
uber-go/goleak#136 has been merged so upstream can be used again. Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
uber-go/goleak#136 has been merged so upstream can be used again. Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Was looking for exactly that today.
Summary by CodeRabbit
New Features
Tests