-
Notifications
You must be signed in to change notification settings - Fork 488
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
Improve trace page title with data and unique emoji (fixes #2256) #2275
Conversation
packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
Outdated
Show resolved
Hide resolved
ce3e732
to
bda5c09
Compare
packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
Show resolved
Hide resolved
f65c4ff
to
4bcc015
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2275 +/- ##
=======================================
Coverage 96.59% 96.60%
=======================================
Files 254 254
Lines 7641 7662 +21
Branches 1990 1931 -59
=======================================
+ Hits 7381 7402 +21
Misses 260 260 ☔ View full report in Codecov by Sentry. |
2d46e31
to
547663d
Compare
cad096d
to
03fe1a8
Compare
It is already provided to the component through the traceName prop. Signed-off-by: Anthony Ramine <nox@nox.paris>
Signed-off-by: Anthony Ramine <nox@nox.paris>
We make trace pages have a title "<trace short ID>: <operation name> (<service name>)" to improve navigation when the user has many traces open in their browser. Signed-off-by: Anthony Ramine <nox@nox.paris>
> Dear colleague, are you talking about the shrimp trace or the baguette trace? To help discussing about traces with coworkers when debugging issues, referring to a unique and distinguishable emoji is good. Signed-off-by: Anthony Ramine <nox@nox.paris>
</h1> | ||
); | ||
|
||
return ( | ||
<header className="TracePageHeader"> | ||
<Helmet title={`${trace.traceEmoji} ${traceShortID}: ${trace.tracePageTitle} — Jaeger UI`} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on optionally including the timestamp? That would be more intuitive than the trace ID.
@nox how do you actually use this new feature? I have several traces open in Chrome tabs. They all have our standard favicon, on the trace page the new emojis are not showing anywhere. The only place where I see them is if I expand the tabs list dropdown (something that I almost never do as a navigation mechanism): |
The emojis are the first thing that show up in the title, as we decided back then to not make them favicons. So far the emojis were good enough as long as you don't have too many tabs open as @bobrik said. Would you want me to make a new PR for a setting that put them as favicons, and another that adds them to the page content itself? |
Which problem is this PR solving?
Description of the changes
<unique emoji> <short trace id>: <operation name> (<service name>) — Jaeger UI
.How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test