runtime/pprof: generate profiles in gzipped proto format

[matloob]

[edit (3 Oct 2016) we now propose to just return the profiles in the serialized proto format. The proposal previously also gzipped the proto]

I propose to add a function
func runtime/pprof.StartCPUProfileProto(w io.Writer) error
and support in the runtime, for generating profiles in serialized profile protocol buffers, rather than in the legacy pprof format.

The profile proto is more extensible than the legacy pprof format and is pprof's preferred profile format. The extensibility will be useful to support generating profiles with labels. pprof supports rendering labeled profiles, but labels can only appear in the proto profile, not in the legacy format.

The profile proto format is already understood by pprof itself and pprof already includes an encoder for producing serialized profile protos without requiring a dependency on the proto package or library.

Here's the pprof proto description: https://github.com/google/pprof/blob/master/proto/profile.proto
and here's the encoder: https://github.com/google/pprof/blob/master/profile/proto.go
For more about labels, take a look at the https://github.com/google/pprof/blob/master/doc/developer/profile.proto.md#labels

[hyangah]

@rsc @rauls5382 @aclements

[rauls5382]

I agree the runtime should generate profile.proto directly. I would further propose to deprecate the legacy pprof formats.

For pprof to retrieve profiles in profile.proto format, the net/http/pprof package would also have to be changed to serve the profiles in that format.

[aclements]

I definitely agree that the runtime should generate profiles in the protocol buffer format. However, I don't think you need to add a new API: just replace the legacy format with the protobuf format. My understanding is that that was the idea from the start with the protobuf format. I believe @rsc even prototyped part of the runtime support just to make sure we weren't backing ourselves into a corner.

[matloob]

Oh, that's even better!

[DarKol13]

I'm interested in the switch to the new format too and could participate in it, I don't want to duplicate efforts, so, could you tell, is any job done?

[matloob]

@DarKol13 I haven't started any work yet. Feel free to contribute. Is there any part in particular you're interested in?

[DarKol13]

@matloob How can I contact you to discuss it in real-time?

[matloob]

Send me an email off thread to (github username) @golang.org and we can
coordinate there.
On Fri, Aug 19, 2016 at 7:27 AM DarKol13 notifications@github.com wrote:

@matloob https://github.com/matloob How can I contact you to discuss it
in real-time?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#16093 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/APtQJc-AhgOkLrygbNSPrU3pkzHtngLuks5qhZMRgaJpZM4I4dy7
.

[rhysh]

What are your plans for how the information included in the profiles will change?

1. Will they include full symbol info (function, file, line), or none at all?
2. Will they set the build_id field .. and will that be the contents of .note.gnu.build-id if present, or the Go toolchain's build id?
3. CPU profiles on linux now include the contents of /proc/self/maps. Will this continue, and extend to non-CPU profiles as well?
4. Where will labels be used?

[matloob]

@rhysh We will not drop any of the information currently included in the profile.

We're going to start off using some code we've open-sourced from Google that converts from the legacy format to the proto format. You can take a look at in patch set 1 of golang.org/cl/28349. See package internal/protopprof.

1. Profiles will include symbol data.
2. I don't think we've been setting that field in the newly open sourced code, but I don't see why not.
3. It will continue. I don't see why it shouldn't extend to non-CPU profiles, but maybe someone else can comment on that.
4. We're going to have to add some mechanism to the runtime for providing label annotations, but I don't know how that will work. I'm working on a proposal, but it's very much open for now. Once the facility is there users will be able to attach additional annotations to profile samples. The go pprof command already knows how to display them. When you get a graphical profile, little "3-d" boxes will appear coming out with each label. Here's a picture I grabbed: https://goo.gl/photos/5WWKPCpXFsyWDGRN7

[robpike]

You're going to put gzip very low in the dependency chain, which doesn't strike me as a good idea. Can you do this without gzip?

[ianlancetaylor]

Or perhaps just the gzip part could most to net/http/pprof where it is most useful.

[matloob]

Yes, we can! pprof understands the ungzipped profiles.

I just talked to @rauls5382 who had the same idea as @ianlancetaylor: runtime/pprof returns uncompressed protos, and the http handler gzips it.

[adg]

I'd like to see how much code this involves. The proto seems non-trivial, and so the encoder is likely non-trivial too, but I'm prepared to be pleasantly surprised.

[robpike]

It would be nice to split this into two separate proposals: 1) switching to protobuf format. 2) gzipping the result.

[matloob]

@adg Most of the code for working with the proto is already in the go tree. For example the core of the encoder code is here: https://golang.org/src/cmd/internal/pprof/profile/proto.go

@robpike Renaming this proposal now. It'll now just be (1).

[gopherbot]

CL https://golang.org/cl/30556 mentions this issue.

[rsc]

On-disk pprof profiles are defined to be gzipped, never ungzipped. We were very clear about this when we wrote the spec for them. If pprof accepts ungzipped, that's arguably a bug in pprof. Go should only ever generate the gzipped form.

That said, I don't think putting a gzip dependency in runtime/pprof is too big a problem: runtime/pprof (the package you import to actually get the profiles out of the runtime) is not a low-level package. I would be concerned if runtime itself grew a gzip dependency, but runtime/pprof is OK.

[rsc]

Sounds like the proposal was accepted earlier in the thread; just adding the label to reflect that.

[gopherbot]

CL https://golang.org/cl/32257 mentions this issue.

[gopherbot]

CL https://golang.org/cl/32353 mentions this issue.

[gopherbot]

CL https://golang.org/cl/32811 mentions this issue.

[gopherbot]

CL https://golang.org/cl/33071 mentions this issue.

[gopherbot]

CL https://golang.org/cl/33422 mentions this issue.