Support loading sources from localhost and sending post requests to servers on double click

[parttimenerd]

This PR adds the capability of setting a source URL for every function (via an optional sourceUrl array in the FuncTable). The URLs have to start with https://raw.githubusercontent.com/ or http://localhost and are used to load files.

Here the URL points to GitHub.

Futhermore, prefixing the URL with post| specifies that instead of loading the file at the passed URL, the profiler should instead send a POST request to the URL passing name, file, line and column information if present. This is important for coupling the UI with other UIs like IDEs using a server that passes the requests on.

These post| URLs are usually not usable when uploaded to a different server, so the post| URL can also have the format post|url|alternative: The alternative URL is only used if the origin of url and the loaded UI location differ, making a fallback option.

This is really important for imported profiles which might be viewed in a Firefox Profiler inside an IDE.

Deployment Preview

[codecov[bot]]

Codecov Report

Base: 88.56% // Head: 88.56% // No change to project coverage :thumbsup:

Coverage data is based on head (0fd216b) compared to base (64cdd7e). Patch has no changes to coverable lines.

:exclamation: Current head 0fd216b differs from pull request most recent head 84e2939. Consider uploading reports for the commit 84e2939 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4259   +/-   ##
=======================================
  Coverage   88.56%   88.56%           
=======================================
  Files         282      282           
  Lines       25333    25333           
  Branches     6817     6817           
=======================================
  Hits        22435    22435           
  Misses       2696     2696           
  Partials      202      202           

Impacted Files	Coverage Δ
src/components/stack-chart/index.js	97.77% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[julienw]

Hey @mstange , I flagged you for review on this, because I believe you know the code most. Please tell me if you'd like somebody from the team to take on the review instead.

[parttimenerd]

Maybe someone other should..

[parttimenerd]

I rebased it on the the current main branch.

[mstange]

Ok, let's look at these four features one by one. The four features being:

1. The ability to load source from a public server on the web (github) for Java/Kotlin functions where the source file name is known.
2. Same as 1, but for a local server.
3. post|: This sends the line and column numbers to the server, and I'm not sure why. Is it because of source maps, so that the "original file" can be found based on the location in the minified file?
4. 3 but with fallback to a public URL.

Let's talk about 1 first. For github we already support "special paths" of the syntax git:github.com/rust-lang/rust:library/core/src/intrinsics.rs:acbe4443cc4c9695c0b74a7b64b60333c990a400. For functions with a file name like that, the source view will fetch the source code from github. Example profile

Now let's talk about 2.

The profiler already has some support for fetching source code from a local server: If you specify a local symbol server (using ?symbolServer=http://127.0.0.1:1234), then the source view will request source from http://127.0.0.1:1234/source/v1 if the clicked function is from native code, i.e. if it has code addresses and an associated lib.

The JSON format for a /source/v1 request is currently designed for native code, not for source map lookups; it gets a code address, not a line+col location.

How would you feel about extending this path to cover your use case? The "native code" check is here: https://github.com/firefox-devtools/profiler/blob/0fd216b3f5593f964b3d79884e5b920e75be1905/src/utils/fetch-source.js#L55-L67

It could be removed, and we could contact the local server even if all we have is a file path.

As for 3, we should come up with an API that would also work for JS source maps for JS code profiled in Firefox. On the other hand, I think Julien's current plans for source map support involve doing the source map lookup inside the front-end and not via an API call to the browser, so maybe JS source map support in Firefox won't need an API.

For 2 with fallback, we also have an existing solution: If we have both a "special path" and a local symbol server, then we try the local symbol server's /source/v1 endpoint first, and then fall back to a public server based on the special path. For example, let's say you profiled some Rust code using samply and your profile shows some functions from a dependency, let's say from symbolic-debuginfo-10.2.0, as in this profile: https://share.firefox.dev/3YD2Qh1 Then the functions in the profile have a file property with a string like cargo:github.com-1ecc6299db9ec823:symbolic-debuginfo-10.2.0:src/function_builder.rs. This is a special path for which the Firefox profiler knows how to get the file from a public server on the internet, i.e. crates.io. But if there is a local symbol server, then we first contact /source/v1 and supply the path cargo:github.com-1ecc6299db9ec823:symbolic-debuginfo-10.2.0:src/function_builder.rs. If the server is running, then it tries to find a local file for that special path, such as ~/.cargo/registry/src/github.com-1ecc6299db9ec823/symbolic-debuginfo-10.2.0/src/function_builder.rs, and returns the content of that file. Otherwise, the server returns an error and the Firefox Profiler falls back to crates.io. You could do something similar in your server for the git: special paths.

For source maps with fallback, what would the fallback look like? Would you get the minified source code?

[parttimenerd]

Regarding 3:

This sends the line and column numbers to the server, and I'm not sure why.

This URL could be used to notify the IDE (in which FirefoxProfiler might be integrated), to show a specific code location. This is really important.

The profiler already has some support for fetching source code from a local server: If you specify a local symbol server (using ?symbolServer=http://127.0.0.1:1234), then the source view will request source from http://127.0.0.1:1234/source/v1 if the clicked function is from native code, i.e. if it has code addresses and an associated lib.

Some, but this quite hard to implement and to work with, usable only for native code where we know addresses, but not for e.g. Java code.

For source maps with fallback, what would the fallback look like? Would you get the minified source code?

This seems reasonable, but I'm not an expert in source maps.

[mstange]

Oh I see, 3 has nothing to do with source maps. Ok. So the API call is more about "I'm notifying you that the user is currently looking at a function which starts at this location" than it is about "Please give me the source code for a file which contains this random location of interest."

We could have a separate symbolServer API endpoint for location-of-interest notifications. It seems rather orthogonal to fetching source code.

[parttimenerd]

We could have a separate symbolServer API endpoint for location-of-interest notifications. It seems rather orthogonal to fetching source code.

No. Because this only uses the post URL when the user clicks normally, but uses it to get the source code if the user presses the shift key while clicking.

[mstange]

Sure, then let's make two different API calls for two different use cases: One API call for obtaining source code and a different API call to navigate the IDE.

We should rename the symbolServer parameter to apiServer so that the name doesn't trigger the wrong associations. Or maybe tolocalIntegrationServer, so that it doesn't get confused with profiler-server. Symbolication is only one of the things that the symbolServer used for, source code is another. "Navigation notification" could be another.

But a function's sourceUrl field is not the right place to say "please notify the apiServer when opening this function". I think some kind of global opening flag would be more appropriate. For example, maybe instead of /from-url/ we want to have a /from-ide/6235/abcdef/ entry point, where 6235 is the port number and abcdef the secret base path, and then the profile would be obtained from http://127.0.0.1:6235/abcdef/profile.json. Just an idea. Or maybe we just want to have another URL parameter called notifyForSourceNavigation. We could strip that parameter from the shared URL when a profile is uploaded.

[parttimenerd]

You're correct, I think this would be the logical next step after this PR: Refining and defining a proper IDE source.

[julienw]

Another idea: Because in the case of an IDE the profiler is embedded, maybe we could define extension points in the form of global functions. The embedder would define them, and the profiler would call them if present, at some specific moment. This would be documented and supported. We could also possibly have browser extensions taking advantage of this.

In this case, we could have a global function called gOnUserOpenFile for example. The embedder could inject anything. A local server may even be unnecessary in this case which could improve the computer security.

[parttimenerd]

I'm all in for defining proper extension points. We could also use this for custom marker types or search filters.

[parttimenerd]

The further I work with it, the less appealing my solution is. My new solution is to just use the normal source URL without any post| prefix. The new code makes a POST request first and if the return is not a simple "ok", then it does a GET request. This is far less intrusive and leads to no changes to other code that has to handle source URLs.