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

stack: export new ParseStack function #51

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

derekperkins
Copy link
Contributor

fixes #50

@codecov
Copy link

codecov bot commented Nov 6, 2020

Codecov Report

Merging #51 (fc5267e) into master (87058aa) will increase coverage by 1.3%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #51     +/-   ##
========================================
+ Coverage    88.7%   90.0%   +1.3%     
========================================
  Files          12      12             
  Lines        4077    3460    -617     
========================================
- Hits         3615    3114    -501     
+ Misses        397     281    -116     
  Partials       65      65             
Impacted Files Coverage Δ
stack/location_string.go 33.3% <0.0%> (-6.7%) ⬇️
stack/webstack/webstack.go 90.6% <0.0%> (-5.0%) ⬇️
stack/reader.go 87.8% <0.0%> (-3.4%) ⬇️
internal/main.go 40.9% <0.0%> (-2.6%) ⬇️
internal/ui.go 59.2% <0.0%> (-2.1%) ⬇️
stack/source.go 87.5% <0.0%> (-1.9%) ⬇️
stack/html.go 89.1% <0.0%> (-1.4%) ⬇️
stack/context.go 85.1% <0.0%> (-1.2%) ⬇️
stack/stack.go 84.1% <0.0%> (-0.7%) ⬇️
stack/bucket.go 100.0% <0.0%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87058aa...fc5267e. Read the comment docs.

@derekperkins
Copy link
Contributor Author

If you're ok with adding this api, I'm happy to add a test

@maruel
Copy link
Owner

maruel commented Nov 6, 2020

Can you expand on the use case? I'm not sure it is a frequent need, worth increasing the API surface. I generally prefer small API surface with o/nly one way of doing something", and I feel this would create two ways of doing one thing that is all in all, relatively trivial.

As such, I'd very like much see a new example in example_test.go named Example_simple() that would essentially be the code you put. Note that ScanSnapshot() will never return a *Snapshot without at least one goroutine in it so you can simplify the code there.

@derekperkins
Copy link
Contributor Author

Before going further, I actually went ahead and tested this and it fails with an EOF error. I'm passing in debug.Stack() as the bytes that looks like this.

goroutine 11 [running]:
runtime/debug.Stack(0x1bd7be0, 0x0, 0x1c34860)
	/usr/local/Cellar/go/1.15.3/libexec/src/runtime/debug/stack.go:24 +0x9f
go.nozzle.io/pkg/e.TestParseStack.func1(0xc000602d80)
	/Users/derek/go/src/go.nozzle.io/pkg/e/stack_test.go:406 +0x45
testing.tRunner(0xc000602d80, 0xc00020e730)
	/usr/local/Cellar/go/1.15.3/libexec/src/testing/testing.go:1123 +0xef
created by testing.(*T).Run
	/usr/local/Cellar/go/1.15.3/libexec/src/testing/testing.go:1168 +0x2b3

@derekperkins
Copy link
Contributor Author

Scratch that, I didn't realize that the io.EOF error was expected to be returned. After ignoring that error, things are working just as expected. I'll change this PR to be an example.

@maruel
Copy link
Owner

maruel commented Nov 7, 2020

Thanks!

@derekperkins
Copy link
Contributor Author

Just pushed it as an example. We rolled this out into production over the weekend and it's working great! Thanks for the awesome library!

@maruel
Copy link
Owner

maruel commented Nov 12, 2020

Thanks!

@maruel maruel merged commit b846486 into maruel:master Nov 12, 2020
@maruel
Copy link
Owner

maruel commented Nov 12, 2020

I'll have to do a dot release for it to show up on pkg.go.dev, will do one before EOY.

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.

How to parse a stacktrace?
2 participants