Skip to content

Commit 85ee2ad

Browse files
committed
refactor(log): use server-side sorting instead of client-side reversal
Move sort direction to the API layer (LogSortDirection in api/logs.ts) and pass it through to both listLogs() and listTraceLogs() endpoints instead of reversing arrays client-side. Rename parseSort to parseLogSort to avoid collision with per-command parseSort functions in trace/span/issue list commands.
1 parent 3302674 commit 85ee2ad

7 files changed

Lines changed: 91 additions & 64 deletions

File tree

src/commands/log/list.ts

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99
// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
1010
import * as Sentry from "@sentry/node-core/light";
1111
import type { SentryContext } from "../../context.js";
12-
import { listLogs, listTraceLogs } from "../../lib/api-client.js";
1312
import {
14-
parseSort,
15-
type SortDirection,
16-
validateLimit,
17-
} from "../../lib/arg-parsing.js";
13+
type LogSortDirection,
14+
listLogs,
15+
listTraceLogs,
16+
} from "../../lib/api-client.js";
17+
import { parseLogSort, validateLimit } from "../../lib/arg-parsing.js";
1818
import { AuthError, stringifyUnknown } from "../../lib/errors.js";
1919
import {
2020
buildLogRowCells,
@@ -50,7 +50,7 @@ type ListFlags = {
5050
readonly query?: string;
5151
readonly follow?: number;
5252
readonly period?: string;
53-
readonly sort: SortDirection;
53+
readonly sort: LogSortDirection;
5454
readonly json: boolean;
5555
readonly fresh: boolean;
5656
readonly fields?: string[];
@@ -180,21 +180,19 @@ async function executeSingleFetch(
180180
query: flags.query,
181181
limit: flags.limit,
182182
statsPeriod: period,
183+
sort: flags.sort,
183184
});
184185

185186
if (logs.length === 0) {
186187
return { result: { logs: [], hasMore: false }, hint: "No logs found." };
187188
}
188189

189-
// API returns newest first. Reverse only when user wants oldest-first.
190-
const ordered = flags.sort === "oldest" ? [...logs].reverse() : logs;
191-
192190
const hasMore = logs.length >= flags.limit;
193191
const countText = `Showing ${logs.length} log${logs.length === 1 ? "" : "s"}.`;
194192
const tip = hasMore ? " Use --limit to show more, or -f to follow." : "";
195193

196194
return {
197-
result: { logs: ordered, hasMore },
195+
result: { logs, hasMore },
198196
hint: `${countText}${tip}`,
199197
};
200198
}
@@ -440,6 +438,7 @@ async function executeTraceSingleFetch(
440438
query: flags.query,
441439
limit: flags.limit,
442440
statsPeriod: period,
441+
sort: flags.sort,
443442
});
444443

445444
if (logs.length === 0) {
@@ -451,14 +450,12 @@ async function executeTraceSingleFetch(
451450
};
452451
}
453452

454-
const ordered = flags.sort === "oldest" ? [...logs].reverse() : logs;
455-
456453
const hasMore = logs.length >= flags.limit;
457454
const countText = `Showing ${logs.length} log${logs.length === 1 ? "" : "s"} for trace ${traceId}.`;
458455
const tip = hasMore ? " Use --limit to show more." : "";
459456

460457
return {
461-
result: { logs: ordered, traceId, hasMore },
458+
result: { logs, traceId, hasMore },
462459
hint: `${countText}${tip}`,
463460
};
464461
}
@@ -645,7 +642,7 @@ export const listCommand = buildListCommand(
645642
},
646643
sort: {
647644
kind: "parsed",
648-
parse: parseSort,
645+
parse: parseLogSort,
649646
brief: 'Sort order: "newest" (default) or "oldest"',
650647
default: "newest",
651648
},

src/commands/trace/logs.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,8 @@
55
*/
66

77
import type { SentryContext } from "../../context.js";
8-
import { listTraceLogs } from "../../lib/api-client.js";
9-
import {
10-
parseSort,
11-
type SortDirection,
12-
validateLimit,
13-
} from "../../lib/arg-parsing.js";
8+
import { type LogSortDirection, listTraceLogs } from "../../lib/api-client.js";
9+
import { parseLogSort, validateLimit } from "../../lib/arg-parsing.js";
1410
import { openInBrowser } from "../../lib/browser.js";
1511
import { buildCommand } from "../../lib/command.js";
1612
import { filterFields } from "../../lib/formatters/json.js";
@@ -35,7 +31,7 @@ type LogsFlags = {
3531
readonly period: string;
3632
readonly limit: number;
3733
readonly query?: string;
38-
readonly sort: SortDirection;
34+
readonly sort: LogSortDirection;
3935
readonly fresh: boolean;
4036
readonly fields?: string[];
4137
};
@@ -154,7 +150,7 @@ export const logsCommand = buildCommand({
154150
},
155151
sort: {
156152
kind: "parsed",
157-
parse: parseSort,
153+
parse: parseLogSort,
158154
brief: 'Sort order: "newest" (default) or "oldest"',
159155
default: "newest",
160156
},
@@ -192,19 +188,18 @@ export const logsCommand = buildCommand({
192188
statsPeriod: flags.period,
193189
limit: flags.limit,
194190
query: flags.query,
191+
sort: flags.sort,
195192
})
196193
);
197194

198-
// API returns newest-first. Reverse only when user wants oldest-first.
199-
const ordered = flags.sort === "oldest" ? [...logs].reverse() : logs;
200-
const hasMore = ordered.length >= flags.limit;
195+
const hasMore = logs.length >= flags.limit;
201196

202197
const emptyMessage =
203198
`No logs found for trace ${traceId} in the last ${flags.period}.\n\n` +
204199
`Try a longer period: sentry trace logs --period 30d ${traceId}`;
205200

206201
return yield new CommandOutput({
207-
logs: ordered,
202+
logs,
208203
traceId,
209204
hasMore,
210205
emptyMessage,

src/lib/api-client.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export {
6060
} from "./api/issues.js";
6161
export {
6262
getLogs,
63+
type LogSortDirection,
6364
listLogs,
6465
listTraceLogs,
6566
} from "./api/logs.js";

src/lib/api/logs.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ import {
2626
unwrapResult,
2727
} from "./infrastructure.js";
2828

29+
/** Sort direction for log queries: newest-first or oldest-first. */
30+
export type LogSortDirection = "newest" | "oldest";
31+
32+
/** Map CLI sort direction to Sentry API sort parameter. */
33+
function toApiSort(sort: LogSortDirection | undefined): string {
34+
return sort === "oldest" ? "timestamp" : "-timestamp";
35+
}
36+
2937
/** Fields to request from the logs API */
3038
const LOG_FIELDS = [
3139
"sentry.item_id",
@@ -43,6 +51,8 @@ type ListLogsOptions = {
4351
limit?: number;
4452
/** Time period for logs (e.g., "90d", "10m") */
4553
statsPeriod?: string;
54+
/** Sort direction: "newest" (default) or "oldest" */
55+
sort?: LogSortDirection;
4656
/** Only return logs after this timestamp_precise value (for streaming) */
4757
afterTimestamp?: number;
4858
};
@@ -84,7 +94,7 @@ export async function listLogs(
8494
query: fullQuery || undefined,
8595
per_page: options.limit || API_MAX_PER_PAGE,
8696
statsPeriod: options.statsPeriod ?? "14d",
87-
sort: "-timestamp",
97+
sort: toApiSort(options.sort),
8898
},
8999
});
90100

@@ -193,6 +203,8 @@ type ListTraceLogsOptions = {
193203
* logs exist for the trace. Defaults to "14d".
194204
*/
195205
statsPeriod?: string;
206+
/** Sort direction: "newest" (default) or "oldest" */
207+
sort?: LogSortDirection;
196208
};
197209

198210
/**
@@ -208,8 +220,8 @@ type ListTraceLogsOptions = {
208220
*
209221
* @param orgSlug - Organization slug
210222
* @param traceId - The 32-character hex trace ID
211-
* @param options - Optional query/limit/statsPeriod overrides
212-
* @returns Array of trace log entries, ordered newest-first
223+
* @param options - Optional query/limit/statsPeriod/sort overrides
224+
* @returns Array of trace log entries
213225
*/
214226
export async function listTraceLogs(
215227
orgSlug: string,
@@ -227,7 +239,7 @@ export async function listTraceLogs(
227239
statsPeriod: options.statsPeriod ?? "14d",
228240
per_page: options.limit ?? API_MAX_PER_PAGE,
229241
query: options.query,
230-
sort: "-timestamp",
242+
sort: toApiSort(options.sort),
231243
},
232244
schema: TraceLogsResponseSchema,
233245
}

src/lib/arg-parsing.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* project list) and single-item commands (issue view, explain, plan).
77
*/
88

9+
import type { LogSortDirection } from "./api/logs.js";
910
import { ContextError, ValidationError } from "./errors.js";
1011
import { validateResourceId } from "./input-validation.js";
1112
import { logger } from "./logger.js";
@@ -262,23 +263,25 @@ export function validateLimit(value: string, min = 1, max = 1000): number {
262263
}
263264

264265
// ---------------------------------------------------------------------------
265-
// Sort direction parsing (shared by log list, trace logs)
266+
// Log sort direction parsing (shared by log list, trace logs)
266267
// ---------------------------------------------------------------------------
267268

268-
/** Sort direction for log output */
269-
export type SortDirection = "newest" | "oldest";
270-
271-
const VALID_SORT_DIRECTIONS: readonly SortDirection[] = ["newest", "oldest"];
269+
const VALID_LOG_SORT_DIRECTIONS: readonly LogSortDirection[] = [
270+
"newest",
271+
"oldest",
272+
];
272273

273274
/**
274-
* Parse --sort flag value.
275+
* Parse --sort flag value for log commands.
275276
* @throws Error if value is not "newest" or "oldest"
276277
*/
277-
export function parseSort(value: string): SortDirection {
278-
if (!VALID_SORT_DIRECTIONS.includes(value as SortDirection)) {
279-
throw new Error(`--sort must be "newest" or "oldest", got "${value}"`);
278+
export function parseLogSort(value: string): LogSortDirection {
279+
if (!VALID_LOG_SORT_DIRECTIONS.includes(value as LogSortDirection)) {
280+
throw new Error(
281+
`Invalid sort value. Must be one of: ${VALID_LOG_SORT_DIRECTIONS.join(", ")}`
282+
);
280283
}
281-
return value as SortDirection;
284+
return value as LogSortDirection;
282285
}
283286

284287
/** Default span depth when no value is provided */

test/commands/log/list.test.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,24 +289,24 @@ describe("listCommand.func — standard mode", () => {
289289
expect(parsed.data[2]["sentry.item_id"]).toBe("item001");
290290
});
291291

292-
test("outputs JSON in oldest-first order with sort=oldest", async () => {
293-
const newestFirst = [...sampleLogs].reverse();
294-
listLogsSpy.mockResolvedValue(newestFirst);
292+
test("passes sort=oldest to API when requested", async () => {
293+
listLogsSpy.mockResolvedValue(sampleLogs);
295294
resolveOrgProjectSpy.mockResolvedValue({ org: ORG, project: PROJECT });
296295

297-
const { context, stdoutWrite } = createMockContext();
296+
const { context } = createMockContext();
298297
const func = await listCommand.loader();
299298
await func.call(
300299
context,
301300
{ ...BATCH_FLAGS, sort: "oldest" },
302301
`${ORG}/${PROJECT}`
303302
);
304303

305-
const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
306-
const parsed = JSON.parse(output);
307-
// sort=oldest reverses to chronological order
308-
expect(parsed.data[0]["sentry.item_id"]).toBe("item001");
309-
expect(parsed.data[2]["sentry.item_id"]).toBe("item003");
304+
// sort=oldest is passed to API for server-side sorting
305+
expect(listLogsSpy).toHaveBeenCalledWith(
306+
ORG,
307+
PROJECT,
308+
expect.objectContaining({ sort: "oldest" })
309+
);
310310
});
311311

312312
test("shows 'No logs found' for empty result (human mode)", async () => {
@@ -388,6 +388,7 @@ describe("listCommand.func — standard mode", () => {
388388
query: "level:error",
389389
limit: 50,
390390
statsPeriod: "30d",
391+
sort: "newest",
391392
});
392393
});
393394

@@ -539,14 +540,15 @@ describe("listCommand.func — trace mode", () => {
539540
const func = await listCommand.loader();
540541
await func.call(
541542
context,
542-
{ json: false, limit: 50, query: "level:error" },
543+
{ json: false, limit: 50, query: "level:error", sort: "newest" },
543544
TRACE_ID
544545
);
545546

546547
expect(listTraceLogsSpy).toHaveBeenCalledWith(ORG, TRACE_ID, {
547548
query: "level:error",
548549
limit: 50,
549550
statsPeriod: "14d",
551+
sort: "newest",
550552
});
551553
});
552554

@@ -700,6 +702,7 @@ describe("listCommand.func — period flag", () => {
700702
query: undefined,
701703
limit: 100,
702704
statsPeriod: "14d",
705+
sort: "newest",
703706
});
704707
});
705708

@@ -716,6 +719,7 @@ describe("listCommand.func — period flag", () => {
716719
query: undefined,
717720
limit: 100,
718721
statsPeriod: "30d",
722+
sort: "newest",
719723
});
720724
});
721725
});

0 commit comments

Comments
 (0)