grafana / pyroscope

Continuous Profiling Platform. Debug performance issues down to a single line of code
https://grafana.com/oss/pyroscope/
GNU Affero General Public License v3.0
10.14k stars 614 forks source link

feat: support submodules for github integration #3531

Closed alsoba13 closed 2 months ago

alsoba13 commented 3 months ago

In this PR we extend the VCS api to accept a new rootPath field for the ServiceVersion. This will let some specific repository setups to effectively find sources with the GitHub Integration. This change is backwards compatible as the absence of rootPath is handled.

Understanding the need

Sometimes, the application that generates profiling data has no context on where the code was located inside the repository. This makes it difficult for the GitHub Integration to locate those sources.

A common case is when an app has been built with Dockerfile. The sources/binaries are placed somewhere inside the image and are relative to the Dockerfile. The resulting profile data will refer to those image sources that are not really matching repository paths.

If the Dockerfile is not at the root path of the repository, it's very hard to precisely determine which sources we are referring to inside the repository. This may be the case for large repositories with submodules as independent applications.

With the rootPath field, we give a hint on where the sources may be found inside the repository. Most of the cases it will be used to point to the path of that module.

Changes in frontend

Fronted will need to consume the new version of the API by extracting the root path from the pprof data. This change may be found at:

Changes in instrumented code

Users may define a root path for their profile data in the same way they were giving other information on service version. An example of it may be found at rideshare example

Changes in documentation

I've proposed those changes in the doc:

I still have to look for some checks there. On the other hand, that documentation should not be merged until these changes hit cloud instances.

CLAassistant commented 3 months ago

CLA assistant check
All committers have signed the CLA.

knylander-grafana commented 2 months ago

I love that you include the documentation! Thank you!