From a8a83880b41e4d399192fc583fffcffb386366f8 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 13 Apr 2026 22:19:33 +0000 Subject: [PATCH 1/2] feat(trace-view): expose span attributes in trace and span views (#736) Add --full flag to `trace view` that fetches full span attributes via the per-span trace-items detail endpoint. Auto-enabled in --json mode so JSON consumers (agents, scripts) get complete data by default. - Extract shared `attributesToDict` and `fetchMultiSpanDetails` from span/view.ts into api/traces.ts for reuse across both commands - Fetch details for ALL spans (no cap) with concurrency 15; progress on stderr for >20 spans, informational warning for >500 spans - Merge `data` dicts onto each span in JSON output (consistent with existing span view --json shape) - Human output notes detail count; hint suggests --full or --json - Property-based tests for flattenSpanTree; unit tests for --full and --json auto-enable behavior --- .../skills/sentry-cli/references/trace.md | 1 + src/commands/span/view.ts | 70 +----- src/commands/trace/view.ts | 233 +++++++++++++++--- src/lib/api-client.ts | 8 +- src/lib/api/traces.ts | 86 +++++++ test/commands/trace/view.func.test.ts | 158 +++++++++++- test/commands/trace/view.property.test.ts | 117 +++++++++ 7 files changed, 569 insertions(+), 104 deletions(-) diff --git a/plugins/sentry-cli/skills/sentry-cli/references/trace.md b/plugins/sentry-cli/skills/sentry-cli/references/trace.md index 052be8bb1..2d034feba 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/trace.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/trace.md @@ -56,6 +56,7 @@ View details of a specific trace **Flags:** - `-w, --web - Open in browser` +- `--full - Fetch full span attributes (auto-enabled with --json)` - `--spans - Span tree depth limit (number, "all" for unlimited, "no" to disable) - (default: "3")` - `-f, --fresh - Bypass cache, re-detect projects, and fetch fresh data` diff --git a/src/commands/span/view.ts b/src/commands/span/view.ts index 1a9f86be3..f7f4cfb1c 100644 --- a/src/commands/span/view.ts +++ b/src/commands/span/view.ts @@ -4,14 +4,12 @@ * View detailed information about one or more spans within a trace. */ -import pLimit from "p-limit"; - import type { SentryContext } from "../../context.js"; -import type { TraceItemDetail } from "../../lib/api-client.js"; import { + attributesToDict, + fetchMultiSpanDetails, getDetailedTrace, - getSpanDetails, - REDUNDANT_DETAIL_ATTRS, + type TraceItemDetail, } from "../../lib/api-client.js"; import { spansFlag } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; @@ -45,27 +43,6 @@ import { const log = logger.withTag("span.view"); -/** Concurrency limit for parallel span detail fetches */ -const SPAN_DETAIL_CONCURRENCY = 5; - -/** - * Convert a trace-items attribute array into a key-value dict, - * filtering out attributes already shown in the standard span fields - * and EAP storage internals (tags[], precise timestamps, etc.). - */ -function attributesToDict( - attributes: TraceItemDetail["attributes"] -): Record { - return Object.fromEntries( - attributes - .filter( - (a) => - !(REDUNDANT_DETAIL_ATTRS.has(a.name) || a.name.startsWith("tags[")) - ) - .map((a) => [a.name, a.value]) - ); -} - type ViewFlags = { readonly json: boolean; readonly spans: number; @@ -288,39 +265,6 @@ function formatSpanViewHuman(data: SpanViewData): string { return parts.join(""); } -/** - * Fetch full attribute details for found spans in parallel. - * - * Uses p-limit to cap concurrency and avoid overwhelming the API. - * Failures for individual spans are logged as warnings — the command - * still returns the basic span data from the trace tree. - */ -async function fetchSpanDetails( - results: SpanResult[], - org: string, - project: string, - traceId: string -): Promise> { - const limit = pLimit(SPAN_DETAIL_CONCURRENCY); - const details = new Map(); - - await limit.map(results, async (r) => { - try { - const detail = await getSpanDetails( - org, - r.span.project_slug || project, - r.spanId, - traceId - ); - details.set(r.spanId, detail); - } catch { - log.warn(`Could not fetch details for span ${r.spanId}`); - } - }); - - return details; -} - /** * Transform span view data for JSON output. * Applies `--fields` filtering per element. @@ -423,7 +367,13 @@ export const viewCommand = buildCommand({ // Fetch full attribute details for each found span in parallel. // Uses the trace-items detail endpoint which returns ALL attributes. - const details = await fetchSpanDetails(results, org, project, traceId); + const details = await fetchMultiSpanDetails( + results.map((r) => ({ + span_id: r.spanId, + project_slug: r.span.project_slug, + })), + { org, fallbackProject: project, traceId } + ); yield new CommandOutput({ results, diff --git a/src/commands/trace/view.ts b/src/commands/trace/view.ts index 192b46f08..2326b6691 100644 --- a/src/commands/trace/view.ts +++ b/src/commands/trace/view.ts @@ -6,9 +6,12 @@ import type { SentryContext } from "../../context.js"; import { + attributesToDict, + fetchMultiSpanDetails, getDetailedTrace, getIssueByShortId, getLatestEvent, + type TraceItemDetail, } from "../../lib/api-client.js"; import { detectSwappedViewArgs, @@ -46,6 +49,7 @@ type ViewFlags = { readonly web: boolean; readonly spans: number; readonly fresh: boolean; + readonly full: boolean; readonly fields?: string[]; }; @@ -166,6 +170,8 @@ export type TraceViewData = { spans: unknown[]; /** Pre-formatted span tree lines for human output (not serialized) */ spanTreeLines?: string[]; + /** Per-span attribute details from trace-items endpoint (when --full or --json) */ + details?: Map; }; /** @@ -187,12 +193,74 @@ function countSpansWithAdditionalAttrs(spans: unknown[]): number { return count; } +/** Span count threshold for the large-trace informational warning */ +const LARGE_TRACE_WARN_THRESHOLD = 500; + +/** Span count threshold for showing progress indicator */ +const PROGRESS_THRESHOLD = 20; + +/** + * Flatten a nested span tree into an array in depth-first order. + * Collects ALL spans — no cap. + * + * @internal Exported for testing + */ +export function flattenSpanTree(spans: TraceSpan[]): TraceSpan[] { + const result: TraceSpan[] = []; + // Reverse so the first child is popped first (depth-first order) + const stack = [...spans].reverse(); + let span = stack.pop(); + while (span) { + result.push(span); + if (span.children) { + for (let i = span.children.length - 1; i >= 0; i--) { + const child = span.children[i]; + if (child) { + stack.push(child); + } + } + } + span = stack.pop(); + } + return result; +} + +/** + * Recursively merge detail data dicts onto a span tree for JSON output. + * Each span with a matching detail entry gets a `data` dict containing + * filtered custom attributes. + */ +function mergeSpanDetails( + span: TraceSpan, + details: Map +): TraceSpan & { data?: Record } { + const { children, ...rest } = span; + const result: TraceSpan & { data?: Record } = { ...rest }; + + if (span.span_id) { + const detail = details.get(span.span_id); + if (detail) { + const data = attributesToDict(detail.attributes); + if (Object.keys(data).length > 0) { + result.data = data; + } + } + } + + if (children) { + result.children = children.map((c) => mergeSpanDetails(c, details)); + } + + return result; +} + /** * Format trace view data for human-readable terminal output. * * Renders trace summary and optional span tree. * When spans carry `additional_attributes` (requested via `--fields`), * a note is appended indicating how many spans have extra data. + * When `details` are present (from --full or --json), notes their count. * * @internal Exported for testing */ @@ -205,6 +273,7 @@ export function formatTraceView(data: TraceViewData): string { parts.push(data.spanTreeLines.join("\n")); } + // Note spans with additional_attributes (from --fields) const attrsCount = countSpansWithAdditionalAttrs(data.spans); if (attrsCount > 0) { const spanWord = attrsCount === 1 ? "span has" : "spans have"; @@ -213,6 +282,13 @@ export function formatTraceView(data: TraceViewData): string { ); } + // Note spans with full detail data (from --full or auto-fetched with --json) + if (data.details && data.details.size > 0) { + parts.push( + `\n${data.details.size} span(s) have attribute data. Use --json to see full details.` + ); + } + return parts.join("\n"); } @@ -265,8 +341,13 @@ function jsonTransformTraceView( data: TraceViewData, fields?: string[] ): unknown { - const { summary, spans } = data; - const cleanedSpans = (spans as TraceSpan[]).map(filterSpanMeasurements); + const { summary, spans, details } = data; + const cleanedSpans = (spans as TraceSpan[]).map((span) => { + const cleaned = filterSpanMeasurements(span); + return details && details.size > 0 + ? mergeSpanDetails(cleaned, details) + : cleaned; + }); const result: Record = { ...summary, spans: cleanedSpans }; if (fields && fields.length > 0) { return filterFields(result, fields); @@ -274,6 +355,88 @@ function jsonTransformTraceView( return result; } +/** Resolved trace target: org, project (optional), and trace ID */ +type ResolvedTrace = { + traceId: string; + org: string; + project?: string; +}; + +/** + * Resolve a trace from an issue short ID by looking up the issue, + * fetching its latest event, and extracting the trace ID. + */ +async function resolveTraceFromIssue( + issueShortId: string, + cwd: string +): Promise { + const log = logger.withTag("trace.view"); + log.warn( + `'${issueShortId}' is an issue short ID, not a trace ID. Looking up the issue's trace.` + ); + + const resolved = await resolveOrg({ cwd }); + if (!resolved) { + throw new ContextError("Organization", `sentry issue view ${issueShortId}`); + } + const org = resolved.org; + + const issue = await getIssueByShortId(org, issueShortId); + let project: string | undefined; + if (issue.project?.slug) { + setOrgProjectContext([org], [issue.project.slug]); + project = issue.project.slug; + } + + const event = await getLatestEvent(org, issue.id); + const traceId = event?.contexts?.trace?.trace_id; + if (!traceId) { + throw new ValidationError( + `Could not find a trace for issue '${issueShortId}'. The latest event has no trace context.\n\n` + + `Try: sentry issue view ${issueShortId}` + ); + } + + return { traceId, org, project }; +} + +/** + * Fetch per-span details when --full or --json is active. + * + * Extracted from func() to keep cognitive complexity under the Biome + * limit of 15. Logs a warning for large traces and reports progress + * on stderr for traces with more than {@link PROGRESS_THRESHOLD} spans. + */ +function fetchTraceSpanDetails( + spans: TraceSpan[], + totalCount: number, + options: { + org: string; + fallbackProject: string; + traceId: string; + } +): Promise> { + const log = logger.withTag("trace.view"); + const flat = flattenSpanTree(spans); + + if (totalCount > LARGE_TRACE_WARN_THRESHOLD) { + log.warn( + `Trace has ${totalCount} spans \u2014 this may take a moment. ` + + "Use 'sentry span view' for specific spans." + ); + } + + return fetchMultiSpanDetails(flat, { + ...options, + onProgress: + flat.length > PROGRESS_THRESHOLD + ? (done, total) => { + log.info(`Fetching span data (${done}/${total})...`); + } + : undefined, + }); +} + export const viewCommand = buildCommand({ docs: { brief: "View details of a specific trace", @@ -305,6 +468,11 @@ export const viewCommand = buildCommand({ brief: "Open in browser", default: false, }, + full: { + kind: "boolean", + brief: "Fetch full span attributes (auto-enabled with --json)", + default: false, + }, ...spansFlag, fresh: FRESH_FLAG, }, @@ -325,47 +493,15 @@ export const viewCommand = buildCommand({ log.warn(suggestion); } - let traceId: string; - let org: string; - + let resolved: ResolvedTrace; if (issueShortId) { - // Auto-recover: user passed an issue short ID instead of a trace ID. - // Resolve the issue → get its latest event → extract trace ID. - log.warn( - `'${issueShortId}' is an issue short ID, not a trace ID. Looking up the issue's trace.` - ); - - const resolved = await resolveOrg({ cwd }); - if (!resolved) { - throw new ContextError( - "Organization", - `sentry issue view ${issueShortId}` - ); - } - org = resolved.org; - - const issue = await getIssueByShortId(org, issueShortId); - // Enrich telemetry with the issue's project (resolveOrg only sets sentry.org) - if (issue.project?.slug) { - setOrgProjectContext([org], [issue.project.slug]); - } - const event = await getLatestEvent(org, issue.id); - const eventTraceId = event?.contexts?.trace?.trace_id; - if (!eventTraceId) { - throw new ValidationError( - `Could not find a trace for issue '${issueShortId}'. The latest event has no trace context.\n\n` + - `Try: sentry issue view ${issueShortId}` - ); - } - traceId = eventTraceId; + resolved = await resolveTraceFromIssue(issueShortId, cwd); } else { - // Normal flow: parse and resolve org/project/trace-id const parsed = parseTraceTarget(correctedArgs, USAGE_HINT); warnIfNormalized(parsed, "trace.view"); - const resolved = await resolveTraceOrgProject(parsed, cwd, USAGE_HINT); - traceId = resolved.traceId; - org = resolved.org; + resolved = await resolveTraceOrgProject(parsed, cwd, USAGE_HINT); } + const { traceId, org, project } = resolved; if (flags.web) { await openInBrowser(buildTraceUrl(org, traceId), "trace"); @@ -401,9 +537,26 @@ export const viewCommand = buildCommand({ ? formatSimpleSpanTree(traceId, spans, flags.spans) : undefined; - yield new CommandOutput({ summary, spans, spanTreeLines }); + // Fetch per-span details when --full is set or --json auto-enables it + const shouldFetchDetails = flags.full || flags.json; + const spanDetails = shouldFetchDetails + ? await fetchTraceSpanDetails(spans, summary.spanCount, { + org, + fallbackProject: project ?? spans[0]?.project_slug ?? "", + traceId, + }) + : undefined; + + yield new CommandOutput({ + summary, + spans, + spanTreeLines, + details: spanDetails, + }); return { - hint: `Tip: Open in browser with 'sentry trace view --web ${traceId}'`, + hint: shouldFetchDetails + ? `Tip: Open in browser with 'sentry trace view --web ${traceId}'` + : "Tip: Use --full to fetch span attributes, or --json for complete data", }; }, }); diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index a4ca14ab2..1e3e6170b 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -122,8 +122,14 @@ export { listTeams, listTeamsPaginated, } from "./api/teams.js"; -export type { TraceItemAttribute, TraceItemDetail } from "./api/traces.js"; +export type { + FetchMultiSpanDetailsOptions, + TraceItemAttribute, + TraceItemDetail, +} from "./api/traces.js"; export { + attributesToDict, + fetchMultiSpanDetails, getDetailedTrace, getSpanDetails, listSpans, diff --git a/src/lib/api/traces.ts b/src/lib/api/traces.ts index e3a7c2ed4..00b4f7c12 100644 --- a/src/lib/api/traces.ts +++ b/src/lib/api/traces.ts @@ -4,6 +4,8 @@ * Functions for retrieving detailed traces, listing transactions, and listing spans. */ +import pLimit from "p-limit"; + import { type SpanListItem, type SpansResponse, @@ -14,6 +16,7 @@ import { TransactionsResponseSchema, } from "../../types/index.js"; +import { logger } from "../logger.js"; import { resolveOrgRegion } from "../region.js"; import { isAllDigits } from "../utils.js"; @@ -23,6 +26,8 @@ import { parseLinkHeader, } from "./infrastructure.js"; +const log = logger.withTag("api.traces"); + // --------------------------------------------------------------------------- // Trace item (span) detail types // --------------------------------------------------------------------------- @@ -152,6 +157,87 @@ export async function getSpanDetails( return data; } +// --------------------------------------------------------------------------- +// Shared span detail helpers +// --------------------------------------------------------------------------- + +/** Concurrency for parallel detail fetches (shared between trace/span view) */ +const SPAN_DETAIL_CONCURRENCY = 15; + +/** + * Convert a trace-items attribute array into a key-value dict, + * filtering out attributes already shown in the standard span fields + * and EAP storage internals (tags[], precise timestamps, etc.). + */ +export function attributesToDict( + attributes: TraceItemDetail["attributes"] +): Record { + return Object.fromEntries( + attributes + .filter( + (a) => + !(REDUNDANT_DETAIL_ATTRS.has(a.name) || a.name.startsWith("tags[")) + ) + .map((a) => [a.name, a.value]) + ); +} + +/** Options for {@link fetchMultiSpanDetails} */ +export type FetchMultiSpanDetailsOptions = { + /** Organization slug */ + org: string; + /** Project slug to use when a span has no project_slug */ + fallbackProject: string; + /** The parent trace ID (required by the API) */ + traceId: string; + /** Callback fired after each successful fetch for progress reporting */ + onProgress?: (done: number, total: number) => void; +}; + +/** + * Fetch full attribute details for multiple spans in parallel. + * + * Uses p-limit to cap concurrency at {@link SPAN_DETAIL_CONCURRENCY}. + * Failures for individual spans are logged as warnings — callers + * still get partial results for the spans that succeeded. + * + * @param spans - Spans to fetch details for (must have span_id and optionally project_slug) + * @param options - Org, project, traceId, and optional progress callback + * @returns Map of span_id to detail + */ +export async function fetchMultiSpanDetails( + spans: Array<{ span_id: string; project_slug?: string }>, + options: FetchMultiSpanDetailsOptions +): Promise> { + const { org, fallbackProject, traceId, onProgress } = options; + const limit = pLimit(SPAN_DETAIL_CONCURRENCY); + const details = new Map(); + let completed = 0; + const total = spans.length; + + await limit.map(spans, async (span) => { + try { + const detail = await getSpanDetails( + org, + span.project_slug || fallbackProject, + span.span_id, + traceId + ); + details.set(span.span_id, detail); + } catch { + log.warn(`Could not fetch details for span ${span.span_id}`); + } + completed += 1; + onProgress?.(completed, total); + }); + + return details; +} + +// --------------------------------------------------------------------------- +// Normalization +// --------------------------------------------------------------------------- + /** * The trace detail API (`/trace/{id}/`) returns each span's unique identifier * as `event_id` rather than `span_id`. The value is the same 16-hex-char span diff --git a/test/commands/trace/view.func.test.ts b/test/commands/trace/view.func.test.ts index dfedad9d5..b75ddc3dd 100644 --- a/test/commands/trace/view.func.test.ts +++ b/test/commands/trace/view.func.test.ts @@ -18,6 +18,7 @@ import { test, } from "bun:test"; import { + flattenSpanTree, formatTraceView, viewCommand, } from "../../../src/commands/trace/view.js"; @@ -85,6 +86,7 @@ describe("formatTraceView", () => { describe("viewCommand.func", () => { let getDetailedTraceSpy: ReturnType; + let fetchMultiSpanDetailsSpy: ReturnType; let getIssueByShortIdSpy: ReturnType; let getLatestEventSpy: ReturnType; let findProjectsBySlugSpy: ReturnType; @@ -131,6 +133,10 @@ describe("viewCommand.func", () => { beforeEach(async () => { getDetailedTraceSpy = spyOn(apiClient, "getDetailedTrace"); + fetchMultiSpanDetailsSpy = spyOn( + apiClient, + "fetchMultiSpanDetails" + ).mockResolvedValue(new Map()); getIssueByShortIdSpy = spyOn(apiClient, "getIssueByShortId"); getLatestEventSpy = spyOn(apiClient, "getLatestEvent"); findProjectsBySlugSpy = spyOn(apiClient, "findProjectsBySlug"); @@ -143,6 +149,7 @@ describe("viewCommand.func", () => { afterEach(() => { getDetailedTraceSpy.mockRestore(); + fetchMultiSpanDetailsSpy.mockRestore(); getIssueByShortIdSpy.mockRestore(); getLatestEventSpy.mockRestore(); findProjectsBySlugSpy.mockRestore(); @@ -185,7 +192,8 @@ describe("viewCommand.func", () => { const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("aaaa1111bbbb2222cccc3333dddd4444"); - expect(output).toContain("sentry trace view --web"); + // Human mode without --full shows hint about --full/--json + expect(output).toContain("--full"); }); test("throws ValidationError when no spans found", async () => { @@ -259,8 +267,8 @@ describe("viewCommand.func", () => { // Summary should be present expect(output).toContain("aaaa1111bbbb2222cccc3333dddd4444"); // Span tree details should not appear (no span_id rendered) - // The footer should still be present - expect(output).toContain("sentry trace view --web"); + // The footer should still be present (hint about --full in human mode) + expect(output).toContain("--full"); }); test("throws ContextError for org-all target", async () => { @@ -436,4 +444,148 @@ describe("viewCommand.func", () => { func.call(context, { json: false, web: false, spans: 100 }, "CLI-G5") ).rejects.toThrow(ContextError); }); + + test("--json auto-enables detail fetching", async () => { + getDetailedTraceSpy.mockResolvedValue(sampleSpans); + + const { context } = createMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: true, web: false, spans: 100 }, + "test-org/test-project", + "aaaa1111bbbb2222cccc3333dddd4444" + ); + + expect(fetchMultiSpanDetailsSpy).toHaveBeenCalled(); + }); + + test("--full flag enables detail fetching in human mode", async () => { + getDetailedTraceSpy.mockResolvedValue(sampleSpans); + + const { context } = createMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: false, full: true, web: false, spans: 100 }, + "test-org/test-project", + "aaaa1111bbbb2222cccc3333dddd4444" + ); + + expect(fetchMultiSpanDetailsSpy).toHaveBeenCalled(); + }); + + test("human mode without --full does not fetch details", async () => { + getDetailedTraceSpy.mockResolvedValue(sampleSpans); + + const { context } = createMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: false, web: false, spans: 100 }, + "test-org/test-project", + "aaaa1111bbbb2222cccc3333dddd4444" + ); + + expect(fetchMultiSpanDetailsSpy).not.toHaveBeenCalled(); + }); + + test("--json output includes data dict when details are available", async () => { + getDetailedTraceSpy.mockResolvedValue(sampleSpans); + fetchMultiSpanDetailsSpy.mockResolvedValue( + new Map([ + [ + "span-root-001", + { + itemId: "span-root-001", + timestamp: "2024-01-30T13:32:15Z", + attributes: [ + { name: "http.url", type: "str", value: "https://example.com" }, + { name: "span.op", type: "str", value: "http.server" }, + ], + meta: {}, + links: null, + }, + ], + ]) + ); + + const { context, stdoutWrite } = createMockContext(); + const func = await viewCommand.loader(); + await func.call( + context, + { json: true, web: false, spans: 100 }, + "test-org/test-project", + "aaaa1111bbbb2222cccc3333dddd4444" + ); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(output); + // Root span should have a data dict with filtered attributes + const rootSpan = parsed.spans[0]; + expect(rootSpan.data).toBeDefined(); + // http.url should be included (not in REDUNDANT_DETAIL_ATTRS) + expect(rootSpan.data["http.url"]).toBe("https://example.com"); + // span.op should be filtered out (in REDUNDANT_DETAIL_ATTRS) + expect(rootSpan.data["span.op"]).toBeUndefined(); + }); +}); + +// ============================================================================ +// flattenSpanTree +// ============================================================================ + +describe("flattenSpanTree", () => { + test("returns empty array for empty input", () => { + expect(flattenSpanTree([])).toEqual([]); + }); + + test("returns single span for single root", () => { + const span: TraceSpan = { + span_id: "a", + start_timestamp: 1, + }; + const result = flattenSpanTree([span]); + expect(result).toHaveLength(1); + expect(result[0]?.span_id).toBe("a"); + }); + + test("returns all spans in depth-first order", () => { + const tree: TraceSpan[] = [ + { + span_id: "root", + start_timestamp: 1, + children: [ + { + span_id: "child-1", + start_timestamp: 2, + children: [{ span_id: "grandchild-1", start_timestamp: 3 }], + }, + { span_id: "child-2", start_timestamp: 4 }, + ], + }, + ]; + const result = flattenSpanTree(tree); + const ids = result.map((s) => s.span_id); + expect(ids).toEqual(["root", "child-1", "grandchild-1", "child-2"]); + }); + + test("handles multiple roots", () => { + const tree: TraceSpan[] = [ + { span_id: "root-1", start_timestamp: 1 }, + { span_id: "root-2", start_timestamp: 2 }, + ]; + const result = flattenSpanTree(tree); + expect(result).toHaveLength(2); + expect(result[0]?.span_id).toBe("root-1"); + expect(result[1]?.span_id).toBe("root-2"); + }); + + test("handles spans with empty children array", () => { + const tree: TraceSpan[] = [ + { span_id: "root", start_timestamp: 1, children: [] }, + ]; + const result = flattenSpanTree(tree); + expect(result).toHaveLength(1); + }); }); diff --git a/test/commands/trace/view.property.test.ts b/test/commands/trace/view.property.test.ts index 40991702f..7bfac4855 100644 --- a/test/commands/trace/view.property.test.ts +++ b/test/commands/trace/view.property.test.ts @@ -10,12 +10,16 @@ import { describe, expect, test } from "bun:test"; import { assert as fcAssert, + integer, property, stringMatching, tuple, + uniqueArray, } from "fast-check"; +import { flattenSpanTree } from "../../../src/commands/trace/view.js"; import { ContextError, ValidationError } from "../../../src/lib/errors.js"; import { parseTraceTarget } from "../../../src/lib/trace-target.js"; +import type { TraceSpan } from "../../../src/types/sentry.js"; import { DEFAULT_NUM_RUNS } from "../../model-based/helpers.js"; /** Valid trace IDs (32-char hex) */ @@ -144,3 +148,116 @@ describe("parseTraceTarget properties", () => { ); }); }); + +// ============================================================================ +// flattenSpanTree properties +// ============================================================================ + +/** + * Build a span tree from a flat list of (spanId, parentIndex) pairs. + * parentIndex = -1 means root. Otherwise index into the flat list. + */ +function buildTree( + items: Array<{ id: string; parentIdx: number }> +): TraceSpan[] { + const nodes: TraceSpan[] = items.map((item) => ({ + span_id: item.id, + start_timestamp: 1, + })); + + const roots: TraceSpan[] = []; + for (let i = 0; i < items.length; i++) { + const item = items[i]; + const node = nodes[i]; + if (!item) { + continue; + } + if (!node) { + continue; + } + if (item.parentIdx < 0 || item.parentIdx >= i) { + roots.push(node); + } else { + const parent = nodes[item.parentIdx]; + if (parent) { + if (!parent.children) { + parent.children = []; + } + parent.children.push(node); + } + } + } + return roots; +} + +/** Generate a span tree of 0-20 spans via flat list + tree construction */ +const spanTreeArb = uniqueArray(stringMatching(/^[a-f0-9]{16}$/), { + minLength: 0, + maxLength: 20, +}) + .chain((ids) => + tuple( + ...ids.map((id, i) => + integer({ min: -1, max: Math.max(0, i - 1) }).map((parentIdx) => ({ + id, + parentIdx, + })) + ) + ) + ) + .map((items) => buildTree(items)); + +/** Count all spans in a tree recursively */ +function countSpans(spans: TraceSpan[]): number { + let count = 0; + for (const span of spans) { + count += 1; + if (span.children) { + count += countSpans(span.children); + } + } + return count; +} + +/** Collect all span IDs from a tree recursively */ +function collectSpanIds(spans: TraceSpan[]): Set { + const ids = new Set(); + for (const span of spans) { + ids.add(span.span_id); + if (span.children) { + for (const id of collectSpanIds(span.children)) { + ids.add(id); + } + } + } + return ids; +} + +describe("flattenSpanTree properties", () => { + test("result length equals total span count", async () => { + await fcAssert( + property(spanTreeArb, (tree) => { + const result = flattenSpanTree(tree); + expect(result).toHaveLength(countSpans(tree)); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("all returned spans exist in original tree", async () => { + await fcAssert( + property(spanTreeArb, (tree) => { + const result = flattenSpanTree(tree); + const originalIds = collectSpanIds(tree); + for (const span of result) { + expect(originalIds.has(span.span_id)).toBe(true); + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("empty input returns empty output", () => { + expect(flattenSpanTree([])).toEqual([]); + }); +}); From b835ec07fb1a89ce47e63f6bc87f584075d07e9c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 13 Apr 2026 22:31:14 +0000 Subject: [PATCH 2/2] fix: use Array.from() in flattenSpanTree and cache trace-items as immutable - Replace spread with Array.from() per codebase convention - Add /trace-items/ to the immutable cache tier (24h TTL) since span attributes never change once ingested, same rationale as events and trace detail endpoints --- src/commands/trace/view.ts | 2 +- src/lib/response-cache.ts | 8 ++++++-- test/lib/response-cache.property.test.ts | 6 ++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/commands/trace/view.ts b/src/commands/trace/view.ts index 2326b6691..4953cf1f7 100644 --- a/src/commands/trace/view.ts +++ b/src/commands/trace/view.ts @@ -208,7 +208,7 @@ const PROGRESS_THRESHOLD = 20; export function flattenSpanTree(spans: TraceSpan[]): TraceSpan[] { const result: TraceSpan[] = []; // Reverse so the first child is popped first (depth-first order) - const stack = [...spans].reverse(); + const stack = Array.from(spans).reverse(); let span = stack.pop(); while (span) { result.push(span); diff --git a/src/lib/response-cache.ts b/src/lib/response-cache.ts index 6f3b67440..e4fbe77c3 100644 --- a/src/lib/response-cache.ts +++ b/src/lib/response-cache.ts @@ -62,8 +62,12 @@ const FALLBACK_TTL_MS: Record = { const URL_TIER_REGEXPS: Readonly> = { // Polling endpoints where state changes rapidly "no-cache": [/\/(?:autofix|root-cause)\//], - // Specific resources by ID (events, traces) — never change once created - immutable: [/\/events\/[^/?]+\/?(?:\?|$)/, /\/trace\/[0-9a-f]{32}\//], + // Specific resources by ID (events, traces, span details) — never change once created + immutable: [ + /\/events\/[^/?]+\/?(?:\?|$)/, + /\/trace\/[0-9a-f]{32}\//, + /\/trace-items\/[0-9a-f]+\//, + ], // Issue endpoints (lists AND detail views), dataset queries, trace-logs volatile: [ /\/issues\//, diff --git a/test/lib/response-cache.property.test.ts b/test/lib/response-cache.property.test.ts index e98a4dfd3..6a00cd359 100644 --- a/test/lib/response-cache.property.test.ts +++ b/test/lib/response-cache.property.test.ts @@ -195,6 +195,12 @@ describe("property: classifyUrl", () => { expect(classifyUrl(url)).toBe("immutable"); }); + test("trace-items (span detail) URLs are immutable", () => { + const url = + "https://us.sentry.io/api/0/projects/org/proj/trace-items/a1b2c3d4e5f67890/?trace_id=abc&item_type=spans"; + expect(classifyUrl(url)).toBe("immutable"); + }); + test("issue URLs are volatile (lists and detail views)", () => { const urls = [ "https://us.sentry.io/api/0/projects/org/proj/issues/",