Derive DDG from search results

[rubenvp8510]

Which problem is this PR solving?

• Derived a DDG form the search results.

Resolves #399

Short description of the changes

• Used trace search results for derive a DDG. this is still work in progress..

Added a button on the top right of the trace results (it does not have a button style yet)

[yurishkuro]

Do you have some ideas how that would be useful?

[objectiser]

@yurishkuro Its the approach that was discussed on the bi-weekly call a couple of months back - for users who want a minimal backend installation (so no flink or spark job running once a day), but still benefit from a service dependency graph showing the services involved in the traces they are interested in.

[objectiser]

@rubenvp8510 On the trace instance view page it is possible to switch between views by using a dropdown. Wondering if the same would work on the trace search page for the dependency view - so the region below the scatter plot could be switched between a trace instance list and the dependency view? Not sure if the region would be too small - but the benefit is that the search criteria would still be visible allowing the user to tweak the criteria and press the search button again and see the update in the dependency view.

[rubenvp8510]

@objectiser yes it could be, my concern was the size of the view, but I can modify this PR and attach an screenshot so we can evaluate it.

[everett980]

@rubenvp8510 I think the button should exist in the "Compare Traces" section.

Something like this (except without hiding the trace name and details):

This way you can select a subset of search results. It would also support making selections, then searching again, and adding selections from the second search.

The DDG view would need a button to go back to search when generated in this manner, which would preserve the selected traceIDs (there is a url param that supports this).

Some of the text would need to be updated of course. Something like ${n} Traces Selected instead of ${2} Selected for Comparison. And the text before selecting traces would need to include that selection is used for more than just comparison.

[rubenvp8510]

@everett980 That sounds good, the only thing is that in that way there is no way to use all traces, unless I selected all traces, or am i missing something here?, I don''t see a button "select all" ,but of course we can add it.

[everett980]

@rubenvp8510 yeah we would need to add a select all (or "add all" to current selection if any traces are already selected).
as well as maybe a "clear selection" button.

[rubenvp8510]

Attached two screenshots with @objectiser suggestions, this does not look bad.

Not sure about the suggestion of select traces for generate a deep dependency graph, Correct me if I'm wrong but I think most of the time we would want to generate the deep graph with all traces we have. (so we can call it deep dependency graph),

[objectiser]

I like the simplicity of being able to switch between the list and dependency graph view with a single button - so possibly if the "select individual traces" option is also required, that could be in addition?

@rubenvp8510 When in the graph view, is it possible to change the search criteria, press the search button and see the graph update (i.e. without first being in the list view)?

[rubenvp8510]

@objectiser yes , the graph will be updated with the new search criteria.

[codecov]

Codecov Report

Merging #445 into master will increase coverage by 0.12%.
The diff coverage is 92.42%.

@@            Coverage Diff             @@
##           master     #445      +/-   ##
==========================================
+ Coverage    89.3%   89.43%   +0.12%     
==========================================
  Files         189      192       +3     
  Lines        4434     4479      +45     
  Branches     1063     1073      +10     
==========================================
+ Hits         3960     4006      +46     
+ Misses        428      427       -1     
  Partials       46       46

Impacted Files	Coverage Δ
...kages/jaeger-ui/src/model/ddg/transformDdgData.tsx	100% <ø> (ø)	⬆️
...ui/src/components/DeepDependencies/Graph/index.tsx	66.66% <ø> (ø)	⬆️
...ts/DeepDependencies/Graph/DdgNodeContent/index.tsx	35.29% <0%> (ø)	⬆️
.../jaeger-ui/src/components/SearchTracePage/index.js	88.13% <100%> (ø)	⬆️
...i/src/components/DeepDependencies/Header/index.tsx	100% <100%> (ø)	⬆️
...s/SearchTracePage/SearchResults/AltViewOptions.tsx	100% <100%> (ø)
...jaeger-ui/src/model/ddg/transformTracesToPaths.tsx	100% <100%> (ø)
...eger-ui/src/components/DeepDependencies/traces.tsx	100% <100%> (ø)
.../jaeger-ui/src/components/DeepDependencies/url.tsx	100% <100%> (ø)	⬆️
...aeger-ui/src/components/DeepDependencies/index.tsx	73.17% <75%> (-0.58%)	⬇️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98530d6...12f31c1. Read the comment docs.

[everett980]

@objectiser

I like the simplicity of being able to switch between the list and dependency graph view with a single button

I agree that the simplicity of this approach is very nice.

so possibly if the "select individual traces" option is also required, that could be in addition?

The ability to include selected traces from previous searches would be beneficial but can be tackled later on.

[everett980]

@rubenvp8510 It seems as though DeepDependencies/traces.tsx is a fork of DeepDependencies/index.tsx. This is going to result in a lot of maintenance effort as the feature is rapidly evolving.

I'm wondering if it would make sense for DeepDependencies/traces.tsx to import { DeepDependencyGraphPageImpl } from './';
That way DeepDependencies/traces.tsx and DeepDependencies/index.tsx can have different mapStateToProps functions, but otherwise have less repetition and less to maintain.

I believe the only change that would be necessary for DeepDependencyGraphPageImpl would be to hide the select service & operation drop downs:

(Which would require a small change to DeepDependencies/Header.tsx as well)

[rubenvp8510]

@everett980 I totally agree with your comments. there are a couple of more changes that we need to do it other than just hidden a menu, but I don't think it will be hard to do it now that I understand better how the graph works.

I'll update the PR with this, I think it will also help with the coverage, as we are reusing more parts.

[everett980]

Just left some minor observations and noticed the chance to reuse an existing util to simplify transformTracesToPaths

[objectiser]

@everett980 Are there any other issues that need to be addressed with this one, or can it be merged?

[everett980]

@objectiser I gave it a quick look at the changes last week and it looked good to me. haven't had the chance to give it a final pass over yet but I should have time Monday.

[objectiser]

@everett980 Great, thanks!

[everett980]

This diff looks good to me, aside from some small comments.
I am, however, complicating the situation by making conflicting changes. I have one more in the works that will hopefully be done today.
I'll handle the merge conflicts I'm creating and the comments I left, and hopefully get this merged in the next couple days.
That also might be a good point to cut a release of the UI.