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/go: the trace command should not contain hardcoded javascript #16377

Open
jcorbin opened this Issue Jul 14, 2016 · 7 comments

Comments

Projects
None yet
6 participants
@jcorbin

jcorbin commented Jul 14, 2016

Please answer these questions before submitting your issue. Thanks!

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

Latest master 1.7.

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

n/a, but darwin/linux amd64.

  1. What did you do?

I originally tried to use go tool trace in 1.6:

  • found that the bundled catapult trace viewer was so old as to need a deprecated experimental browser api
  • followed the thread from misc/trace/README.md
  • after copying in a newer trace viewer bundle, it turned out that the api had shifted so much that the inline javascript was now broken

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.

  1. What did you expect to see?

Principled separation between frontend code and server-side code.

  1. 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

This comment has been minimized.

Contributor

josharian commented Jul 14, 2016

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

This comment has been minimized.

Contributor

josharian commented Jul 14, 2016

But maybe I misunderstand the issue.

@jcorbin

This comment has been minimized.

jcorbin commented Jul 14, 2016

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

This comment has been minimized.

Contributor

josharian commented Jul 14, 2016

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): josharian@a5a9f75. 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).

@josharian josharian changed the title from The `go tool trace` command should not contain hardcoded javascript. to cmd/go: the trace command should not contain hardcoded javascript Jul 14, 2016

@josharian josharian added this to the Go1.8 milestone Jul 14, 2016

@jcorbin

This comment has been minimized.

jcorbin commented Jul 14, 2016

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

This comment has been minimized.

gopherbot commented Sep 30, 2016

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

@quentinmit quentinmit added the NeedsFix label Oct 6, 2016

@rsc

This comment has been minimized.

Contributor

rsc commented Oct 21, 2016

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.

@rsc rsc modified the milestones: Go1.9, Go1.8 Oct 21, 2016

@ALTree ALTree modified the milestones: Unplanned, Go1.9 Jun 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment