Skip to content

Commit

Permalink
feat(new-trace): Added logic for red errored rows. (#66194)
Browse files Browse the repository at this point in the history 
<img width="1035" alt="Screenshot 2024-03-03 at 9 36 22 PM"
src="https://github.com/getsentry/sentry/assets/60121741/860a6370-b1aa-41dc-b5c8-b62b9778ffa5">

---------

Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
  • Loading branch information
@Abdkhan14 @aliu3ntry
2 people authored and aliu3ntry committed Mar 6, 2024
1 parent 35a45b3 commit 9ae6f76
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 17 deletions.
26 changes: 21 additions & 5 deletions static/app/views/performance/newTraceDetails/trace.tsx
View file
Expand Up @@ -609,7 +609,7 @@ function RenderRow(props: {
}}
>
<div
className="TraceLeftColumnInner"
className={`TraceLeftColumnInner ${props.node.has_error ? 'Errored' : ''}`}
style={{
paddingLeft: props.node.depth * props.manager.row_depth_padding,
}}
Expand All @@ -621,6 +621,7 @@ function RenderRow(props: {
status={props.node.fetchStatus}
expanded={!props.node.expanded}
onClick={e => props.onExpand(e, props.node, !props.node.expanded)}
errored={props.node.has_error}
>
{COUNT_FORMATTER.format(props.node.groupCount)}
</ChildrenButton>
Expand Down Expand Up @@ -652,6 +653,9 @@ function RenderRow(props: {
}

if (isTransactionNode(props.node)) {
const errored =
props.node.value.errors.length > 0 ||
props.node.value.performance_issues.length > 0;
return (
<div
key={props.index}
Expand Down Expand Up @@ -679,15 +683,16 @@ function RenderRow(props: {
}}
>
<div
className="TraceLeftColumnInner"
className={`TraceLeftColumnInner ${errored ? 'Errored' : ''}`}
style={{
paddingLeft: props.node.depth * props.manager.row_depth_padding,
}}
>
<div
className={`TraceChildrenCountWrapper ${
props.node.isOrphaned ? 'Orphaned' : ''
}`}
}
`}
>
<Connectors node={props.node} manager={props.manager} />
{props.node.children.length > 0 || props.node.canFetch ? (
Expand All @@ -708,6 +713,7 @@ function RenderRow(props: {
? props.onZoomIn(e, props.node, !props.node.zoomedIn)
: props.onExpand(e, props.node, !props.node.expanded)
}
errored={errored}
>
{props.node.children.length > 0
? COUNT_FORMATTER.format(props.node.children.length)
Expand Down Expand Up @@ -742,6 +748,7 @@ function RenderRow(props: {
}

if (isSpanNode(props.node)) {
const errored = props.node.value.relatedErrors.length > 0;
return (
<div
key={props.index}
Expand Down Expand Up @@ -769,7 +776,7 @@ function RenderRow(props: {
}}
>
<div
className="TraceLeftColumnInner"
className={`TraceLeftColumnInner ${errored ? 'Errored' : ''}`}
style={{
paddingLeft: props.node.depth * props.manager.row_depth_padding,
}}
Expand Down Expand Up @@ -798,6 +805,7 @@ function RenderRow(props: {
? props.onZoomIn(e, props.node, !props.node.zoomedIn)
: props.onExpand(e, props.node, !props.node.expanded)
}
errored={errored}
>
{props.node.children.length > 0
? COUNT_FORMATTER.format(props.node.children.length)
Expand Down Expand Up @@ -1192,9 +1200,13 @@ function ChildrenButton(props: {
icon: React.ReactNode;
onClick: (e: React.MouseEvent) => void;
status: TraceTreeNode<any>['fetchStatus'] | undefined;
errored?: boolean;
}) {
return (
<button className="TraceChildrenCount" onClick={props.onClick}>
<button
className={`TraceChildrenCount ${props.errored ? 'Errored' : ''}`}
onClick={props.onClick}
>
<div className="TraceChildrenCountContent">{props.children}</div>
<div className="TraceChildrenCountAction">
{props.icon}
Expand Down Expand Up @@ -1608,6 +1620,10 @@ const TraceStylingWrapper = styled('div')`
box-shadow: ${p => p.theme.dropShadowLight};
margin-right: ${space(1)};
&.Errored {
border: 2px solid ${p => p.theme.error};
}
.TraceChildrenCountContent {
+ .TraceChildrenCountAction {
margin-left: 2px;
Expand Down
78 changes: 78 additions & 0 deletions static/app/views/performance/newTraceDetails/traceTree.spec.tsx
View file
Expand Up @@ -1718,6 +1718,31 @@ describe('TraceTree', () => {
expect(root.children.length).toBe(1);
});

it('collects errored children for sibling autogrouped node', () => {
const root = new TraceTreeNode(null, makeSpan({description: 'span1'}), {
project_slug: '',
event_id: '',
});

for (let i = 0; i < 5; i++) {
const node = new TraceTreeNode(root, makeSpan({description: 'span', op: 'db'}), {
project_slug: '',
event_id: '',
});
node.value.relatedErrors = [makeTraceError()];
root.children.push(node);
}

expect(root.children.length).toBe(5);

TraceTree.AutogroupSiblingSpanNodes(root);

expect(root.children.length).toBe(1);
assertAutogroupedNode(root.children[0]);
expect(root.children[0].has_error).toBe(true);
expect(root.children[0].errored_children).toHaveLength(5);
});

it('adds autogrouped siblings as children under autogrouped node', () => {
const root = new TraceTreeNode(null, makeSpan({description: 'span1'}), {
project_slug: '',
Expand Down Expand Up @@ -1825,6 +1850,59 @@ describe('TraceTree', () => {
);
});

it('collects errored children for parent autogrouped node', () => {
// db db db
// http -> parent autogroup (3) -> parent autogroup (3)
// http http
// http http
// http

const root: TraceTreeNode<TraceTree.Span> = new TraceTreeNode(
null,
makeSpan({
description: `span1`,
span_id: `1`,
op: 'db',
}),
{project_slug: '', event_id: ''}
);

let last: TraceTreeNode<any> = root;

for (let i = 0; i < 3; i++) {
const node = new TraceTreeNode(
last,
makeSpan({
description: `span${i}`,
span_id: `${i}`,
op: 'http',
}),
{
project_slug: '',
event_id: '',
}
);
node.value.relatedErrors = [makeTraceError()];
last.children.push(node);
last = node;
}

if (!root) {
throw new Error('root is null');
}

expect(root.children.length).toBe(1);
expect(root.children[0].children.length).toBe(1);

TraceTree.AutogroupDirectChildrenSpanNodes(root);

expect(root.children.length).toBe(1);

assertAutogroupedNode(root.children[0]);
expect(root.children[0].has_error).toBe(true);
expect(root.children[0].errored_children).toHaveLength(3);
});

it('autogrouping direct children skips rendering intermediary nodes', () => {
// db db db
// http autogrouped (3) autogrouped (3)
Expand Down
77 changes: 65 additions & 12 deletions static/app/views/performance/newTraceDetails/traceTree.tsx
View file
Expand Up @@ -413,7 +413,9 @@ export class TraceTree {
const spanNodeValue: TraceTree.Span = {
...span,
event: data as EventTransaction,
relatedErrors: childTxn ? getRelatedErrorsOrIssues(span, childTxn.value) : [],
relatedErrors: childTxn
? getSpanErrorsOrIssuesFromTransaction(span, childTxn.value)
: [],
childTxn: childTxn?.value,
};
const node: TraceTreeNode<TraceTree.Span> = new TraceTreeNode(null, spanNodeValue, {
Expand Down Expand Up @@ -478,17 +480,28 @@ export class TraceTree {
const head = node;
let tail = node;
let groupMatchCount = 0;
const erroredChildren: TraceTreeNode<TraceTree.NodeValue>[] = [];

while (
tail &&
tail.children.length === 1 &&
isSpanNode(tail.children[0]) &&
tail.children[0].value.op === head.value.op
) {
if (tail.value?.relatedErrors.length > 0) {
erroredChildren.push(tail);
}

groupMatchCount++;
tail = tail.children[0];
}

// Checking the tail node for errors as it is not included in the grouping
// while loop, but is hidden when the autogrouped node is collapsed
if (tail.value?.relatedErrors.length > 0) {
erroredChildren.push(tail);
}

if (groupMatchCount < 1) {
for (const child of head.children) {
queue.push(child);
Expand Down Expand Up @@ -517,6 +530,7 @@ export class TraceTree {
}

autoGroupedNode.groupCount = groupMatchCount + 1;
autoGroupedNode.errored_children = erroredChildren;
autoGroupedNode.space = [
Math.min(head.value.start_timestamp, tail.value.timestamp) *
autoGroupedNode.multiplier,
Expand Down Expand Up @@ -611,6 +625,16 @@ export class TraceTree {
start_timestamp = child.value.start_timestamp;
}

if (!isSpanNode(child)) {
throw new TypeError(
'Expected child of autogrouped node to be a span node.'
);
}

if (child.value?.relatedErrors.length > 0) {
autoGroupedNode.errored_children.push(child);
}

autoGroupedNode.children.push(node.children[j]);
autoGroupedNode.children[autoGroupedNode.children.length - 1].parent =
autoGroupedNode;
Expand Down Expand Up @@ -1257,6 +1281,7 @@ export class MissingInstrumentationNode extends TraceTreeNode<TraceTree.MissingI
export class ParentAutogroupNode extends TraceTreeNode<TraceTree.ChildrenAutogroup> {
head: TraceTreeNode<TraceTree.Span>;
tail: TraceTreeNode<TraceTree.Span>;
errored_children: TraceTreeNode<TraceTree.NodeValue>[] = [];
groupCount: number = 0;

private _autogroupedSegments: [number, number][] | undefined;
Expand All @@ -1282,6 +1307,10 @@ export class ParentAutogroupNode extends TraceTreeNode<TraceTree.ChildrenAutogro
return this.tail.children;
}

get has_error(): boolean {
return this.errored_children.length > 0;
}

get autogroupedSegments(): [number, number][] {
if (this._autogroupedSegments) {
return this._autogroupedSegments;
Expand All @@ -1304,6 +1333,7 @@ export class ParentAutogroupNode extends TraceTreeNode<TraceTree.ChildrenAutogro

export class SiblingAutogroupNode extends TraceTreeNode<TraceTree.SiblingAutogroup> {
groupCount: number = 0;
errored_children: TraceTreeNode<TraceTree.NodeValue>[] = [];
private _autogroupedSegments: [number, number][] | undefined;

constructor(
Expand All @@ -1315,6 +1345,10 @@ export class SiblingAutogroupNode extends TraceTreeNode<TraceTree.SiblingAutogro
this.expanded = false;
}

get has_error(): boolean {
return this.errored_children.length > 0;
}

get autogroupedSegments(): [number, number][] {
if (this._autogroupedSegments) {
return this._autogroupedSegments;
Expand Down Expand Up @@ -1505,20 +1539,39 @@ export function computeAutogroupedBarSegments(
return segments;
}

function getRelatedErrorsOrIssues(
// Returns a list of errors or performance issues related to the txn
// with ids matching the span id
function getSpanErrorsOrIssuesFromTransaction(
span: RawSpanType,
currentEvent: TraceTree.Transaction
txn: TraceTree.Transaction
): TraceErrorOrIssue[] {
const performanceIssues = currentEvent.performance_issues.filter(
issue =>
issue.span.some(id => id === span.span_id) ||
issue.suspect_spans.some(suspectSpanId => suspectSpanId === span.span_id)
);
if (!txn.performance_issues.length && !txn.errors.length) {
return [];
}

const errorsOrIssues: TraceErrorOrIssue[] = [];

for (const perfIssue of txn.performance_issues) {
for (const s of perfIssue.span) {
if (s === span.span_id) {
errorsOrIssues.push(perfIssue);
}
}

for (const suspect of perfIssue.suspect_spans) {
if (suspect === span.span_id) {
errorsOrIssues.push(perfIssue);
}
}
}

for (const error of txn.errors) {
if (error.span === span.span_id) {
errorsOrIssues.push(error);
}
}

return [
...currentEvent.errors.filter(error => error.span === span.span_id),
...performanceIssues, // Spans can be shown when embedded in performance issues
];
return errorsOrIssues;
}

function printNode(t: TraceTreeNode<TraceTree.NodeValue>, offset: number): string {
Expand Down

0 comments on commit 9ae6f76

Please sign in to comment.