Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

feat(frontend): add single view page #611

Merged
merged 16 commits into from
Apr 10, 2023

Conversation

eh-am
Copy link
Collaborator

@eh-am eh-am commented Apr 4, 2023

Adds a simple UI page similar to Pyroscope's "Single View".

Screen.Recording.2023-04-04.at.17.09.31.mov

Breakdown:

  • Tried to use as much from pyroscope-oss as possible. This is so that we have a single "source of truth". As requirements come up we will definitely diverge from this approach.

  • It works by importing the pyroscope-oss git repo as a npm dependency. Then we refer to the files using typescript's path/alias functionality.

  • Certain files are overriden using the same path/alias functionality. One problem is that these overrides need to be set explicitly in both tsconfig.json and webpack.common.js. I tried to unify them using tsconfig-paths-webpack-plugin, but it didn't work when you have 2 definitions for the same file (for example a catch all then a more specific rule). It's okay for now, since it's just an intermediate step until we reimplement the entire functionality here, and therefore these overrides won't be necessary anymore.

  • To query application names we use /pyroscope/label-values?label=__name__ which doesn't translate very well to phlare's storage/query system since there's no concept of "Applications". Need some guidance on how to approach this.

  • Tags don't work.

  • It doesn't infer units/spyName correctly, for units it's technically possible, just need to map based on the "app name". For example, process_cpu:samples:count:cpu:nanoseconds is samples, memory:alloc_space:bytes:: is bytes etc.

@eh-am eh-am changed the title [WIP] feat:add single view UI feat:add single view UI Apr 6, 2023
@eh-am eh-am changed the title feat:add single view UI feat(frontend): add single view page Apr 6, 2023
@eh-am eh-am marked this pull request as ready for review April 6, 2023 12:35
Copy link
Collaborator

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

I have given that a try locally and all seems to have worked as described by you.

Thanks for the hardwork, great to see things coming together 🙂

.github/workflows/frontend.yaml Outdated Show resolved Hide resolved
Co-authored-by: Christian Simon <simon@swine.de>
@petethepig petethepig merged commit a2a8308 into grafana:main Apr 10, 2023
@Rustin170506
Copy link
Contributor

@petethepig Maybe we should use squash as merge to merge PR in this repo. Since that's what we've done before. 😄

@Rustin170506
Copy link
Contributor

Can I ask how it relates to the original UI of the phare and the UI of the pyroscope? In what way will they be combined or coexist in the future? Thanks!

simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
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.

4 participants