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

Show seconds in trace start time on the trace page #430

Merged
merged 1 commit into from
Aug 4, 2019

Conversation

tiffon
Copy link
Member

@tiffon tiffon commented Aug 4, 2019

Which problem is this PR solving?

Trace start time, on the trace page, is not granularity enough.

Short description of the changes

Show the seconds and sub-seconds of the trace start time in the trace page header.

The seconds and sub-seconds are presented with reduced contrast:

Screen Shot 2019-08-03 at 11 02 48 PM

The seconds and sub-seconds are shown at full contrast when the datetime value is hovered:

Screen Shot 2019-08-03 at 11 02 55 PM

Signed-off-by: Joe Farro <joef@uber.com>
renderer: (trace: Trace) => formatDatetime(trace.startTime),
renderer: (trace: Trace) => {
const dateStr = formatDatetime(trace.startTime);
const match = dateStr.match(/^(.+)(:\d\d\.\d+)$/);
Copy link
Member

Choose a reason for hiding this comment

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

I would only lower the contrast for sub-seconds, not for seconds

<span className="TracePageHeader--overviewItem--valueDetail">{match[2]}</span>
</span>
) : (
dateStr
Copy link
Member

Choose a reason for hiding this comment

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

should this also use <span> for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's better to leave it off if it's not serving a purpose.

@codecov
Copy link

codecov bot commented Aug 4, 2019

Codecov Report

Merging #430 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #430      +/-   ##
==========================================
+ Coverage   91.67%   91.67%   +<.01%     
==========================================
  Files         176      176              
  Lines        4010     4012       +2     
  Branches      928      957      +29     
==========================================
+ Hits         3676     3678       +2     
+ Misses        296      292       -4     
- Partials       38       42       +4
Impacted Files Coverage Δ
...ents/TracePage/TracePageHeader/TracePageHeader.tsx 100% <100%> (ø) ⬆️
packages/jaeger-ui/src/utils/date.tsx 90.47% <100%> (ø) ⬆️
...ents/TracePage/TraceTimelineViewer/TimelineRow.tsx 77.77% <0%> (ø) ⬆️
...kages/jaeger-ui/src/model/transform-trace-data.tsx 85.07% <0%> (ø) ⬆️
.../jaeger-ui/src/components/common/BreakableText.tsx 83.33% <0%> (ø) ⬆️

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 1e26e57...ff5a56d. Read the comment docs.

@tiffon tiffon merged commit 5159ae5 into jaegertracing:master Aug 4, 2019
@tiffon tiffon deleted the trace-start-datetime-seconds branch August 4, 2019 03:31
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021
Show seconds in trace start time on the trace page
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
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.

None yet

2 participants