golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.14k stars 17.69k forks source link

cmd/trace: the trace command should not contain hardcoded javascript #16377

Open jcorbin opened 8 years ago

jcorbin commented 8 years ago

Please answer these questions before submitting your issue. Thanks!

1) What version of Go are you using (go version)?

Latest master 1.7.

2) What operating system and processor architecture are you using (go env)?

n/a, but darwin/linux amd64.

3) What did you do?

I originally tried to use go tool trace in 1.6:

From there I started hacking on my copy of the go 1.6 source, eventually I got a hold of the 1.7 source and found that at least all of the assets had already been upgraded (now including a polyfill for the ill-fated Object.observe).

However the future surface area fur such drift had only increased since 1.6; compare:

This is truly unfortunate since the only dynamic part of that entire "template" is a URL parameter for the XHR request to load data.

4) What did you expect to see?

Principled separation between frontend code and server-side code.

5) What did you see instead?

A hardcoded html+javascript "template" (not even an html/template

I have a proposed change that I'm working to get into Gerrit now.

josharian commented 8 years ago

There is a significant advantage to having a single file contain everything, including data, javascript, etc. A standalone file can easily be shared and does not require any special tools to open. If anything, I'd like to see the trace viewer go the other direction—rather than generate a file with the data but still require 'go tool trace' to be running to serve js and assets, I think go tool trace should spit out standalone output.

I plan to add a -trace flag to cmd/go for 1.8, and in that context, a standalone file is even more important for usability.

The bitrot vs the upstream trace viewer code is unfortunate and should be fixed. That's just something we have to maintain.

josharian commented 8 years ago

But maybe I misunderstand the issue.

jcorbin commented 8 years ago

Indeed, it'd be great to have a different mode for go tool trace that just spits out a data file to load into trace viewer out of band; maybe go tool trace && curl .../jsontrace would suffice fro now, but something like a go tool trace -out flag would be a bit better. I did consider opening a separate issue about the higher level usage pattern concern. This issue is more towards "if you're going to do these sorts of things, here's a more maintainable way to do X" (gerrit submission coming Soon ™).

josharian commented 8 years ago

We could have go tool trace launch a browser that opens a file written to a temporary location, and go tool trace -out <path> just write the file. Then at least we'd only have a single trace generation code path.

In case it's helpful, here's the crud I hacked together when I was last working on this (unstable url): https://github.com/josharian/go/commit/a5a9f7595338afbfd5f814956b17dc60e0bc586e. I'd love to see it done better.

I look forward to the gerrit CL. Just advance warning, we're in a code freeze, so nothing will go into the tree until it opens for 1.8 (in a few weeks, I think).

jcorbin commented 8 years ago

Okay got the gerrit cl submitted: https://go-review.googlesource.com/#/c/24916/ ; this is exciting as it's my first rodeo with a gerrit instance, looking forward to learning how this thing works :-)

gopherbot commented 8 years ago

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

rsc commented 8 years ago

I can't tell from the discussion here what the actual plan is. Sounds like it is not CL 24916. Leaving for Go 1.9.