From 086a24453824a9a5f5506e45a348fca12b133cf1 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Sat, 6 Dec 2025 13:21:56 -0500 Subject: [PATCH] feat(trace-tree-node): Mitigating eap span node type guards usage --- .../insights/agents/components/aiSpanList.tsx | 40 +++++++------------ .../newTraceDetails/traceApi/utils.tsx | 6 --- .../newTraceDetails/traceGuards.tsx | 8 ---- .../traceModels/traceTreeNode/baseNode.tsx | 4 +- .../traceModels/traceTreeNode/eapSpanNode.tsx | 16 ++++---- 5 files changed, 25 insertions(+), 49 deletions(-) diff --git a/static/app/views/insights/agents/components/aiSpanList.tsx b/static/app/views/insights/agents/components/aiSpanList.tsx index b1611981728aa3..99fa166af38a9c 100644 --- a/static/app/views/insights/agents/components/aiSpanList.tsx +++ b/static/app/views/insights/agents/components/aiSpanList.tsx @@ -22,10 +22,7 @@ import { } from 'sentry/views/insights/agents/utils/query'; import type {AITraceSpanNode} from 'sentry/views/insights/agents/utils/types'; import {SpanFields} from 'sentry/views/insights/types'; -import { - isEAPSpanNode, - isTransactionNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; +import {isTransactionNode} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode'; import type {TransactionNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode'; @@ -275,26 +272,18 @@ function getNodeInfo(node: AITraceSpanNode, colors: readonly string[]) { return nodeInfo; } - const getNodeAttribute = (key: string) => { - if (isEAPSpanNode(node)) { - return node.value.additional_attributes?.[key]; - } - - return node.value?.data?.[key]; - }; - const op = node.op ?? 'default'; const truncatedOp = op.startsWith('gen_ai.') ? op.slice(7) : op; nodeInfo.title = truncatedOp; if (getIsAiRunSpan({op}) || getIsAiCreateAgentSpan({op})) { const agentName = - getNodeAttribute(SpanFields.GEN_AI_AGENT_NAME) || - getNodeAttribute(SpanFields.GEN_AI_FUNCTION_ID) || + node.attributes?.[SpanFields.GEN_AI_AGENT_NAME] || + node.attributes?.[SpanFields.GEN_AI_FUNCTION_ID] || ''; const model = - getNodeAttribute(SpanFields.GEN_AI_REQUEST_MODEL) || - getNodeAttribute(SpanFields.GEN_AI_RESPONSE_MODEL) || + node.attributes?.[SpanFields.GEN_AI_REQUEST_MODEL] || + node.attributes?.[SpanFields.GEN_AI_RESPONSE_MODEL] || ''; nodeInfo.icon = ; nodeInfo.title = agentName || truncatedOp; @@ -308,8 +297,12 @@ function getNodeInfo(node: AITraceSpanNode, colors: readonly string[]) { } nodeInfo.color = colors[0]; } else if (getIsAiGenerationSpan({op})) { - const tokens = getNodeAttribute(SpanFields.GEN_AI_USAGE_TOTAL_TOKENS); - const cost = getNodeAttribute(SpanFields.GEN_AI_USAGE_TOTAL_COST); + const tokens = node.attributes?.[SpanFields.GEN_AI_USAGE_TOTAL_TOKENS] as + | number + | undefined; + const cost = node.attributes?.[SpanFields.GEN_AI_USAGE_TOTAL_COST] as + | number + | undefined; nodeInfo.icon = ; nodeInfo.subtitle = tokens ? ( @@ -328,7 +321,7 @@ function getNodeInfo(node: AITraceSpanNode, colors: readonly string[]) { } nodeInfo.color = colors[2]; } else if (getIsExecuteToolSpan({op})) { - const toolName = getNodeAttribute(SpanFields.GEN_AI_TOOL_NAME); + const toolName = node.attributes?.[SpanFields.GEN_AI_TOOL_NAME] as string | undefined; nodeInfo.icon = ; nodeInfo.title = toolName || truncatedOp; nodeInfo.subtitle = toolName ? truncatedOp : ''; @@ -355,12 +348,9 @@ function hasError(node: AITraceSpanNode) { return true; } - if (isEAPSpanNode(node)) { - const status = node.value.additional_attributes?.[SpanFields.SPAN_STATUS]; - if (typeof status === 'string') { - return status.includes('error'); - } - return false; + const status = node.attributes?.[SpanFields.SPAN_STATUS] as string | undefined; + if (typeof status === 'string') { + return status.includes('error'); } return false; diff --git a/static/app/views/performance/newTraceDetails/traceApi/utils.tsx b/static/app/views/performance/newTraceDetails/traceApi/utils.tsx index ad7c2ee57b471b..c2b0e412d65530 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/utils.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/utils.tsx @@ -3,7 +3,6 @@ import type {OurLogsResponseItem} from 'sentry/views/explore/logs/types'; import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; import { isEAPTraceNode, - isEAPTransaction, isRootEvent, isTraceNode, isTraceSplitResult, @@ -74,11 +73,6 @@ export const getRepresentativeTraceEvent = ( break; } - if (isEAPTransaction(event)) { - // If we find a root EAP transaction, we can stop looking and use it for the title. - break; - } - // Otherwise we keep looking for a root eap transaction. If we don't find one, we use other roots, like standalone spans. continue; } else if ( // If we haven't found a root transaction, but we found a candidate transaction diff --git a/static/app/views/performance/newTraceDetails/traceGuards.tsx b/static/app/views/performance/newTraceDetails/traceGuards.tsx index af3de4cef3a7d9..84c6f6f2804d0d 100644 --- a/static/app/views/performance/newTraceDetails/traceGuards.tsx +++ b/static/app/views/performance/newTraceDetails/traceGuards.tsx @@ -31,14 +31,6 @@ export function isEAPSpan(value: TraceTree.NodeValue): value is TraceTree.EAPSpa return !!(value && 'is_transaction' in value); } -export function isEAPTransaction(value: TraceTree.NodeValue): value is TraceTree.EAPSpan { - return isEAPSpan(value) && value.is_transaction; -} - -export function isEAPTransactionNode(node: BaseNode): node is EapSpanNode { - return isEAPTransaction(node.value); -} - export function isEAPSpanNode(node: BaseNode): node is EapSpanNode { return isEAPSpan(node.value); } diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx index 93cbe40e4b1b5d..a82fe7843bf221 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx @@ -7,7 +7,7 @@ import type {Organization} from 'sentry/types/organization'; import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; import { - isEAPTransactionNode, + isEAPSpanNode, isTransactionNode, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; @@ -471,7 +471,7 @@ export abstract class BaseNode(p => isEAPTransactionNode(p)); + return this.findParent(p => isEAPSpanNode(p) && p.value.is_transaction); } expand(expanding: boolean, tree: TraceTree): boolean { diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx index ff1629e3b2d18a..94498c6b3a995f 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx @@ -6,9 +6,7 @@ import {SpanNodeDetails} from 'sentry/views/performance/newTraceDetails/traceDra import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; import { isBrowserRequestNode, - isEAPSpan, isEAPSpanNode, - isEAPTransaction, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import {TraceEAPSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceEAPSpanRow'; @@ -35,7 +33,7 @@ export class EapSpanNode extends BaseNode { // For eap transactions, on load we only display the embedded transactions children. // Mimics the behavior of non-eap traces, enabling a less noisy/summarized view of the trace const parentNode = value.is_transaction - ? (parent?.findParent(p => isEAPTransaction(p.value)) ?? parent) + ? (parent?.findParent(p => isEAPSpanNode(p) && p.value.is_transaction) ?? parent) : parent; super(parentNode, value, extra); @@ -141,10 +139,12 @@ export class EapSpanNode extends BaseNode { } get directVisibleChildren(): Array> { - if (isEAPTransaction(this.value) && !this.expanded) { + if (this.value.is_transaction && !this.expanded) { // For collapsed eap-transactions we still render the embedded eap-transactions as visible children. // Mimics the behavior of non-eap traces, enabling a less noisy/summarized view of the trace - return this.children.filter(child => isEAPTransaction(child.value)); + return this.children.filter( + child => isEAPSpanNode(child) && child.value.is_transaction + ); } return super.directVisibleChildren; @@ -181,14 +181,14 @@ export class EapSpanNode extends BaseNode { // the eap-spans (by their parent_span_id) that were previously hidden. Note that this only impacts the // direct eap-transaction children of the targetted eap-transaction node. if (this.value.is_transaction) { - const eapTransactions = this.children.filter(c => - isEAPTransaction(c.value) + const eapTransactions = this.children.filter( + c => isEAPSpanNode(c) && c.value.is_transaction ) as EapSpanNode[]; for (const txn of eapTransactions) { // Find the eap-span that is the parent of the transaction const newParent = this.findChild(n => { - if (isEAPSpan(n.value)) { + if (isEAPSpanNode(n)) { return n.value.event_id === txn.value.parent_span_id; } return false;