mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-02 22:17:14 +00:00
fix(ui): remove intra-trace coalescing — preserve every stage for inspection
PR-Agent flagged that the trace-child coalesce was hiding legitimate repeated stage emissions: a request that retries an upstream call or emits the same stage twice for any reason would show only the first occurrence behind a `×N` badge, with subsequent attempts no longer selectable or inspectable. The original noise problem motivating intra-trace coalesce was the 149-stage `web-server:http` self-polling trace, but that's already hidden by default via the `hideDashboardInternals` filter. Users who opt in to see internals are deliberately debugging the dashboard and SHOULD see all 149 stages — collapsing them defeats the opt-in. Drop `coalesceChildren` entirely. Trace children render uncoalesced so retries, duplicated stages, and multi-attempt traces stay individually inspectable. Standalone-leaf coalesce stays — that's the actual user-facing dedup the PR was about. Tests: drop the 7 `coalesceChildren` cases; keep 16 covering `deriveStageHint` and `deriveTraceGroups`. Refs #1138, #1151
This commit is contained in:
@@ -18,11 +18,16 @@ export interface LeafItem {
|
|||||||
export type DerivedItem = LeafItem | TraceGroup;
|
export type DerivedItem = LeafItem | TraceGroup;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Tuple key for coalescing. Includes `message` so two adjacent logs that
|
* Tuple key for coalescing standalone leaves. Includes `message` and
|
||||||
* share event/module/level but report different content stay distinct
|
* `source` so two adjacent logs that share event/module/level but report
|
||||||
|
* different content (or come from a different service) stay distinct
|
||||||
* (e.g. `User logged in: alice` and `User logged in: bob`). Excludes
|
* (e.g. `User logged in: alice` and `User logged in: bob`). Excludes
|
||||||
* `latencyMs` and `metadata` because those drift per request even on
|
* `latencyMs` and `metadata` because those drift per request even on
|
||||||
* truly redundant polls — including them would prevent any coalescing.
|
* truly redundant polls — including them would prevent any coalescing.
|
||||||
|
*
|
||||||
|
* NB: this only applies to *leaves* (entries without `requestId`). Trace
|
||||||
|
* children render uncoalesced so retries and duplicated-stage emissions
|
||||||
|
* stay individually inspectable.
|
||||||
*/
|
*/
|
||||||
function coalesceKey(entry: LogsEntry): string {
|
function coalesceKey(entry: LogsEntry): string {
|
||||||
return [
|
return [
|
||||||
@@ -35,52 +40,6 @@ function coalesceKey(entry: LogsEntry): string {
|
|||||||
].join(' ');
|
].join(' ');
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Coalesce identical consecutive children inside a single trace.
|
|
||||||
*
|
|
||||||
* Key tuple includes `message` so two children that share
|
|
||||||
* event/stage/level/module but report different content stay visible as
|
|
||||||
* separate rows. Includes `source` so adjacent rows from different
|
|
||||||
* services with otherwise-identical fields stay distinct (a request can
|
|
||||||
* fan out across multiple sources participating in the same trace).
|
|
||||||
* Excludes `latencyMs` — it drifts per request and would defeat the
|
|
||||||
* dedup on truly redundant polls.
|
|
||||||
*
|
|
||||||
* Exported so it can be unit-tested independently from the React row.
|
|
||||||
*/
|
|
||||||
export function coalesceChildren(children: LogsEntry[]): Array<{ head: LogsEntry; count: number }> {
|
|
||||||
if (children.length === 0) return [];
|
|
||||||
const out: Array<{ head: LogsEntry; count: number }> = [];
|
|
||||||
let head: LogsEntry | null = null;
|
|
||||||
let count = 0;
|
|
||||||
let key = '';
|
|
||||||
const flush = () => {
|
|
||||||
if (head) out.push({ head, count });
|
|
||||||
head = null;
|
|
||||||
count = 0;
|
|
||||||
};
|
|
||||||
for (const child of children) {
|
|
||||||
const k = [
|
|
||||||
child.event ?? '',
|
|
||||||
child.message ?? '',
|
|
||||||
child.stage ?? '',
|
|
||||||
child.level,
|
|
||||||
child.module ?? '',
|
|
||||||
child.source ?? '',
|
|
||||||
].join(' ');
|
|
||||||
if (head && k === key) {
|
|
||||||
count += 1;
|
|
||||||
} else {
|
|
||||||
flush();
|
|
||||||
head = child;
|
|
||||||
count = 1;
|
|
||||||
key = k;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
flush();
|
|
||||||
return out;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* For an entry without an explicit `stage` field, derive a short chip label
|
* For an entry without an explicit `stage` field, derive a short chip label
|
||||||
* from the event name so the trace timeline still renders meaningful badges
|
* from the event name so the trace timeline still renders meaningful badges
|
||||||
@@ -108,7 +67,8 @@ export function deriveStageHint(entry: LogsEntry): string | undefined {
|
|||||||
* Trace groups gather all children sharing a requestId regardless of
|
* Trace groups gather all children sharing a requestId regardless of
|
||||||
* interleaving; they're sorted by `ts asc` before display, with the
|
* interleaving; they're sorted by `ts asc` before display, with the
|
||||||
* group's positional `ts` set to the oldest child so it slots correctly
|
* group's positional `ts` set to the oldest child so it slots correctly
|
||||||
* in the reverse-chronological display sort below.
|
* in the reverse-chronological display sort below. Children are NOT
|
||||||
|
* coalesced — every stage is preserved for individual inspection.
|
||||||
*/
|
*/
|
||||||
export function deriveTraceGroups(entries: LogsEntry[]): DerivedItem[] {
|
export function deriveTraceGroups(entries: LogsEntry[]): DerivedItem[] {
|
||||||
const items: DerivedItem[] = [];
|
const items: DerivedItem[] = [];
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import { memo } from 'react';
|
|||||||
import { ChevronDown, ChevronRight, GitBranch } from 'lucide-react';
|
import { ChevronDown, ChevronRight, GitBranch } from 'lucide-react';
|
||||||
import type { LogsEntry, LogsLevel } from '@/lib/api-client';
|
import type { LogsEntry, LogsLevel } from '@/lib/api-client';
|
||||||
import { cn } from '@/lib/utils';
|
import { cn } from '@/lib/utils';
|
||||||
import { coalesceChildren, deriveStageHint } from './derive-trace-groups';
|
import { deriveStageHint } from './derive-trace-groups';
|
||||||
import { LogLevelBadge } from './log-level-badge';
|
import { LogLevelBadge } from './log-level-badge';
|
||||||
import { LogsRow } from './logs-row';
|
import { LogsRow } from './logs-row';
|
||||||
import { FOCUS_RING, MONO_NUMERIC, ROW_DENSITY, ROW_INTERACTIVE, type RowDensity } from './tokens';
|
import { FOCUS_RING, MONO_NUMERIC, ROW_DENSITY, ROW_INTERACTIVE, type RowDensity } from './tokens';
|
||||||
@@ -110,18 +110,24 @@ function LogsTraceRowImpl({
|
|||||||
{group.requestId.slice(-8)}
|
{group.requestId.slice(-8)}
|
||||||
</span>
|
</span>
|
||||||
</button>
|
</button>
|
||||||
|
{/* Children render unconditionally — every stage is preserved so
|
||||||
|
retries, duplicated stage logs, and multi-attempt traces stay
|
||||||
|
individually inspectable. The user-facing noise problem this
|
||||||
|
PR addresses lives at the standalone-leaf level (dashboard
|
||||||
|
self-polling spam) where leaf-coalesce in deriveTraceGroups
|
||||||
|
handles it; opting in to see internals is a deliberate debug
|
||||||
|
choice, so collapsing trace stages there would hide intent. */}
|
||||||
{isExpanded
|
{isExpanded
|
||||||
? coalesceChildren(group.children).map((run) => (
|
? group.children.map((child) => (
|
||||||
<LogsRow
|
<LogsRow
|
||||||
key={run.head.id}
|
key={child.id}
|
||||||
entry={run.head}
|
entry={child}
|
||||||
isSelected={run.head.id === selectedEntryId}
|
isSelected={child.id === selectedEntryId}
|
||||||
density={density}
|
density={density}
|
||||||
sourceLabel={sourceLabel}
|
sourceLabel={sourceLabel}
|
||||||
onSelect={onSelect}
|
onSelect={onSelect}
|
||||||
indent={20}
|
indent={20}
|
||||||
stageHint={deriveStageHint(run.head)}
|
stageHint={deriveStageHint(child)}
|
||||||
repeatCount={run.count > 1 ? run.count : undefined}
|
|
||||||
/>
|
/>
|
||||||
))
|
))
|
||||||
: null}
|
: null}
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
import { describe, expect, it } from 'vitest';
|
import { describe, expect, it } from 'vitest';
|
||||||
import type { LogsEntry } from '@/lib/api-client';
|
import type { LogsEntry } from '@/lib/api-client';
|
||||||
import {
|
import {
|
||||||
coalesceChildren,
|
|
||||||
deriveStageHint,
|
deriveStageHint,
|
||||||
deriveTraceGroups,
|
deriveTraceGroups,
|
||||||
type LeafItem,
|
type LeafItem,
|
||||||
@@ -51,53 +50,6 @@ describe('deriveStageHint', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('coalesceChildren', () => {
|
|
||||||
it('returns empty array for empty input', () => {
|
|
||||||
expect(coalesceChildren([])).toEqual([]);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('passes through a single child as count=1', () => {
|
|
||||||
const e = entry({ id: '1', timestamp: 't', event: 'a.b' });
|
|
||||||
expect(coalesceChildren([e])).toEqual([{ head: e, count: 1 }]);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('coalesces two adjacent identical children', () => {
|
|
||||||
const a = entry({ id: '1', timestamp: 't1', event: 'a.b', message: 'm' });
|
|
||||||
const b = entry({ id: '2', timestamp: 't2', event: 'a.b', message: 'm' });
|
|
||||||
const result = coalesceChildren([a, b]);
|
|
||||||
expect(result).toHaveLength(1);
|
|
||||||
expect(result[0]?.count).toBe(2);
|
|
||||||
expect(result[0]?.head).toBe(a);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('keeps children with different messages distinct (round-1 fix)', () => {
|
|
||||||
const a = entry({ id: '1', timestamp: 't1', event: 'auth.ok', message: 'alice' });
|
|
||||||
const b = entry({ id: '2', timestamp: 't2', event: 'auth.ok', message: 'bob' });
|
|
||||||
expect(coalesceChildren([a, b])).toHaveLength(2);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('keeps children with different sources distinct (round-2 fix)', () => {
|
|
||||||
const a = entry({ id: '1', timestamp: 't1', source: 'svc-a', event: 'e' });
|
|
||||||
const b = entry({ id: '2', timestamp: 't2', source: 'svc-b', event: 'e' });
|
|
||||||
expect(coalesceChildren([a, b])).toHaveLength(2);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('keeps children with different stages distinct', () => {
|
|
||||||
const a = entry({ id: '1', timestamp: 't1', stage: 'route', event: 'e' });
|
|
||||||
const b = entry({ id: '2', timestamp: 't2', stage: 'dispatch', event: 'e' });
|
|
||||||
expect(coalesceChildren([a, b])).toHaveLength(2);
|
|
||||||
});
|
|
||||||
|
|
||||||
it('only collapses adjacent duplicates — interleaving breaks the run', () => {
|
|
||||||
const a = entry({ id: '1', timestamp: 't1', event: 'a' });
|
|
||||||
const b = entry({ id: '2', timestamp: 't2', event: 'b' });
|
|
||||||
const c = entry({ id: '3', timestamp: 't3', event: 'a' });
|
|
||||||
const result = coalesceChildren([a, b, c]);
|
|
||||||
// a, b, a — three distinct runs of count 1 each
|
|
||||||
expect(result.map((r) => r.count)).toEqual([1, 1, 1]);
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
describe('deriveTraceGroups', () => {
|
describe('deriveTraceGroups', () => {
|
||||||
it('returns empty array for empty input', () => {
|
it('returns empty array for empty input', () => {
|
||||||
expect(deriveTraceGroups([])).toEqual([]);
|
expect(deriveTraceGroups([])).toEqual([]);
|
||||||
|
|||||||
Reference in New Issue
Block a user