Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UI docs #185

Merged
merged 8 commits into from
Jan 4, 2019
Merged

Add UI docs #185

merged 8 commits into from
Jan 4, 2019

Conversation

aljesusg
Copy link
Contributor

@aljesusg aljesusg commented Dec 4, 2018

WIP Add dosc for the UI

  • Add embedded components

Fix issue #183
Require: jaegertracing/jaeger-ui#286

I do not know exactly what kind of documentation would be the most appropriate for this

cc @tiffon

Demo: https://deploy-preview-185--jaegertracing.netlify.com/docs/next-release/frontend-ui/

@aljesusg
Copy link
Contributor Author

aljesusg commented Dec 4, 2018

I called interface instead of UI. WDYT @tiffon ?

Demo: https://deploy-preview-185--jaegertracing.netlify.com/docs/next-release/frontend-ui/

@yurishkuro
Copy link
Member

"interface" is confusing, I would rather call it "Frontend/UI".

Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
@aljesusg aljesusg changed the title WIP Add UI docs Add UI docs Dec 12, 2018
Copy link
Member

@tiffon tiffon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few suggestions. Let me know what you think.

Thanks!

weight: 10
---

Jaeger Web UI is implemented in Javascript using popular open source frameworks like React. Several performance improvements have been released in v1.0 to allow the UI to efficiently deal with large volumes of data, and to display traces with tens of thousands of spans (e.g. we tried a trace with 80,000 spans).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javascript => JavaScript

I don't think we need the second sentence...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I just move the content in docs/1.8/features/#modern-web-ui but I agree with you about remove the second sentence


## Embed Mode

Enhancement to embed mode in Jaeger UI for some components allowing us to integrate it into other applications.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest rephrasing this paragraph to:


Starting with version [TODO], Jaeger UI provides an "embedded" layout mode which is intended to support integrating Jaeger UI into other applications. Currently (as of v0), the approach taken is to remove various UI elements from the page to make the UI better suited for space-constrained layouts.

The embedded mode is induced and configured via URL query parameters.

To enter embedded mode, the uiEmbed=v0 query parameter and value must be added to the URL. For example, the following URL will show the trace with ID abc123 in embedded mode:

https://example.com/trace/abc123?uiEmbed=v0

uiEmbed=v0 is required.

Further, each page supported has an Embed open window button added that will open the non-embedded page in a new tab.


What do you think?

Here is the markdown:

Starting with version **[TODO]**, Jaeger UI provides an "embedded" layout mode which is intended to support integrating Jaeger UI into other applications. Currently (as of `v0`), the approach taken is to remove various UI elements from the page to make the UI better suited for space-constrained layouts.

The embedded mode is induced and configured via URL query parameters.

To enter embedded mode, the `uiEmbed=v0` query parameter and value must be added to the URL. For example, the following URL will show the trace with ID `abc123` in embedded mode:

```
https://example.com/trace/abc123?uiEmbed=v0
```

`uiEmbed=v0` is required.

Further, each page supported has an <img src="/static/img/frontend-ui/embed-open-icon.png" style="width: 20px; height:20px;" alt="Embed open window"> button added that will open the non-embedded page in a new tab.

uiEmbed=v0
```

![Embed Search Traces](/img/frontend-ui/embed-search-traces.png)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image URLs need /static/ prepended.

The images will also need to be updated whenever we land on a final layout and design.


##### Optionally

We can use an extra options in the embed mode :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest rephrasing as:

The following query parameter can be used to configure the layout of the search page:

  • uiSearchHideGraph=1
    • Do not display the scatter plot above the search results


![Embed Search Traces](/img/frontend-ui/embed-search-traces.png)

We have a button in this kind of view that redirect us to the search traces page in the Jaeger Site <img src="/img/frontend-ui/embed-open-icon.png" style="width: 20px; height:20px; display:inline;" alt="Embed open window">.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ^ can be removed if it is added to the section above "EmbedMode" intro section.

service=jaeger-query&
start=1543917759557000&
uiEmbed=v0&
uiSearchHideGraph=1s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a typo, =1s


We have a button in this kind of view that redirect us to the search traces page in the Jaeger Site <img src="/img/frontend-ui/embed-open-icon.png" style="width: 20px; height:20px; display:inline;" alt="Embed open window">.

##### Optionally
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest renaming this title to "Configuration options"


![Embed Trace view](/img/frontend-ui/embed-trace-view-with-back-button.png)

We have a new button in this kind of view that redirect us to the trace page in the Jaeger Site <img src="/img/interface/embed-open-icon.png" style="width: 20px; height:20px; display:inline;" alt="Embed open window">.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ^ can be removed if it is added to the section above "EmbedMode" intro section.


We have a new button in this kind of view that redirect us to the trace page in the Jaeger Site <img src="/img/interface/embed-open-icon.png" style="width: 20px; height:20px; display:inline;" alt="Embed open window">.

##### Optionally
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest renaming this title to "Configuration options"


##### Optionally

We can use an extra options in the embed mode :
Copy link
Member

@tiffon tiffon Dec 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest rephrasing to the following and adding images after summarizing the query parameters.

The following query parameters can be used to configure the layout of the trace page:

  • uiTimelineCollapseTitle=1
    • The trace header starts out collapsed, which hides the summary and minimap
  • uiTimelineHideMinimap=1
    • Removes the minimap, entirely, regardless of whether the trace header is expanded or not
  • uiTimelineHideSummary=1
    • Removes the trace summary information (number of services, etc.), entirely, regardless of whether the trace header is expanded or not

@aljesusg
Copy link
Contributor Author

@tiffon I did the changes. I added the images about trace embed that you uploaded in your jaegertracing/jaeger-ui#286 and I added the images to the public/ too. Anyway We should wait until the merge of jaegertracing/jaeger-ui#286

Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
@tiffon
Copy link
Member

tiffon commented Dec 20, 2018

Thanks, @aljesusg, the changes look great! jaegertracing/jaeger-ui#286 is merged. 👍

@aljesusg
Copy link
Contributor Author

Great !!!

@yurishkuro
Copy link
Member

Wouldn't it better to consolidate the configuration options from https://www.jaegertracing.io/docs/1.8/deployment/ into this new page as well, and link to it from Deployment?

@aljesusg
Copy link
Contributor Author

As you want, if you think it's better to make the change I can do it but ...I think that it could be confuse for the user have the deployment manual (components) with the documentation about the UI and how to use/work it. What do you think ? @tiffon @yurishkuro

@tiffon
Copy link
Member

tiffon commented Dec 22, 2018 via email

@aljesusg
Copy link
Contributor Author

So What do you think about to change the first sentence to something like

Jaeger Web UI is implemented in JavaScript using popular open source frameworks like React. 
Check the [administration](/docs/next-release/deployment#query-service-ui) documentation to configure the UI.

Or What was your idea?

@yurishkuro
Copy link
Member

yurishkuro commented Dec 24, 2018

I was thinking these docs would be for users.

@tiffon someone deploying jaeger is also a "user". If you are talking about real end users who actually use the UI to investigate traces, then this document is clearly not for them - why would they care about embedding and the internal API to control the embedding? In fact, I almost want to argue that having documentation for end users is an anti-pattern - it would mean the UI is poorly designed and non-intuitive. End users may need a tutorial mode in the UI that walks them through the UI elements.

Or What was your idea?

@aljesusg my proposal was to move UI section from Deployment to this doc, and only leave a link in the Deployment. I think both configuration and embedding are part of the UI deployment story, and this doc should be a sub-document of the Deployment (similar to how Client Features doc is sub-doc of Client Libraries).

@tiffon
Copy link
Member

tiffon commented Dec 30, 2018

@yurishkuro Yes, I was referring to folks using the UI. You're right, people deploying Jaeger are also users. I would tend to think of them as administrators, but they're definitely users, either way.

real end users who actually use the UI to investigate traces, then this document is clearly not for them

Great point. I do think the "Jaeger UI" section is the right place for the docs on embedding, though.

In fact, I almost want to argue that having documentation for end users is an anti-pattern - it would mean the UI is poorly designed and non-intuitive.

I don't see how having documentation means the UI is poorly designed and non-intuitive. For instance, Gmail has docs and I'd say it's very easy to use. Same with GitHub.

As an aside: I think there is very little likelihood that documentation would make up for a confusing UI, so I don't see docs as running the risk of enabling a bad UI.

End users may need a tutorial mode in the UI that walks them through the UI elements.

I'd say step-by-step guides in the UI would be great for introducing features but unmanageable for a full tutorial or introduction to the UI. For that, I think taking inspiration from something like this Chrome overview would be great.

I think both configuration and embedding are part of the UI deployment story

Seems like integrating the UI into another product is not a sub-topic of deploying Jaeger. For instance, the Studio team (a reference internal to Uber) doesn't need to know anything about deploying Jaeger if they only plan to embed it.

@aljesusg, you're consuming the embed functionality, are you also involved in deploying Jaeger?

@aljesusg
Copy link
Contributor Author

aljesusg commented Jan 2, 2019

no @tiffon, only consuming from Kiali project

Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
@aljesusg
Copy link
Contributor Author

aljesusg commented Jan 2, 2019

I moved the docs to deployment @yurishkuro https://deploy-preview-185--jaegertracing.netlify.com/docs/next-release/deployment/#frontend-ui but I think that we should add the index to the frontend docs like the deployment, I saw that in client features is disable but I think that we have so information in the UI docs. What do you think?

----- | ------- | ---
16686 | HTTP | **/api/*** endpoints and Jaeger UI at **/**
16687 | HTTP | Health check at **/**

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think text up to here should remain in the Deployment, with the same title "Query Service & UI". The new page is only about the UI configuration.

Copy link
Contributor Author

@aljesusg aljesusg Jan 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed thanks

Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
Port | Protocol | Function
----- | ------- | ---
16686 | HTTP | **/api/*** endpoints and Jaeger UI at **/**
16687 | HTTP | Health check at **/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please don't remove this section above from the deployment doc. It refers to query-service, a microservice in Jaeger, and logically belongs to deployment. The rest of this doc refers to customization / configuration of the UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed, sorry but I didn't not understand correctly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a commit with the change that I wanted.

Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
@ghost ghost assigned yurishkuro Jan 4, 2019
@ghost ghost added the review label Jan 4, 2019
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ready to be merged? Are the images final?

@aljesusg
Copy link
Contributor Author

aljesusg commented Jan 4, 2019

Yes, the images are final from @tiffon PR with the last changes

@yurishkuro yurishkuro merged commit eb3e259 into jaegertracing:master Jan 4, 2019
@ghost ghost removed the review label Jan 4, 2019
@yurishkuro
Copy link
Member

thanks!

@aljesusg
Copy link
Contributor Author

aljesusg commented Jan 4, 2019

Thanks @yurishkuro & @tiffon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants