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: add support for 1.18 stack traces #74

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

MordFustang21
Copy link
Contributor

Go 1.18 introduced a ? to annotate an inaccurate value in stack traces. This code should resolve that let me know if there are any places I've missed or tests that I should add to better handle it.

Resolves #73

Copy link
Owner

@maruel maruel left a comment

Choose a reason for hiding this comment

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

This was actually broken in go1.17; https://go.dev/doc/go1.17#compiler but forgot to file a bug about it. Let's keep this change and reassign it to fix this separate bug.

stack/context.go Outdated Show resolved Hide resolved
stack/context_test.go Outdated Show resolved Hide resolved
stack/context.go Outdated Show resolved Hide resolved
stack/context.go Outdated
@@ -854,13 +855,18 @@ func parseArgs(line []byte) (Args, error) {
arg := Arg{IsOffsetTooLarge: true}
cur.Values = append(cur.Values, arg)
default:
inaccurate := bytes.HasSuffix(a, inaccurateQuestionMark)
if inaccurate {
a = a[:len(a)-1]
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry I just realized, for consistency, use -size(inaccurateQuestionMark) instead -1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

@codecov
Copy link

codecov bot commented Mar 17, 2022

Codecov Report

Merging #74 (f59ce80) into main (19b8520) will decrease coverage by 0.3%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main     #74     +/-   ##
=======================================
- Coverage   88.4%   88.1%   -0.3%     
=======================================
  Files         12      12             
  Lines       4259    4263      +4     
=======================================
- Hits        3765    3754     -11     
- Misses       423     433     +10     
- Partials      71      76      +5     
Impacted Files Coverage Δ
stack/stack.go 83.7% <ø> (-0.7%) ⬇️
stack/context.go 85.5% <100.0%> (-1.8%) ⬇️

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 19b8520...f59ce80. Read the comment docs.

@tommie
Copy link

tommie commented Apr 4, 2022

@maruel Thanks for writing this library. I'm using it to check for leaking goroutines in tests.

I just hit this question mark issue and was wondering if it will be made into a v2.2.1 release soon.

@maruel
Copy link
Owner

maruel commented Apr 4, 2022

No sorry I haven't done the release yet. I wanted to change cmd/panic/main.go so reduce recursiveness conditionally so I could make tests pass on 1.18. See linked bug.

tbg added a commit to tbg/cockroach that referenced this pull request May 6, 2022
This picks up maruel/panicparse#74.
Without this commit, the `pkg/kv/kvserver/concurrency` datadriven
tests are flaky on Mac M1s since those tests rely on panicparse
to parse strack traces for goroutine monitoring.

Release note: None
craig bot pushed a commit to cockroachdb/cockroach that referenced this pull request May 6, 2022
80422: sql: support virtual table indexes with generators r=AlexTalks a=AlexTalks

While previously virtual schema tables had support for defining tables
with a `populate` function, which eagerly loads all rows, or a
`generator` function, which lazy loads each row when called (possibly
running in a worker Goroutine), and also had support for virtual indexes
which would have their own `populate` functions, there was a subtle lack
of support for using virtual indexes with virtual tables that used a
`generator`, since the virtual index constraint logic would fall back to
a (possibly undefined) `populate` function in several cases. This change
fixes the nil pointer exception that could occur if using virtual
indexes with a table using a `generator`, and validates that the virtual
index is supported prior to use.

Release Note: None

Release Justification: Bug fix

80989: outliers: collect statement fingerprint id r=matthewtodd a=matthewtodd

This change also helps set us up for #79451, where we'll be working with
per-fingerprint statement latencies.

Release note: None

81076: Authors: add linville to authors r=davidwding a=mdlinville

Release note: None

81080: opt: calculate lookup join remaining filters more accurately r=mgartner a=mgartner

#### opt: return ordinals of equality filters from memo.ExtractJoinEqualityColumns

This is a prerequisite for future refactoring of
`lookupjoin.ConstraintBuilder`.

Release note: None

#### opt: calculate lookup join remaining filters more accurately

`lookupjoin.ConstraintBuilder` now determines the remaining filters for
a lookup join constraint more accurately by tracking the ordinals of
filters that are covered by the constraint, rather than trying to reduce
the filters original filters based on the key columns, lookup
expression, and constant expression.

Release note: None


81089: vendor: bump panicparse r=erikgrinaker,nvanbenschoten a=tbg

This picks up maruel/panicparse#74.
Without this commit, the `pkg/kv/kvserver/concurrency` datadriven
tests are flaky on Mac M1s since those tests rely on panicparse
to parse strack traces for goroutine monitoring.

Release note: None


Co-authored-by: Alex Sarkesian <sarkesian@cockroachlabs.com>
Co-authored-by: Matthew Todd <todd@cockroachlabs.com>
Co-authored-by: Matt Linville <linville@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
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.

Make panicparse compatible with go1.17 uncertain variables
3 participants