Skip to content
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

[Feature]: combine all graph views into one - LFX Mentorship #1466

Closed
yurishkuro opened this issue Jun 5, 2023 · 13 comments
Closed

[Feature]: combine all graph views into one - LFX Mentorship #1466

yurishkuro opened this issue Jun 5, 2023 · 13 comments

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Jun 5, 2023

For the application process please refer to this issue.

Requirement

As a user of Jaeger UI I want to have a consistent experience with service graph views.

Problem

Jaeger currently uses three distinct renderings for service graphs

  • the DAG view, good for small architectures (uses Cytoscape.js)
  • the System view good for large architectures (uses react-viz-force-graph)
  • the internal Plexus lib used for deep dependency graphs (uses D3 and graphviz rendering engine)

Proposal

To reduce the different code paths and provide a consistent user experience we should converge on a single graph visualization.

Plexus is the likely choice, but it needs to be benchmarked on large graphs (the System view can handle several thousand nodes). We can also consider server-side filtering (proposed in #784).

We may also have to address #1081 (deprecated graphing libs)

Some Evaluation Criteria

  • is there support for stable / deterministic layouts? This is mandatory for graph views of a single trace, where nodes are arranged according to time and depth.
  • how "future proof" is the library - actively supported, big contributor community, etc. (including its own dependencies). Are there big projects that use it?
  • how many new dependencies it pulls
  • can it handle large graphs (say up to 5k nodes)?
  • does it allow customization of the graph? Including labels on edges, different viz for notes, interaction with nodes (left/right click)
@yurishkuro yurishkuro changed the title [Feature]: combine all graph views into one [Feature]: combine all graph views into one - LFX Mentorship Jul 27, 2023
@punithnayak
Copy link

Hey @yurishkuro I want to work on this for the upcoming LFX mentorship . Will go through good first Issues and suggest improvements if needed

@sivasathyaseeelan
Copy link

Hi @yurishkuro, I am very interested and definitely apply for this project in the upcoming LFX mentorship!!

@shashwatm1111
Copy link

Hey @yurishkuro , would love to contribute to this project under the LFX mentorship . Also going through the first good issues and will work upon them .

@yurishkuro
Copy link
Member Author

@shashwatm1111 welcome

@Nupoor10
Copy link

Nupoor10 commented Aug 3, 2023

Hello @yurishkuro, I would love to get involved with this project. I am currently getting familiar with Jaeger and Plexus. Looking forward to making an amazing contribution!

@ShivangRawat30
Copy link

Hello @yurishkuro, I would love to contribute to this project under the LFX mentorship.

@PranitPatil03
Copy link

Hello @yurishkuro👋

I'm Pranit Patil, a student pursuing a Bachelor's degree in Computer Science at Mumbai University. Recently, I completed my internship at hubx.ai, where I worked as a full-stack developer intern. Prior to that, I gained experience as a front-end developer intern at Tech Cryptors.

My skills include JavaScript, ReactJS, Node, Express, MongoDB, and CSS libraries, such as Bootstrap. Through various projects, including full-stack ones, I have honed my expertise in full-stack development

I'm keen on participating in the LFX mentorship program and becoming an active community member. I kindly request your assistance in guiding me through project discussions.

Thank you.

@yurishkuro yurishkuro pinned this issue Aug 13, 2023
@yurishkuro
Copy link
Member Author

@prathamesh-mutkure's application was selected for this project. Congratulations!

@anikdhabal
Copy link
Contributor

congrats @prathamesh-mutkure

@yashrsharma44
Copy link
Contributor

Plexus is the likely choice, but it needs to be benchmarked on large graphs (the System view can handle several thousand nodes). We can also consider server-side filtering (proposed in #784).

Do we have any sample(s) of data, which we can use to run the benchmarks against?

@yurishkuro
Copy link
Member Author

not to my knowledge. We have a storage integration test that saves a trace with 10k spans, but it just generates that on the fly. I would do the same here - a fake trace is not difficult to generate. It's a bit tricky to decide on the good way to connect different nodes so that we don't test on a contrived cases like a graph which is a single very long branch.

@yurishkuro
Copy link
Member Author

Another interesting lib I just stumbled upon in some internal tool: https://reactflow.dev/

@yurishkuro
Copy link
Member Author

Another alternative: SigmaJS with Graphology

yurishkuro added a commit that referenced this issue Dec 18, 2023
## Which problem is this PR solving?
- This issue is related to LFX task #1466 
- I can create a more specific sub-issue if that's more suitable

## Description of the changes
- I've removed `cytoscape` and replaced it with `Digraph` from `Plexus`
- Had to add support for labels in `Digraph` to support this change

## How was this change tested?
- The changes were tested visually
- Tested with test data involving around 50 nodes

Before - 

<img width="1785" alt="Screenshot 2023-11-18 at 22 06 10"
src="https://github.com/jaegertracing/jaeger-ui/assets/28570857/dd130e06-94d5-41b8-8217-7e0cdb442dc2">

Now -

<img width="1785" alt="Screenshot 2023-11-18 at 22 06 22"
src="https://github.com/jaegertracing/jaeger-ui/assets/28570857/6249207d-b1e6-4774-82ad-2f9e6a589f3e">

Tha additional tab visible in above images has been removed

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants