Skip to content
This repository has been archived by the owner on Oct 29, 2021. It is now read-only.

traceapp: add trace permalinks which encode the trace within the URL #154

Merged
merged 3 commits into from
Apr 27, 2016

Conversation

emidoots
Copy link
Member

@emidoots emidoots commented Apr 26, 2016

This change adds a secondary button next to "Copy as JSON" (now renamed to "Export as JSON") called "Permalink" which gives you a perminant link to the trace. It encodes the entire trace within the URL query such that it can be viewed at any point in the future.

This change adds a secondary button next to "Copy as JSON" (now renamed
to "Export as JSON") called "Permalink" which gives you a perminant link
to the trace. It encodes the entire trace within the URL query such that
it can be viewed at any point in the future.
@emidoots emidoots force-pushed the sg/2-trace-permalinks branch from f45464e to 0f0d431 Compare April 26, 2016 19:41

// uploadTraces uploads literal traces into the storage system for later viewing.
func (a *App) uploadTraces(traces ...*appdash.Trace) error {
// Collect the unmarshaled traces, ignoring any previously existing ones (i.e.
Copy link
Member

Choose a reason for hiding this comment

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

cool, so if you click on a trace permalink for something that still exists in storage, this still work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, this exact case is handled by the first a.Store.Trace call in the for loop below.

@keegancsmith
Copy link
Member

Have you tried this out on the traces from Sourcegraph? Wondering what limitations encoding the entire trace in the URL has (fairly certain there are length constraints). How easy would it to indicate to the storage system to store something for a long time?

Otherwise, LGTM

@emidoots
Copy link
Member Author

emidoots commented Apr 27, 2016

Regarding query string length limits, http://stackoverflow.com/a/812962/5905344 is a pretty extensive answer which I found. Another example of a system that stores (albeit somewhat smaller) payloads in query strings would be https://play.rust-lang.org

Signaling to the storage system to keep data around isn't impossible, but it does add some overhead and adds some additional complexity (much more than this change does) so I'd like to avoid it for now if possible.

I will test on Sourcegraph traces before merging.

Update: Tested on some of the Sourcegraph traces and it works fine, albeit generating length (7k character) URLs :) We can improve this more in the future.

@emidoots emidoots merged commit 9d4aef8 into master Apr 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants