Embedded components (SearchTraces and Tracepage )

[aljesusg]

Related issue : #248

The point is make a PoC about how whould this look .

We have here 2 uses of cases:

• Show the results of a search
  

• Show a Trace
  

So I added query param called embed, with this params the top navigation and search are removed.
See demo GIF

How works ? pass embed like a query param

TracePage View

• minimap option
• details option

SearchTraces View

• disableComparision option
• hideGraph option

[aljesusg]

Kiali PoC in kiali/kiali-ui#736 and how looks

[objectiser]

@aljesusg Is the time duration linked to the duration used on other pages? So if navigating from a page where duration is set to 30mins, that will be used when navigating to the traces page?

Will you also be supporting the tag filter field?

[aljesusg]

@objectiser this form is a created form in the application that is passed to the /search url like query params.

Example:
When the user cahnge my form I opoen an iframe to this url with the params that I have in the form
search?end=1539087907927000&limit=100&lookback=2d&maxDuration&minDuration&service=productpage&start=1538915107927000&tags=%7B"error"%3A"true"%7D

[codecov]

Codecov Report

Merging #263 into master will increase coverage by 0.29%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
+ Coverage   77.15%   77.45%   +0.29%     
==========================================
  Files         135      137       +2     
  Lines        2955     2976      +21     
  Branches      613      617       +4     
==========================================
+ Hits         2280     2305      +25     
+ Misses        532      529       -3     
+ Partials      143      142       -1

Impacted Files	Coverage Δ
...ger-ui/src/components/TracePage/TracePageHeader.js	70% <ø> (ø)	⬆️
packages/jaeger-ui/src/components/App/Page.js	100% <ø> (ø)	⬆️
...s/SearchTracePage/SearchResults/ResultItemTitle.js	80% <ø> (ø)	⬆️
...ckages/jaeger-ui/src/components/TracePage/index.js	72.05% <100%> (+6.9%)	⬆️
packages/jaeger-ui/src/utils/embedded/index.js	100% <100%> (ø)
...onents/SearchTracePage/SearchResults/ResultItem.js	100% <100%> (ø)	⬆️
.../jaeger-ui/src/components/SearchTracePage/index.js	86.2% <60%> (-0.84%)	⬇️
...i/src/components/TracePage/TracePageHeaderEmbed.js	66.66% <66.66%> (ø)
.../components/SearchTracePage/SearchResults/index.js	73.68% <75%> (-1.32%)	⬇️
packages/jaeger-ui/src/utils/date.js	85% <0%> (-5%)	⬇️
... and 4 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 e01bc36...b480f52. Read the comment docs.

[tiffon]

This looks great.

I'd say for the search page, we might want to add the ability to

• Hide the scatter plot
• Disable trace comparisons

For the trace detail page, it might be nice to be able to:

• Start with the mini-map collapsed
• Have a way to go back to the search results page (if the user navigated from there)

What do you think?

For both pages, does it makes sense to show a link which would let the user open the page in a new tab (without the embed view restrictions)?

Looks great in the Kiali gif!

[aljesusg]

Hi @tiffon now I am working in the tracePage embed element, I did the SearchTraces. What do you think about the position of the button to open in a new tab the full view? (You can see the button in the Gif Demo)

How works ?

Search page

with disableComparision option

Sample : search?embed&end=1539596003745000&disableComparision&limit=20&lookback=1h&maxDuration&minDuration&service=productpage&start=1539592403745000

With hideGraph and disableComparision option

Sample : search?disableComparision&embed&end=1539596003745000&hideGraph&limit=20&lookback=1h&maxDuration&minDuration&service=productpage&start=1539592403745000

Demo

[aljesusg]

Demo TracePage

With option mapCollapsed the map is collapsed.
I don't like where I put the buttons to GoBack and link to a new tab with the full version. Any idea?

[aljesusg]

Changes addressed @tiffon , I don't know where is the best position for the buttons of goBack and go to the Jaeger Site

[tiffon]

@aljesusg Looks great!

Regarding the where to put the buttons, I think you're right; they kind of stick out, right now.

I've been looking at other embed styles, and they usually have a footer which gives a title and some other details:

GitHub gist

Plotly (not an embedded view, but similar)

(The red mark in the above image is from the blog post I copied it from; I didn't add it.)

Maybe we can do something similar, but move it to the top?

The top bar can link to the full view and replace the current trace header (title, extra buttons, etc).

This would make the view more compressed and make the "Back to search results" button seem less out of place.

Here is one possible aesthetic, with the back and without:

What do you think?

We could apply a similar treatment to the search results view so they'd be consistent.

Thanks for your time and hard work!

[aljesusg]

Ok @tiffon I am going to make the changes asap. Thanks

[aljesusg]

@tiffon for example for trace view is ok like this or do you want create a header for embed component without search and options? Where is the antd component for that example? I can't find that component in https://ant.design/

What do you think about how look this ?

Trace View Bar

Search Results Bar

[tiffon]

@aljesusg I think a header without the "View Options" and keyboard help modal would probably be preferable. It might also be nice to be abel to toggle the additional details (datetime, etc) and the minimap. What do you think?

The screenshots from my previous comment are not from an Ant Design component, I use the Chrome Dev Tools to work something up from a random page to just take the screenshot.

Would it be possible to match the aesthetic from the screenshots?

[aljesusg]

Hi @tiffon

Sorry but I was on holidays this weeks. I'll try to adapt the ui to be like your screnshoots but I don't know if we can do it with antd

[objectiser]

@tiffon Looks like only documenting kafka ingester is currently missing from 1.8 milestone, so anytime now - although that task has been open for a while. I agree that the comparison feature in embedded mode could be left out for the first cut of this feature.

@aljesusg Any thoughts on how long it may take to implement the changes @tiffon has requested (including leaving out compare)?

[aljesusg]

I'll try to have the changes today @objectiser

[tiffon]

@aljesusg We can use the value of the embed query string parameter to act as a version specification. So, for now, the only valid value for embed would be embed=v0 which would enable the search and/or trace detail view embedding, and the search results will never be comparable when embed=v0. Presently, any other value for embed would not trigger embed-mode, it would effectively be ignored.

Then, later, we can add trace comparisons as a possibility and use embed=v1. This way, older URLs which use embed=v0 will still not have comparisons.

This also gives us versioning for embedding, in general, which we also need.

What do you think?

[aljesusg]

Great Idea @tiffon . I did the changed that you proposed but in the traceView I can't align the tittle with the buttons, i don't know if I should set in the css margin-top:10px, to fixed or if there is another way with antd to solve this.

On the other hand I am going to show the GoBack button if in the history there is a search url in the previous one.

[tiffon]

@aljesusg thanks for making these changes. A few minor comments, the main thing I noticed is a few issues around embed vs embed=v0.

On both the search and trace detail pages, ?embed (without the =v0) still removes the top-nav.

Also, on the embed search results page, can you remove the "Compare traces by..." message?

Also, on the trace detail page, when there isn't "Go back" button, can you add a margin to the left of the title?

[aljesusg]

Hi @tiffon , here is the last update of the demo with the comments addressed. Thanks

• It's necessary to set embed=v0 (This configuration is in /utils/embedded folder)
• SearchOptions
  • hideGraph
    
• TraceOptions
  • enableDetails
  • mapCollapsed
  • The Go Back button only available if the user come from a search page (see demo for more details)
    

[tiffon]

A few comments are marked as resolved but don't have an accompanying change or replies to the effect of "will not change". Is there an un-pushed commit? If not, in the future can you please mark comments you're not inclined to address as "won't change" or something along those lines?

[aljesusg]

Ok but I think that I addressed all the changes, sorry @tiffon I'll do the changes asap

[tiffon]

@aljesusg Thanks! I didn't meant to interrupt your weekend, just wasn't sure about some of the changes.

[aljesusg]

@tiffon Thanks for your comments !!! I did the changes, don't worry about interrupt my weekend, I am glad to collaborate :)

[tiffon]

@aljesusg Awesome! 🎉

This PR is the beginning of an entirely new type of use-case for the UI. We're super excited about it and thank you for the time, effort and ingenuity!