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

Revamp trace page #3486

Merged
merged 61 commits into from
Dec 13, 2022
Merged

Revamp trace page #3486

merged 61 commits into from
Dec 13, 2022

Conversation

tacigar
Copy link
Member

@tacigar tacigar commented Dec 1, 2022

This PR will

  • Refactor and simplify the process of creating the trace tree.
  • Change the design.
  • Add MiniTimeline feature.
  • Add SpanTable feature.
  • Add AnnotationTooltip feature.
zipkin-revamp-tracepage.mp4

@tacigar tacigar marked this pull request as draft December 3, 2022 01:04
@tacigar tacigar added enhancement ui Zipkin UI labels Dec 4, 2022
@tacigar tacigar changed the title Revamp trace page Revamp Trace page Dec 4, 2022
@tacigar tacigar changed the title Revamp Trace page Revamp trace page Dec 4, 2022
@tacigar tacigar marked this pull request as ready for review December 4, 2022 12:47
@@ -114,7 +114,10 @@ function formatDate(timestamp, utc) {
}

export function mkDurationStr(duration) {
if (duration === 0 || typeof duration === 'undefined') {
if (duration === 0) {
Copy link
Member

@basvanbeek basvanbeek Dec 7, 2022

Choose a reason for hiding this comment

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

This change in behavior makes a test fail... please fix the test because I think the update to this function makes sense.

[INFO] FAIL src/zipkin/trace.test.js
[INFO]   ● mkDurationStr › should return empty string on zero duration
[INFO] 
[INFO]     expect(received).toBe(expected) // Object.is equality
[INFO] 
[INFO]     Expected: ""
[INFO]     Received: "0ms"
[INFO] 
[INFO]       354 | describe('mkDurationStr', () => {
[INFO]       355 |   it('should return empty string on zero duration', () => {
[INFO]     > 356 |     expect(mkDurationStr(0)).toBe('');
[INFO]           |                              ^
[INFO]       357 |   });
[INFO]       358 | 
[INFO]       359 |   it('should return empty string on undefined duration', () => {
[INFO] 
[INFO]       at Object.<anonymous> (src/zipkin/trace.test.js:356:30)
[INFO] 
[INFO] 
[INFO] Test Suites: 1 failed, 23 passed, 24 total
[INFO] Tests:       1 failed, 233 passed, 234 total

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @basvanbeek
Fixed 🙇

Copy link
Member

@basvanbeek basvanbeek left a comment

Choose a reason for hiding this comment

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

@tacigar thank you for this awesome improvement to the Zipkin UI. I wholeheartedly approve it.

@tacigar
Copy link
Member Author

tacigar commented Dec 13, 2022

Thank you @basvanbeek !!

@tacigar tacigar merged commit e5d7f21 into master Dec 13, 2022
@tacigar tacigar deleted the enhance-lens branch December 13, 2022 12:17
@jcchavezs
Copy link
Contributor

This is awesome @tacigar !

@tacigar
Copy link
Member Author

tacigar commented Dec 25, 2022

Upload a screenshot to update the image of README.md

localhost_3000_zipkin_traces_a03ee8fff1dcd9b9

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

Successfully merging this pull request may close these issues.

3 participants