Skip to content

Commit f3b6fe3

Browse files
committed
refactor: address human reviewer feedback on span commands
- Move validateSpanId to shared hex-id.ts (reuses pattern from validateHexId) with automatic dash-stripping for span IDs - Remove MIN_LIMIT constant — inline the value 1 in parseLimit call - Add JSDoc to MAX_LIMIT explaining it's a CLI-side convention (1000) matching issue list, trace list, and log list caps - Extract DEFAULT_SORT constant for sort flag default - Remove '(default)' from sort brief — Stricli shows defaults automatically - Update USAGE_HINT and fullDescription to use slash-separated format: sentry span list <org>/<project>/<trace-id> - Update resolveProjectBySlug suggestion to slash format
1 parent f68d1e6 commit f3b6fe3

File tree

4 files changed

+64
-43
lines changed

4 files changed

+64
-43
lines changed

src/commands/span/list.ts

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,31 @@ type SortValue = "time" | "duration";
4848
/** Accepted values for the --sort flag */
4949
const VALID_SORT_VALUES: SortValue[] = ["time", "duration"];
5050

51-
/** Maximum allowed value for --limit flag */
51+
/**
52+
* CLI-side upper bound for --limit.
53+
*
54+
* The Sentry Events API (spans dataset) accepts `per_page` up to 100 per
55+
* request, but the CLI allows requesting up to 1000 as a convenience —
56+
* the API client paginates internally when needed. This matches the cap
57+
* used by `issue list`, `trace list`, and `log list`.
58+
*/
5259
const MAX_LIMIT = 1000;
5360

54-
/** Minimum allowed value for --limit flag */
55-
const MIN_LIMIT = 1;
56-
5761
/** Default number of spans to show */
5862
const DEFAULT_LIMIT = 25;
5963

64+
/** Default sort order for span results */
65+
const DEFAULT_SORT: SortValue = "time";
66+
6067
/** Usage hint for ContextError messages */
61-
const USAGE_HINT = "sentry span list [<org>/<project>] <trace-id>";
68+
const USAGE_HINT = "sentry span list [<org>/<project>/]<trace-id>";
6269

6370
/**
6471
* Parse positional arguments for span list.
65-
* Handles: `<trace-id>` or `<target> <trace-id>`
72+
* Handles: `<trace-id>` or `<org>/<project>/<trace-id>`
73+
*
74+
* Uses the standard `parseSlashSeparatedArg` pattern: the last `/`-separated
75+
* segment is the trace ID, and everything before it is the org/project target.
6676
*
6777
* @param args - Positional arguments from CLI
6878
* @returns Parsed trace ID and optional target arg
@@ -104,7 +114,7 @@ export function parsePositionalArgs(args: string[]): {
104114
* Parse --limit flag, delegating range validation to shared utility.
105115
*/
106116
function parseLimit(value: string): number {
107-
return validateLimit(value, MIN_LIMIT, MAX_LIMIT);
117+
return validateLimit(value, 1, MAX_LIMIT);
108118
}
109119

110120
/**
@@ -171,16 +181,16 @@ export const listCommand = buildCommand({
171181
fullDescription:
172182
"List spans in a distributed trace with optional filtering and sorting.\n\n" +
173183
"Target specification:\n" +
174-
" sentry span list <trace-id> # auto-detect from DSN or config\n" +
175-
" sentry span list <org>/<proj> <trace-id> # explicit org and project\n" +
176-
" sentry span list <project> <trace-id> # find project across all orgs\n\n" +
184+
" sentry span list <trace-id> # auto-detect from DSN or config\n" +
185+
" sentry span list <org>/<project>/<trace-id> # explicit org and project\n" +
186+
" sentry span list <project> <trace-id> # find project across all orgs\n\n" +
177187
"The trace ID is the 32-character hexadecimal identifier.\n\n" +
178188
"Examples:\n" +
179-
" sentry span list <trace-id> # List spans in trace\n" +
180-
" sentry span list <trace-id> --limit 50 # Show more spans\n" +
181-
' sentry span list <trace-id> -q "op:db" # Filter by operation\n' +
182-
" sentry span list <trace-id> --sort duration # Sort by slowest first\n" +
183-
' sentry span list <trace-id> -q "duration:>100ms" # Spans slower than 100ms',
189+
" sentry span list <trace-id> # List spans in trace\n" +
190+
" sentry span list <trace-id> --limit 50 # Show more spans\n" +
191+
' sentry span list <trace-id> -q "op:db" # Filter by operation\n' +
192+
" sentry span list <trace-id> --sort duration # Sort by slowest first\n" +
193+
' sentry span list <trace-id> -q "duration:>100ms" # Spans slower than 100ms',
184194
},
185195
output: {
186196
human: formatSpanListHuman,
@@ -200,7 +210,7 @@ export const listCommand = buildCommand({
200210
limit: {
201211
kind: "parsed",
202212
parse: parseLimit,
203-
brief: `Number of spans (${MIN_LIMIT}-${MAX_LIMIT})`,
213+
brief: `Number of spans (1-${MAX_LIMIT})`,
204214
default: String(DEFAULT_LIMIT),
205215
},
206216
query: {
@@ -213,8 +223,8 @@ export const listCommand = buildCommand({
213223
sort: {
214224
kind: "parsed",
215225
parse: parseSort,
216-
brief: "Sort by: time (default), duration",
217-
default: "time" as const,
226+
brief: `Sort order: ${VALID_SORT_VALUES.join(", ")}`,
227+
default: DEFAULT_SORT,
218228
},
219229
fresh: FRESH_FLAG,
220230
},
@@ -249,7 +259,7 @@ export const listCommand = buildCommand({
249259
target = await resolveProjectBySlug(
250260
parsed.projectSlug,
251261
USAGE_HINT,
252-
`sentry span list <org>/${parsed.projectSlug} ${traceId}`
262+
`sentry span list <org>/${parsed.projectSlug}/${traceId}`
253263
);
254264
break;
255265

src/commands/span/view.ts

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
import { filterFields } from "../../lib/formatters/json.js";
2323
import { CommandOutput } from "../../lib/formatters/output.js";
2424
import { computeSpanDurationMs } from "../../lib/formatters/trace.js";
25+
import { validateSpanId } from "../../lib/hex-id.js";
2526
import {
2627
applyFreshFlag,
2728
FRESH_ALIASES,
@@ -43,31 +44,10 @@ type ViewFlags = {
4344
readonly fields?: string[];
4445
};
4546

46-
/** Regex for a 16-character hex span ID */
47-
const SPAN_ID_RE = /^[0-9a-f]{16}$/i;
48-
4947
/** Usage hint for ContextError messages */
5048
const USAGE_HINT =
5149
"sentry span view [<org>/<project>/]<trace-id> <span-id> [<span-id>...]";
5250

53-
/**
54-
* Validate that a string is a 16-character hexadecimal span ID.
55-
*
56-
* @param value - The string to validate
57-
* @returns The trimmed, lowercased span ID
58-
* @throws {ValidationError} If the format is invalid
59-
*/
60-
export function validateSpanId(value: string): string {
61-
const trimmed = value.trim().toLowerCase();
62-
if (!SPAN_ID_RE.test(trimmed)) {
63-
throw new ValidationError(
64-
`Invalid span ID "${trimmed}". Expected a 16-character hexadecimal string.\n\n` +
65-
"Example: a1b2c3d4e5f67890"
66-
);
67-
}
68-
return trimmed;
69-
}
70-
7151
/**
7252
* Parse positional arguments for span view.
7353
*

src/lib/hex-id.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
/**
22
* Shared Hex ID Validation
33
*
4-
* Provides regex and validation for 32-character hexadecimal identifiers
5-
* used across the CLI (log IDs, trace IDs, etc.).
4+
* Provides regex and validation for hexadecimal identifiers used across
5+
* the CLI (trace IDs, log IDs, span IDs, etc.).
66
*/
77

88
import { ValidationError } from "./errors.js";
99

1010
/** Regex for a valid 32-character hexadecimal ID */
1111
export const HEX_ID_RE = /^[0-9a-f]{32}$/i;
1212

13+
/** Regex for a valid 16-character hexadecimal span ID */
14+
export const SPAN_ID_RE = /^[0-9a-f]{16}$/i;
15+
1316
/**
1417
* Regex for UUID format with dashes: 8-4-4-4-12 hex groups.
1518
* Users often copy trace/log IDs from tools that display them in UUID format.
@@ -61,3 +64,31 @@ export function validateHexId(value: string, label: string): string {
6164

6265
return trimmed;
6366
}
67+
68+
/**
69+
* Validate that a string is a 16-character hexadecimal span ID.
70+
* Trims whitespace and normalizes to lowercase before validation.
71+
*
72+
* Dashes are stripped automatically so users can paste IDs in dash-separated
73+
* formats (e.g., from debugging tools that format span IDs with dashes).
74+
*
75+
* @param value - The string to validate
76+
* @returns The trimmed, lowercased, validated span ID
77+
* @throws {ValidationError} If the format is invalid
78+
*/
79+
export function validateSpanId(value: string): string {
80+
const trimmed = value.trim().toLowerCase().replace(/-/g, "");
81+
82+
if (!SPAN_ID_RE.test(trimmed)) {
83+
const display =
84+
trimmed.length > MAX_DISPLAY_LENGTH
85+
? `${trimmed.slice(0, MAX_DISPLAY_LENGTH - 3)}...`
86+
: trimmed;
87+
throw new ValidationError(
88+
`Invalid span ID "${display}". Expected a 16-character hexadecimal string.\n\n` +
89+
"Example: a1b2c3d4e5f67890"
90+
);
91+
}
92+
93+
return trimmed;
94+
}

test/commands/span/view.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ import {
1616
} from "bun:test";
1717
import {
1818
parsePositionalArgs,
19-
validateSpanId,
2019
viewCommand,
2120
} from "../../../src/commands/span/view.js";
2221
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
2322
import * as apiClient from "../../../src/lib/api-client.js";
2423
import { ContextError, ValidationError } from "../../../src/lib/errors.js";
24+
import { validateSpanId } from "../../../src/lib/hex-id.js";
2525
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
2626
import * as resolveTarget from "../../../src/lib/resolve-target.js";
2727

0 commit comments

Comments
 (0)