Speed up Trace Statistics view calculation #1941

maxgaponov · 2023-11-03T12:02:16Z

Which problem is this PR solving?

Resolves [Feature]: Speed up Trace Statistics view calculation #1925

Description of the changes

Speed up Trace Statistics view calculation. Now we precompute child spans for each span to calculate statistics. Previously we filtered all spans to find children for each span.

How was this change tested?

Existing tests. Ran manually on a large trace.

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: yarn lint and yarn test

Resolves jaegertracing#1925 Now we precompute child spans for each span to calculate statistics. Previously we filtered all spans to find children for each span. Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

yurishkuro · 2023-11-03T12:41:15Z

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

@@ -97,6 +107,7 @@ function valueFirstDropdown(selectedTagKey: string, trace: Trace) {
  let color = '';
  let allDiffColumnValues = [];
  const allSpans = trace.spans;
+  parentChildOfMap = null;


why do you keep nulling this out?

yurishkuro · 2023-11-03T12:44:52Z

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

-      span.childSpanIds.includes(each.spanID) &&
-      each.references[0].spanID === span.spanID &&
-      each.references[0].refType === 'CHILD_OF'
+  const cdr = new DRange(span.startTime, span.startTime + span.duration - 1).intersect(


I prefer to keep the original logic parent.subtract(children). Your change does not require modifying that logic since all you need to do is cache children mapping.

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

codecov · 2023-11-17T19:09:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (85303aa) 96.54% compared to head (d2943f3) 96.54%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1941   +/-   ##
=======================================
  Coverage   96.54%   96.54%           
=======================================
  Files         256      256           
  Lines        7604     7613    +9     
  Branches     1978     1978           
=======================================
+ Hits         7341     7350    +9     
  Misses        263      263

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

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

maxgaponov · 2023-11-27T15:50:48Z

Seems like everything is fixed. Can you please review it again? @yurishkuro

yurishkuro · 2023-11-27T22:32:28Z

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

-      each.references[0].spanID === span.spanID &&
-      each.references[0].refType === 'CHILD_OF'
+  if (!span.hasChildren) return span.duration;
+  const spanRange = new DRange(span.startTime, span.startTime + span.duration - 1).subtract(


I don't think we can change the logic away from *10. DRange does not work correctly on the raw timestamps (see the comments above).

Side note: we need a test that illustrates that.

I don't understand why the calculations need to change - the speed-up is coming from building a map once instead of n^2 looping, so why not change just that part and leave the calculations intact?

In the change we use DRange(begin, end - 1) to represent halp-open intervals. The length of this DRange is (end-1)-begin+1=end-begin because DRange treats all intervals as inclusive.

In the example above (1, 10) will be represented as DRange(1, 9); (4, 8) will be represented as DRange(4, 7). So first.substract(second) will be range([1..3], [8..9]) (length = 3+2=5).

As I understand there is a mistake in the comment in the calculation of Drange.length.

That still doesn't answer my question why calculations need to change, it a PR that is about efficiency of the code.

If you want to dig down to the exact reasons for *10 logic, you can see the comment as well as the discussion on the original PR that introduced it. TLDR is that DRange does not understand open/close intervals, and subtraction works incorrectly for our purposes.

Agree. It shouldn't be changed in this PR. Returned to the initial version.

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Maksim <gaponovmaxev@gmail.com>

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

Signed-off-by: Yuri Shkuro <github@ysh.us>

Speed up Trace Statistics view calculation

046da80

Resolves jaegertracing#1925 Now we precompute child spans for each span to calculate statistics. Previously we filtered all spans to find children for each span. Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

maxgaponov requested a review from a team as a code owner November 3, 2023 12:02

maxgaponov requested review from yurishkuro and removed request for a team November 3, 2023 12:02

yurishkuro reviewed Nov 3, 2023

View reviewed changes

maxgaponov added 2 commits November 17, 2023 18:59

Refactored the changes

15b77b1

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

Merge branch 'main' into speedup-trace-statistics-calculation

0a19fee

maxgaponov requested a review from yurishkuro November 17, 2023 19:04

Refactored the changes

6f7cf46

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

yurishkuro added the changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements label Nov 27, 2023

yurishkuro reviewed Nov 27, 2023

View reviewed changes

Refactor the changes

2f2d0b9

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

yurishkuro reviewed Nov 28, 2023

View reviewed changes

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx Outdated Show resolved Hide resolved

maxgaponov and others added 4 commits November 29, 2023 09:20

Refactor the changes

8d3593d

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Maksim <gaponovmaxev@gmail.com>

Refactor the changes

ba8fde9

Signed-off-by: Maksim Gaponov <gaponovmaxev@gmail.com>

undo no-op changes

0074d0f

Signed-off-by: Yuri Shkuro <github@ysh.us>

Merge branch 'main' into speedup-trace-statistics-calculation

d2943f3

yurishkuro approved these changes Nov 29, 2023

View reviewed changes

yurishkuro merged commit 112e880 into jaegertracing:main Nov 29, 2023
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed up Trace Statistics view calculation #1941

Speed up Trace Statistics view calculation #1941

maxgaponov commented Nov 3, 2023 •

edited

Loading

yurishkuro Nov 3, 2023

yurishkuro Nov 3, 2023

maxgaponov Nov 17, 2023

codecov bot commented Nov 17, 2023 •

edited

Loading

maxgaponov commented Nov 27, 2023

yurishkuro Nov 27, 2023

yurishkuro Nov 27, 2023 •

edited

Loading

maxgaponov Nov 28, 2023

yurishkuro Nov 28, 2023

maxgaponov Nov 28, 2023

Speed up Trace Statistics view calculation #1941

Speed up Trace Statistics view calculation #1941

Conversation

maxgaponov commented Nov 3, 2023 • edited Loading

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

yurishkuro Nov 3, 2023

Choose a reason for hiding this comment

yurishkuro Nov 3, 2023

Choose a reason for hiding this comment

maxgaponov Nov 17, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 17, 2023 • edited Loading

Codecov Report

maxgaponov commented Nov 27, 2023

yurishkuro Nov 27, 2023

Choose a reason for hiding this comment

yurishkuro Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

maxgaponov Nov 28, 2023

Choose a reason for hiding this comment

yurishkuro Nov 28, 2023

Choose a reason for hiding this comment

maxgaponov Nov 28, 2023

Choose a reason for hiding this comment

maxgaponov commented Nov 3, 2023 •

edited

Loading

codecov bot commented Nov 17, 2023 •

edited

Loading

yurishkuro Nov 27, 2023 •

edited

Loading