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

feat(ui): condensed trace tree #4099

Merged
merged 3 commits into from
Aug 2, 2024
Merged

feat(ui): condensed trace tree #4099

merged 3 commits into from
Aug 2, 2024

Conversation

mikeldking
Copy link
Contributor

@mikeldking mikeldking commented Aug 1, 2024

resolves #4027

New condensed span tree with collapse controls per span.
Screenshot 2024-08-01 at 5 03 10 PM

@mikeldking mikeldking marked this pull request as ready for review August 1, 2024 23:05
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 1, 2024
icon = isFilled ? <RerankerFilledSVG /> : <RerankerSVG />;
break;
case "evaluator":
color = "--ac-global-color-indigo-1000";
color = isDark
? "--ac-global-color-indigo-1000"
Copy link
Contributor

@Parker-Stafford Parker-Stafford Aug 2, 2024

Choose a reason for hiding this comment

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

was the rotation not enough with the new filled icons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah they get super dark. I adjusted them manually with Andy. I think this is necessary overall to make light mode look good

) : null}
<SpanTreeEdge {...leafNode.span} />
<SpanTreeEdge {...leafNode.span} nestingLevel={nestingLevel} />
Copy link
Contributor

Choose a reason for hiding this comment

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

why is the edges level not incremented? because the connection between the two spans should be a the current spans level not the next?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's the edge of the curve at the current level. It's the little rounding on the left of the icon

Copy link
Contributor

@Parker-Stafford Parker-Stafford left a comment

Choose a reason for hiding this comment

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

looks nice

top: 0;
left: -22px;
left: ${nestingLevel * NESTING_INDENT + 29}px;
Copy link
Contributor

Choose a reason for hiding this comment

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

some of thee numbers are pretty magical, this is just to get everything to line up correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so the nesting level is how deep you are in the tree. That automatically pushes the start of the icon in by the amount we want each icon to indent. the 29 makes up for the initial margin (16) + border (4) + half the distance of the icon (10) - 1 for some spacing. I can try to write it out but honestly it's sorta easier to adjust and make sure the math is vaguely right. LMK - happy to make the calcs be variable based.

@mikeldking mikeldking merged commit 548f685 into main Aug 2, 2024
6 checks passed
@mikeldking mikeldking deleted the collapsed-trace-tree branch August 2, 2024 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[annotations][ui] condensed trace tree
2 participants