Add sample graph data when in dev mode #2698

hari45678 · 2025-03-20T17:59:29Z

Which problem is this PR solving?

part of [LFX] Upgrade charts and graphs #2534

Description of the changes

This PR adds an additional npm script npm run start:with-data script that dynamically imports the dependencies array from json file built by parsing the publicly available dataset on https://zenodo.org/records/13956078 using the parse-traces.js script.
Have also included the force focal service selection flow

How was this change tested?

Locally, manually

Checklist

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

Signed-off-by: Hariom Gupta <[email protected]>

hari45678 · 2025-03-20T18:02:22Z

There might be a slight coverage drop because this dynamic import thing is kinda tricky to test.

codecov · 2025-03-20T18:07:15Z

Codecov Report

Attention: Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.67%. Comparing base (ed72165) to head (4b4f0b5).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...jaeger-ui/src/components/DependencyGraph/index.jsx	96.87%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2698   +/-   ##
=======================================
  Coverage   96.67%   96.67%           
=======================================
  Files         255      255           
  Lines        7817     7853   +36     
  Branches     1965     2026   +61     
=======================================
+ Hits         7557     7592   +35     
  Misses        260      260           
- Partials        0        1    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

hari45678 · 2025-03-20T18:08:44Z

Will try adding some sort of test for these lines? Or can we use /* istanbul ignore next */?

hari45678 · 2025-03-20T18:13:10Z

Looks like that dynamic import still made it to bundle in the form of base64 encoding string. Would have to look into this

Edit: The JSON isn't included in bundle.

With empty JSON:

With whole JSON:

scripts/utils/parse-traces.js

packages/jaeger-ui/package.json

yurishkuro

please mention the capability in the README somewhere where we describe how to run in dev.

Signed-off-by: Hariom Gupta <[email protected]>

yurishkuro · 2025-03-21T17:26:36Z

packages/jaeger-ui/src/components/DependencyGraph/index.jsx

+      const selectedLayout = data.length > dagMaxNumServices ? 'sfdp' : 'dot';
+      this.setState({ selectedLayout });


shouldn't this logic be in a more generic place? It's not specific to the samples.

makes sense

This is required at this place as changing the graph data wouldn't supply the graph length information to the graph layout selector

The way I see the workflow is:

user picks which data to use (including from backend when just navigating to the tab)

the data load happens

once the data is loaded/available, but before it's displayed, a check is made for its size and the layout is modified as necessary (or disabled and user is forced to make a focal service selection)

All of the sample datasets should only happen in step 1

Currently there is this dropdown:

user picks which data to use (including from backend when just navigating to the tab)

If we go with this flow, wouldn't the user (in dev mode) be forced to select an initial dataset, even if they want to just see backend data?
As far as I understand, this is about moving backend data also to this dataset selection dropdown

no, just clicking on the tab should always go to the server first, no forced user selection (although it doesn't matter much since it's in dev only)

user is forced to make a focal service selection

Added to this ticket to track progress for this one

For the 1st one, I have centralized the layout selection logic

yurishkuro · 2025-03-21T17:28:00Z

scripts/utils/parse-traces.js

+ * results to an output JSON file.
+ *
+ * Instructions to run this script:
+ * 1. Prepare a directory containing OLTP trace JSON files.


did any of the public data sets already come in OTLP format? The comments could point to it.

Yes, will add a link

Signed-off-by: Hariom Gupta <[email protected]>

hari45678 · 2025-03-22T08:13:51Z

I have also included a fallback screen that would force user to select a focal service beyond 1200 nodes

hari45678 · 2025-03-22T08:30:34Z

please mention the capability in the README somewhere where we describe how to run in dev.

This, I haven't yet mentioned in README. Can you please confirm all sections that we should mention? Like, one section about how to run parse-traces.js script, one about the sample data picker in UI, and?

yurishkuro

Noticed one issue. When the backend does not return any dependency data, the screen just shows the error No service dependencies found, but the DAG Options are not visible so I can't select the sample dataset.

Otherwise lgtm, with minor tweaks. I booked another issue #2700

packages/jaeger-ui/src/components/DependencyGraph/DAG.tsx

packages/jaeger-ui/src/components/DependencyGraph/index.jsx

Signed-off-by: Hariom Gupta <[email protected]>

hari45678 · 2025-03-22T17:46:29Z

Noticed one issue. When the backend does not return any dependency data, the screen just shows the error No service dependencies found, but the DAG Options are not visible so I can't select the sample dataset.

Otherwise lgtm, with minor tweaks. I booked another issue #2700

Yes, this thing I noticed and left intentionally because there are 2 error states on that screen:

When the backend isn't running, the UI shows a Internal Server Error so ideally we shouldn't show the Sample Data selector there.
When there is no service dependency data from backend. I couldn't figure out if someone running Jaeger UI locally in dev mode would be able to reproduce, as after a while, the data shows up?

yurishkuro · 2025-03-22T19:05:31Z

When there is no service dependency data from backend. I couldn't figure out if someone running Jaeger UI locally in dev mode would be able to reproduce, as after a while, the data shows up?

sorry, I didn't follow. When there is no backend at all the UI throws internal errors. But when there is a backend, just no data for SDG, it's then this error about "no data" shows, but the same error also disables the options.

hari45678 · 2025-03-23T12:49:40Z

When there is no service dependency data from backend. I couldn't figure out if someone running Jaeger UI locally in dev mode would be able to reproduce, as after a while, the data shows up?

sorry, I didn't follow. When there is no backend at all the UI throws internal errors. But when there is a backend, just no data for SDG, it's then this error about "no data" shows, but the same error also disables the options.

I got your point, but I mean to say that "no data" error isn't persistent in dev mode, as dependency data eventually shows up after some seconds (and that enables the options again).

add graph test script and config

0aa5929

Signed-off-by: Hariom Gupta <[email protected]>

hari45678 requested a review from a team as a code owner March 20, 2025 17:59

hari45678 requested review from joe-elliott and removed request for a team March 20, 2025 17:59

hari45678 marked this pull request as draft March 20, 2025 18:12

hari45678 marked this pull request as ready for review March 20, 2025 18:19

yurishkuro reviewed Mar 20, 2025

View reviewed changes

scripts/utils/parse-traces.js Show resolved Hide resolved

yurishkuro reviewed Mar 20, 2025

View reviewed changes

scripts/utils/parse-traces.js Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 20, 2025

View reviewed changes

packages/jaeger-ui/package.json Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 20, 2025

View reviewed changes

add multiple sample datas

c476fda

Signed-off-by: Hariom Gupta <[email protected]>

yurishkuro reviewed Mar 21, 2025

View reviewed changes

fix build issue

712600c

Signed-off-by: Hariom Gupta <[email protected]>

yurishkuro added the changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements label Mar 21, 2025

yurishkuro changed the title ~~Add development graph data and script for parsing traces~~ Add sample graph data when in dev mode Mar 21, 2025

hari45678 added 2 commits March 22, 2025 05:53

centralize updateLayout logic

8148289

Signed-off-by: Hariom Gupta <[email protected]>

add max node limit

9cd44c1

Signed-off-by: Hariom Gupta <[email protected]>

yurishkuro reviewed Mar 22, 2025

View reviewed changes

packages/jaeger-ui/src/components/DependencyGraph/DAG.tsx Outdated Show resolved Hide resolved

packages/jaeger-ui/src/components/DependencyGraph/index.jsx Outdated Show resolved Hide resolved

resolve comments

4b4f0b5

Signed-off-by: Hariom Gupta <[email protected]>

yurishkuro approved these changes Mar 22, 2025

View reviewed changes

yurishkuro added this pull request to the merge queue Mar 22, 2025

Merged via the queue into jaegertracing:main with commit f1d2d74 Mar 22, 2025
9 checks passed

hari45678 deleted the sample-data-load branch March 24, 2025 13:50

		const selectedLayout = data.length > dagMaxNumServices ? 'sfdp' : 'dot';
		this.setState({ selectedLayout });

Add sample graph data when in dev mode #2698

Add sample graph data when in dev mode #2698

Uh oh!

Conversation

hari45678 commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

hari45678 commented Mar 20, 2025

Uh oh!

codecov bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hari45678 commented Mar 20, 2025

Uh oh!

hari45678 commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yurishkuro left a comment

Choose a reason for hiding this comment

Uh oh!

yurishkuro Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

hari45678 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

hari45678 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hari45678 Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurishkuro Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

hari45678 Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

hari45678 Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurishkuro Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

hari45678 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

hari45678 commented Mar 22, 2025

Uh oh!

hari45678 commented Mar 22, 2025

Uh oh!

yurishkuro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hari45678 commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurishkuro commented Mar 22, 2025

Uh oh!

Uh oh!

hari45678 commented Mar 23, 2025

Uh oh!

Reviewers

Assignees

hari45678 commented Mar 20, 2025 •

edited

Loading

codecov bot commented Mar 20, 2025 •

edited

Loading

hari45678 commented Mar 20, 2025 •

edited

Loading

yurishkuro Mar 21, 2025 •

edited

Loading

hari45678 Mar 21, 2025 •

edited

Loading

hari45678 Mar 22, 2025 •

edited

Loading

hari45678 commented Mar 22, 2025 •

edited

Loading