[feature] Visualize Critical Path of a trace #1582

GLVSKiriti · 2023-07-14T14:13:44Z

This PR is to add support(UI) for critical path analysis to Jaeger UI.
Resolves #1288

Respective Algorithm PR is #1532
Related bug fixes through PRs #1780 #1785 #1871
@yurishkuro @yashrsharma44

We got trace data to implement critical path for a trace and we added TraceCriticalPath option Signed-off-by: GLVS Kiriti <[email protected]>

This function finds child spans of each span Signed-off-by: GLVS Kiriti <[email protected]>

Instead we critical path algotihm runs on rendering TracePage and then we can visualize critical path Signed-off-by: GLVS Kiriti <[email protected]>

Here in criticalPath array all critical Path sections of each span are added to it Signed-off-by: GLVS Kiriti <[email protected]>

As it is new file license should be added Signed-off-by: GLVS Kiriti <[email protected]>

Signed-off-by: GLVS Kiriti <[email protected]>

…race Signed-off-by: GLVS Kiriti <[email protected]>

Signed-off-by: GLVS Kiriti <[email protected]>

Test passed successfully Signed-off-by: GLVS Kiriti <[email protected]>

And also structured all required data for tests in test1 test2 js files only Signed-off-by: GLVS Kiriti <[email protected]>

This function turncates or drops the child spans if they start/end (overflow) before/after parent span Signed-off-by: GLVS Kiriti <[email protected]>

In this case child span is entirely outside the parent span so it is dropped Signed-off-by: GLVS Kiriti <[email protected]>

Signed-off-by: GLVS Kiriti <[email protected]>

Now it also drops the child spans of already dropped spans Signed-off-by: GLVS Kiriti <[email protected]>

Signed-off-by: GLVS Kiriti <[email protected]>

Now only everyTime the input changes critical path algorithm is computed Signed-off-by: GLVS Kiriti <[email protected]>

Signed-off-by: GLVS Kiriti <[email protected]>

packages/jaeger-ui/src/components/TracePage/CriticalPath/utils/sanitizeOverFlowingChildren.tsx

yashrsharma44 · 2023-08-19T12:58:51Z

I took a look at the PR, and while we are addressing the changes, let's gate this feature(Critical Path Algorithm Component) using a config value - https://github.com/jaegertracing/jaeger-ui/blob/fabd2e756113b8bfdc2103c657ee2915b35fa56d/packages/jaeger-ui/src/constants/default-config.tsx

We can disable this feature to start with, and add the option to enable this, if people want to try this feature. We can merge this PR while keeping this feature gated, and add some follow up items to improve the space complexity of this algorithm. I can suggest more review items, but I think this would make this PR more lengthier than I would like.

As we have comprehensive unit-tests, we can iterate on improving the space complexities of the individual functions without breaking the logical behaviour(one of the perks of having unit-test :))

packages/jaeger-ui/src/components/TracePage/CriticalPath/index.test.js

Signed-off-by: GLVS Kiriti <[email protected]>

Signed-off-by: Yuri Shkuro <[email protected]>

Signed-off-by: GLVSKiriti <[email protected]>

Signed-off-by: GLVS Kiriti <[email protected]>

bobrik · 2023-09-07T18:13:50Z

I tried out the jaeger v1.49.0 and it doesn't seem to show what I would expect to see:

I would expect to see request to be a part of the critical path here.

bobrik · 2023-09-07T18:25:30Z

Manually adjusting duration in a json file doesn't help:

yurishkuro · 2023-09-07T18:37:06Z

is this a follows-from span kind? in the first s/s it was after the parent span so not on the critical path of THAT span, and CP is defined recursively starting from the root span, at least in the current version.

yurishkuro · 2023-09-07T18:39:59Z

btw, glad to see you're trying it out, bug reports are very welcome (maybe start another ticket).

@yurishkuro

## Which problem is this PR solving? - Related to #1582 ## Description of the changes - Added Tooltip which is visible on hovering the critical path @yurishkuro --------- Signed-off-by: GLVS Kiriti <[email protected]> Signed-off-by: GLVSKiriti <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

GLVSKiriti and others added 30 commits June 30, 2023 18:11

Added new option TraceCriticalPath

ee7ee31

We got trace data to implement critical path for a trace and we added TraceCriticalPath option Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'main' into criticalPathFeature

3b4719c

findChildSpanIds functions is added

609c417

This function finds child spans of each span Signed-off-by: GLVS Kiriti <[email protected]>

Critical path is now not a new view in UI

5d54b8f

Instead we critical path algotihm runs on rendering TracePage and then we can visualize critical path Signed-off-by: GLVS Kiriti <[email protected]>

ComputeCriticalPath function can find critical path

29dc918

Here in criticalPath array all critical Path sections of each span are added to it Signed-off-by: GLVS Kiriti <[email protected]>

License is added on top of file

12107f6

As it is new file license should be added Signed-off-by: GLVS Kiriti <[email protected]>

Added a Unit Test for CriticalPath algorithm of a Normal Trace

177440b

Signed-off-by: GLVS Kiriti <[email protected]>

Licenses added to new files

a45f283

Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'main' into criticalPathFeature

a927b32

computeCritialPath function now returns critical path sections of a t…

853f160

…race Signed-off-by: GLVS Kiriti <[email protected]>

Updated to suggested changes

068405b

Signed-off-by: GLVS Kiriti <[email protected]>

Added unit tests and comments for helper functons

facc1fa

Signed-off-by: GLVS Kiriti <[email protected]>

Added a simulation diagram in comments for test1

cc98d5d

Signed-off-by: GLVS Kiriti <[email protected]>

Updated to sugested changes

5f058dd

Signed-off-by: GLVS Kiriti <[email protected]>

Added another test case for happy path

8e5d513

Test passed successfully Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'main' into criticalPathFeature

74f680e

Used descibe.each so we can add more unit tests within short length

db04fea

And also structured all required data for tests in test1 test2 js files only Signed-off-by: GLVS Kiriti <[email protected]>

Added sanitizeOverFlowingChildren function

014c11d

This function turncates or drops the child spans if they start/end (overflow) before/after parent span Signed-off-by: GLVS Kiriti <[email protected]>

Unit test added for a overflwing span case

4c55b72

In this case child span is entirely outside the parent span so it is dropped Signed-off-by: GLVS Kiriti <[email protected]>

Updated to suggested changes

011fac3

Signed-off-by: GLVS Kiriti <[email protected]>

Unit tests data fixtures are refined for ease

3310191

Signed-off-by: GLVS Kiriti <[email protected]>

sanitizeOverFlowingChildren function is updated

bc38511

Now it also drops the child spans of already dropped spans Signed-off-by: GLVS Kiriti <[email protected]>

Unit tests are updated and modified to suggested changes

18788d5

Signed-off-by: GLVS Kiriti <[email protected]>

if-else block changed to switch block

dc011ca

Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'main' into criticalPathFeature

228d6d3

Modified trace diagrams and added parent child tree in comments

36daeb3

Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'main' into criticalPathFeature

8610337

Added unit tests for util functions separately

e157ad8

Signed-off-by: GLVS Kiriti <[email protected]>

Used memoization technique in critical path computation

40f3822

Now only everyTime the input changes critical path algorithm is computed Signed-off-by: GLVS Kiriti <[email protected]>

Runned yarn prettier

b1e8960

Signed-off-by: GLVS Kiriti <[email protected]>

yashrsharma44 reviewed Aug 19, 2023

View reviewed changes

packages/jaeger-ui/src/components/TracePage/CriticalPath/utils/sanitizeOverFlowingChildren.tsx Outdated Show resolved Hide resolved

yashrsharma44 reviewed Aug 19, 2023

View reviewed changes

packages/jaeger-ui/src/components/TracePage/CriticalPath/index.test.js Outdated Show resolved Hide resolved

GLVSKiriti and others added 10 commits August 20, 2023 11:01

Modifying to suggested changes

7fe65be

Signed-off-by: GLVS Kiriti <[email protected]>

spawnTime parameter is renamed as returningChildStartTime

8a7c424

Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'criticalPathFeature' into uiBranch

5b767e0

fixing typos

a7560d6

Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'criticalPathFeature' into uiBranch

720c38b

Merge branch 'main' into uiBranch

50360a3

test file updated

46abe17

Signed-off-by: GLVS Kiriti <[email protected]>

Merge branch 'main' into uiBranch

f5856ee

Testfile updated for criticalPath in spanBar component

f281aef

Signed-off-by: GLVS Kiriti <[email protected]>

CriticalPath feature is gated

5361025

Signed-off-by: GLVS Kiriti <[email protected]>

GLVSKiriti requested a review from yashrsharma44 August 22, 2023 12:05

yurishkuro and others added 2 commits September 4, 2023 08:08

Make CP on by default

6f21604

Signed-off-by: Yuri Shkuro <[email protected]>

Merge branch 'main' into uiBranch

f78afde

Signed-off-by: GLVSKiriti <[email protected]>

yurishkuro previously approved these changes Sep 4, 2023

View reviewed changes

Merge conflicts resolve

c7a0151

Signed-off-by: GLVS Kiriti <[email protected]>

GLVSKiriti dismissed yurishkuro’s stale review via c7a0151 September 4, 2023 14:50

yurishkuro approved these changes Sep 4, 2023

View reviewed changes

yurishkuro merged commit 0c6349e into jaegertracing:main Sep 4, 2023

yurishkuro mentioned this pull request Sep 4, 2023

Implement Critical Path algorithm #1532

Closed

GLVSKiriti deleted the uiBranch branch September 12, 2023 07:44

GLVSKiriti mentioned this pull request Oct 15, 2023

Add tooltip when hovering Critical Path #1871

Merged

svrnm mentioned this pull request Jun 17, 2024

We can't accurately calculate the critically blocking path for async services. (Should we add previous_sibling_span_id as an optional span attribute?) open-telemetry/opentelemetry-specification#4079

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Visualize Critical Path of a trace #1582

[feature] Visualize Critical Path of a trace #1582

GLVSKiriti commented Jul 14, 2023 •

edited

Loading

yashrsharma44 commented Aug 19, 2023 •

edited

Loading

bobrik commented Sep 7, 2023

bobrik commented Sep 7, 2023

yurishkuro commented Sep 7, 2023

yurishkuro commented Sep 7, 2023

[feature] Visualize Critical Path of a trace #1582

[feature] Visualize Critical Path of a trace #1582

Conversation

GLVSKiriti commented Jul 14, 2023 • edited Loading

yashrsharma44 commented Aug 19, 2023 • edited Loading

bobrik commented Sep 7, 2023

bobrik commented Sep 7, 2023

yurishkuro commented Sep 7, 2023

yurishkuro commented Sep 7, 2023

GLVSKiriti commented Jul 14, 2023 •

edited

Loading

yashrsharma44 commented Aug 19, 2023 •

edited

Loading