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

cmd/pprof: Dots and slashes aren't allowed in profile names #13195

Closed
rhysh opened this issue Nov 9, 2015 · 8 comments
Closed

cmd/pprof: Dots and slashes aren't allowed in profile names #13195

rhysh opened this issue Nov 9, 2015 · 8 comments
Assignees
Milestone

Comments

@rhysh
Copy link
Contributor

@rhysh rhysh commented Nov 9, 2015

The docs for runtime/pprof.NewProfile suggest a particular naming format for custom profiles.

The convention is to use a 'import/path.' prefix to create separate name spaces for each package.

The cmd/pprof tool does not load profiles named with that convention, giving the error "parsing profile: unrecognized profile format".

I'd expect go tool pprof to successfully load profiles made using the runtime/pprof package and its instructions. The regexp that's supposed to match custom profiles, \A(\w+) profile: total \d+\n\z, should match the format described in the runtime/pprof docs.

The full variety of import paths may be too much to support, but at the very least '.' and '/' should be allowed.

$ go version
go version devel +321a407 Fri Nov 6 15:16:28 2015 +0000 darwin/amd64

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Nov 9, 2015
@rsc rsc modified the milestones: Go1.9Early, Unplanned Dec 21, 2016
@rsc
Copy link
Contributor

@rsc rsc commented Dec 21, 2016

/cc @matloob

@Sajmani Sajmani self-assigned this Feb 6, 2017
@rsc
Copy link
Contributor

@rsc rsc commented Feb 6, 2017

Now that we've got support for generating binary profiles, we should use that for all the profiles we generate. Then the legacy parsing code here won't matter.

@josharian
Copy link
Contributor

@josharian josharian commented Feb 6, 2017

Never generating legacy profiles will take a bit more work; see e.g. #18641.

@rsc
Copy link
Contributor

@rsc rsc commented Feb 6, 2017

It's fine to generate them when debug=1. That's not the problem here.

Sajmani added a commit to Sajmani/pprof that referenced this issue Feb 7, 2017
In addition to the "goroutine" and "threadcreate" profiles,
Go code can generate custom profiles using the runtime/pprof package.
The user must name these profiles, and the docs recommend using the
convention "import/path" to avoid namespace conflicts.  This CL
updates the pprof tool to be able to parse legacy profiles whose types
contain slashes and other non-space characters.

This is the upstream fix for golang/go#13195.
This change will need to be mirrored to
github.com/golang/go/src/cmd/pprof/internal/profile/legacy_profile.go
@Sajmani
Copy link
Contributor

@Sajmani Sajmani commented Feb 7, 2017

Upstream PR pending review: google/pprof#81

@rauls5382
Copy link
Contributor

@rauls5382 rauls5382 commented Feb 7, 2017

I've merged the fix into the upstream pprof, and will include it in my upcoming rebase of golang pprof.

+1 to generating profiles in profile.proto format, though.

@Sajmani
Copy link
Contributor

@Sajmani Sajmani commented Feb 7, 2017

@Sajmani
Copy link
Contributor

@Sajmani Sajmani commented Feb 7, 2017

https://go-review.googlesource.com/36533 updates the runtime/pprof docs.

@bradfitz bradfitz closed this May 3, 2017
@golang golang locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.