Skip to content

feat(trace-tree-node): Mitigating SpanNode type guard usage #104481

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.

Sign up for GitHub

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

Open
Abdkhan14 wants to merge 3 commits into
base: abdk/trace-tree-node-missing-ins-guards
Choose a base branch
from
Open

feat(trace-tree-node): Mitigating SpanNode type guard usage #104481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b6ce85d
feat(trace-tree-node): Mitigating SpanNode type guard usage
Dec 5, 2025
0e96967
Merge branch 'abdk/trace-tree-node-missing-ins-guards' into abdk/trac…
Dec 6, 2025
dfe2925
feat(trace-tree-node): Fixing knip errors
Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 31 additions & 113 deletions static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,124 +1,40 @@
import {useMemo} from 'react';

import {ExternalLink} from 'sentry/components/core/link';
import LoadingError from 'sentry/components/loadingError';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import {t, tct} from 'sentry/locale';
import type {EventTransaction} from 'sentry/types/event';
import type {Project} from 'sentry/types/project';
import useProjects from 'sentry/utils/useProjects';
import {useTransaction} from 'sentry/views/performance/newTraceDetails/traceApi/useTransaction';
import {getCustomInstrumentationLink} from 'sentry/views/performance/newTraceDetails/traceConfigurations';
import {ProfilePreview} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/profiling/profilePreview';
import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails';
import {
isEAPSpanNode,
isSpanNode,
} from 'sentry/views/performance/newTraceDetails/traceGuards';
import type {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode';
import type {NoInstrumentationNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/noInstrumentationNode';
import {ProfileGroupProvider} from 'sentry/views/profiling/profileGroupProvider';
import {ProfileContext, ProfilesProvider} from 'sentry/views/profiling/profilesProvider';

import {TraceDrawerComponents} from './styles';
import {getProfileMeta} from './utils';

interface BaseProps extends TraceTreeNodeDetailsProps<NoInstrumentationNode> {
event: EventTransaction | null;
profileId: string | undefined;
profileMeta: ReturnType<typeof getProfileMeta>;
profilerId: string | undefined;
project: Project | undefined;
}

export function MissingInstrumentationNodeDetails({
...props
}: TraceTreeNodeDetailsProps<NoInstrumentationNode>) {
const {node} = props;
const {projects} = useProjects();
const previous = node.previous;

if (isEAPSpanNode(node.previous)) {
Copy link
Contributor Author

@Abdkhan14 Abdkhan14 Dec 9, 2025

Choose a reason for hiding this comment

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

Combined the two MissingInstrumentationNodeDetails to one.

return <EAPMissingInstrumentationNodeDetails {...props} projects={projects} />;
}

if (isSpanNode(node.previous)) {
const event = node.previous.event;
const project = projects.find(proj => proj.slug === event?.projectSlug);
const profileMeta = getProfileMeta(event) || '';
const profileContext = event?.contexts?.profile ?? {};

return (
<BaseMissingInstrumentationNodeDetails
{...props}
profileMeta={profileMeta}
project={project}
event={event}
profileId={profileContext.profile_id}
profilerId={profileContext.profiler_id}
/>
);
}

return null;
}

function EAPMissingInstrumentationNodeDetails({
projects,
...props
}: TraceTreeNodeDetailsProps<NoInstrumentationNode> & {
projects: Project[];
}) {
const {node} = props;
const previous = node.previous as EapSpanNode;

const {
data: eventTransaction = null,
isLoading: isEventTransactionLoading,
isError: isEventTransactionError,
} = useTransaction({
event_id: previous.value.transaction_id,
const {data: eventTransaction = null} = useTransaction({
event_id: previous.transactionId ?? '',
organization: props.organization,
project_slug: previous.value.project_slug,
project_slug: previous.projectSlug ?? '',
Comment on lines 23 to +27
Copy link

@sentry sentry bot Dec 9, 2025

Choose a reason for hiding this comment

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

Bug: Refactoring removed type guards, causing SpanNodes to pass an empty project_slug to useTransaction due to incorrect property access.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The refactoring removed type guards, causing SpanNodes to be incorrectly processed. The new unified code project_slug: previous.projectSlug ?? '' relies on BaseNode.projectSlug getter. For SpanNodes, this.value (of type TraceTree.Span) lacks a project_slug property, causing BaseNode.projectSlug to return undefined. This undefined is then coerced to an empty string ''. Consequently, the useTransaction hook receives an empty project_slug, preventing correct data fetching and breaking the missing instrumentation feature for SpanNodes.

💡 Suggested Fix

Reintroduce type-specific logic to correctly extract project_slug for SpanNodes, likely by accessing node.previous.event?.projectSlug or adjusting the BaseNode.projectSlug getter to handle SpanNode's structure.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.tsx#L23-L27

Potential issue: The refactoring removed type guards, causing `SpanNode`s to be
incorrectly processed. The new unified code `project_slug: previous.projectSlug ?? ''`
relies on `BaseNode.projectSlug` getter. For `SpanNode`s, `this.value` (of type
`TraceTree.Span`) lacks a `project_slug` property, causing `BaseNode.projectSlug` to
return `undefined`. This `undefined` is then coerced to an empty string `''`.
Consequently, the `useTransaction` hook receives an empty `project_slug`, preventing
correct data fetching and breaking the missing instrumentation feature for `SpanNode`s.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6321181

Copy link
Contributor Author

@Abdkhan14 Abdkhan14 Dec 9, 2025

Choose a reason for hiding this comment

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

Invalid

Copy link
Contributor Author

@Abdkhan14 Abdkhan14 Dec 9, 2025

Choose a reason for hiding this comment

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

We assign the parent's projectSlug for spanNodes.

});

const profileMeta = useMemo(
() => getProfileMeta(eventTransaction) || '',
[eventTransaction]
);

if (isEventTransactionLoading) {
return <LoadingIndicator />;
}

if (isEventTransactionError) {
return <LoadingError message={t('Failed to fetch span details')} />;
}

const project = projects.find(proj => proj.slug === eventTransaction?.projectSlug);
const profileContext = eventTransaction?.contexts?.profile ?? {};

return (
<BaseMissingInstrumentationNodeDetails
{...props}
profileMeta={profileMeta}
project={project}
event={eventTransaction}
profileId={profileContext.profile_id}
profilerId={profileContext.profiler_id}
/>
);
}

function BaseMissingInstrumentationNodeDetails({
node,
organization,
onTabScrollToNode,
profileMeta,
project,
event,
profileId,
profilerId,
}: BaseProps) {
return (
<TraceDrawerComponents.DetailContainer>
<TraceDrawerComponents.HeaderContainer>
Expand All @@ -135,8 +51,8 @@ function BaseMissingInstrumentationNodeDetails({
</TraceDrawerComponents.Title>
<TraceDrawerComponents.NodeActions
node={node}
organization={organization}
onTabScrollToNode={onTabScrollToNode}
organization={props.organization}
onTabScrollToNode={props.onTabScrollToNode}
/>
</TraceDrawerComponents.HeaderContainer>
<TraceDrawerComponents.BodyContainer>
Expand All @@ -150,29 +66,31 @@ function BaseMissingInstrumentationNodeDetails({
}
)}
</p>
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={event?.projectSlug ?? ''}
profileMeta={profileMeta || ''}
>
<ProfileContext.Consumer>
{profiles => (
<ProfileGroupProvider
type="flamechart"
input={profiles?.type === 'resolved' ? profiles.data : null}
traceID={profileId ?? ''}
>
<ProfilePreview
project={project}
profileID={profileId}
profilerID={profilerId}
event={event}
missingInstrumentationNode={node}
/>
</ProfileGroupProvider>
)}
</ProfileContext.Consumer>
</ProfilesProvider>
{eventTransaction ? (
<ProfilesProvider
orgSlug={props.organization.slug}
projectSlug={eventTransaction?.projectSlug ?? ''}
profileMeta={profileMeta || ''}
>
<ProfileContext.Consumer>
{profiles => (
<ProfileGroupProvider
type="flamechart"
input={profiles?.type === 'resolved' ? profiles.data : null}
traceID={profileContext.profile_id ?? ''}
>
<ProfilePreview
project={project}
profileID={profileContext.profile_id ?? ''}
profilerID={profileContext.profiler_id ?? ''}
event={eventTransaction}
missingInstrumentationNode={node}
/>
</ProfileGroupProvider>
)}
</ProfileContext.Consumer>
</ProfilesProvider>
) : null}
<p>
{t("If you'd prefer, you can also turn the feature off in the settings above.")}
</p>
Expand Down
12 changes: 6 additions & 6 deletions static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/keys.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
type SectionCardKeyValueList,
} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/styles';
import {TraceDrawerActionValueKind} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/utils';
import {isSpanNode} from 'sentry/views/performance/newTraceDetails/traceGuards';
import type {SpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/spanNode';
import {getPerformanceDuration} from 'sentry/views/performance/utils/getPerformanceDuration';

Expand Down Expand Up @@ -77,11 +76,12 @@ function getSpanAggregateMeasurements(node: SpanNode) {

let sum = 0;
node.forEachChild(n => {
if (
isSpanNode(n) &&
typeof n?.value?.measurements?.ai_total_tokens_used?.value === 'number'
) {
sum += n.value.measurements.ai_total_tokens_used.value;
const tokens =
typeof n.measurements?.ai_total_tokens_used === 'number'
? n.measurements?.ai_total_tokens_used
: (n.measurements?.ai_total_tokens_used?.value ?? 0);
if (tokens) {
sum += tokens;
}
});
return [
Expand Down
9 changes: 0 additions & 9 deletions static/app/views/performance/newTraceDetails/traceGuards.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {SiblingAutogroupNode} from './traceModels/traceTreeNode/siblingAuto
import type {SpanNode} from './traceModels/traceTreeNode/spanNode';
import type {TransactionNode} from './traceModels/traceTreeNode/transactionNode';
import type {UptimeCheckNode} from './traceModels/traceTreeNode/uptimeCheckNode';
import type {UptimeCheckTimingNode} from './traceModels/traceTreeNode/uptimeCheckTimingNode';

export function isMissingInstrumentationNode(
node: BaseNode
Expand Down Expand Up @@ -47,14 +46,6 @@ export function isUptimeCheckNode(node: BaseNode): node is UptimeCheckNode {
return isUptimeCheck(node.value);
}

export function isUptimeCheckTimingNode(node: BaseNode): node is UptimeCheckTimingNode {
return !!(
node.value &&
'event_type' in node.value &&
node.value.event_type === 'uptime_check_timing'
);
}

export function isTransactionNode(node: BaseNode): node is TransactionNode {
return !!(node.value && 'transaction.op' in node.value);
}
Expand Down
11 changes: 10 additions & 1 deletion static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ import {
isSpanNode,
isTransactionNode,
isUptimeCheckNode,
isUptimeCheckTimingNode,
} from './../traceGuards';
import type {BaseNode} from './traceTreeNode/baseNode';
import type {EapSpanNode} from './traceTreeNode/eapSpanNode';
import type {ParentAutogroupNode} from './traceTreeNode/parentAutogroupNode';
import type {SiblingAutogroupNode} from './traceTreeNode/siblingAutogroupNode';
import type {UptimeCheckTimingNode} from './traceTreeNode/uptimeCheckTimingNode';
import {TraceShape, TraceTree} from './traceTree';
import {
assertEAPSpanNode,
Expand Down Expand Up @@ -2043,6 +2044,14 @@ describe('TraceTree', () => {
});

describe('uptime check integration', () => {
function isUptimeCheckTimingNode(node: BaseNode): node is UptimeCheckTimingNode {
return !!(
node.value &&
'event_type' in node.value &&
node.value.event_type === 'uptime_check_timing'
);
}

it('automatically creates timing nodes when uptime check node is created', () => {
const uptimeCheck = makeUptimeCheck({
additional_attributes: {
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ export class TraceTree extends TraceTreeEventDispatcher {
tree,
c,
c.space[0],
c.value.measurements,
c.measurements,
tree.vitals,
tree.vital_types
)
Expand Down Expand Up @@ -678,7 +678,7 @@ export class TraceTree extends TraceTreeEventDispatcher {
tree,
node,
baseTraceNode.space[0],
node.value.measurements,
node.measurements,
this.vitals,
this.vital_types
)
Expand Down
7 changes: 7 additions & 0 deletions static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {Theme} from '@emotion/react';

import type {Client} from 'sentry/api';
import {pickBarColor} from 'sentry/components/performance/waterfall/utils';
import type {Measurement} from 'sentry/types/event';
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';
Expand Down Expand Up @@ -326,6 +327,12 @@ export abstract class BaseNode<T extends TraceTree.NodeValue = TraceTree.NodeVal
: undefined;
}

get measurements(): Record<string, number> | Record<string, Measurement> | undefined {
return this.value && 'measurements' in this.value
? this.value.measurements
: undefined;
}

get attributes(): Record<string, string | number | boolean> | undefined {
return this.value && 'additional_attributes' in this.value
? this.value.additional_attributes
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
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';
import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow';
import {TraceSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceSpanRow';

import {BaseNode, type TraceTreeNodeExtra} from './baseNode';
import {traceChronologicalSort} from './utils';
Expand Down Expand Up @@ -282,7 +282,7 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> {
renderWaterfallRow<T extends TraceTree.Node = TraceTree.Node>(
props: TraceRowProps<T>
): React.ReactNode {
return <TraceSpanRow {...props} node={this} />;
return <TraceEAPSpanRow {...props} node={this} />;
}

renderDetails<T extends BaseNode>(
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {UptimeNodeDetails} from 'sentry/views/performance/newTraceDetails/traceD
import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails';
import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree';
import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow';
import {TraceSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceSpanRow';
import {TraceUptimeCheckNodeRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceUptimeCheckNode';

import {BaseNode, type TraceTreeNodeExtra} from './baseNode';
import {UptimeCheckTimingNode} from './uptimeCheckTimingNode';
Expand Down Expand Up @@ -140,7 +140,7 @@ export class UptimeCheckNode extends BaseNode<TraceTree.UptimeCheck> {
renderWaterfallRow<NodeType extends TraceTree.Node = TraceTree.Node>(
props: TraceRowProps<NodeType>
): React.ReactNode {
return <TraceSpanRow {...props} node={this} />;
return <TraceUptimeCheckNodeRow {...props} node={this} />;
}

renderDetails<NodeType extends BaseNode>(
Expand Down
4 changes: 2 additions & 2 deletions ...app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {UptimeTimingDetails} from 'sentry/views/performance/newTraceDetails/trac
import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails';
import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree';
import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow';
import {TraceSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceSpanRow';
import {TraceUptimeCheckTimingNodeRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceUptimeCheckTimingNode';

import {BaseNode} from './baseNode';

Expand Down Expand Up @@ -33,7 +33,7 @@ export class UptimeCheckTimingNode extends BaseNode<TraceTree.UptimeCheckTiming>
props: TraceRowProps<NodeType>
): React.ReactNode {
return (
<TraceSpanRow
<TraceUptimeCheckTimingNodeRow
{...props}
// Won't need this cast once we use BaseNode type for props.node
node={this}
Expand Down
Loading
Loading