Skip to content

Commit a5e4011

Browse files
committed
address comments
Signed-off-by: Joshua Li <joshuali925@gmail.com>
1 parent 44a2725 commit a5e4011

17 files changed

Lines changed: 315 additions & 133 deletions

src/plugins/agent_traces/public/application/pages/traces/flow/node_types.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ describe('nodeTypes', () => {
1414
expect(nodeTypes.agent).toBe('MockSpanNode');
1515
expect(nodeTypes.llm).toBe('MockSpanNode');
1616
expect(nodeTypes.tool).toBe('MockSpanNode');
17-
expect(nodeTypes.error).toBe('MockSpanNode');
17+
expect(nodeTypes.content).toBe('MockSpanNode');
18+
expect(nodeTypes.embeddings).toBe('MockSpanNode');
19+
expect(nodeTypes.retrieval).toBe('MockSpanNode');
1820
expect(nodeTypes.other).toBe('MockSpanNode');
1921
});
2022
});

src/plugins/agent_traces/public/application/pages/traces/flow/node_types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,5 @@ export const nodeTypes: NodeTypes = {
1414
content: SpanNode,
1515
embeddings: SpanNode,
1616
retrieval: SpanNode,
17-
error: SpanNode,
1817
other: SpanNode,
1918
};

src/plugins/agent_traces/public/application/pages/traces/flow/span_node.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('SpanNode', () => {
6565

6666
it('renders category badge', () => {
6767
render(<SpanNode data={{ span: makeSpan(), totalDuration: 1000 }} />);
68-
expect(screen.getByText('AGENT')).toBeInTheDocument();
68+
expect(screen.getByText('Agent')).toBeInTheDocument();
6969
});
7070

7171
it('renders latency', () => {

src/plugins/agent_traces/public/application/pages/traces/flow/span_node.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
hexToRgba,
1414
SpanCategory,
1515
} from '../../../../services/span_categorization';
16-
import { parseLatencyMs } from '../flyout/tree_helpers';
16+
import { parseLatencyMs } from '../trace_details/utils/span_timerange_utils';
1717
import './span_node.scss';
1818

1919
interface SpanNodeProps {

src/plugins/agent_traces/public/application/pages/traces/flyout/timeline_gantt.tsx

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import { FormattedMessage } from '@osd/i18n/react';
1818
import { euiThemeVars } from '@osd/ui-shared-deps/theme';
1919
import { getCategoryMeta, hexToRgba } from '../../../../services/span_categorization';
20+
import { formatMs } from '../trace_details/utils/span_timerange_utils';
2021
import { TimelineSpan } from './tree_helpers';
2122
import './timeline_gantt.scss';
2223

@@ -31,11 +32,6 @@ export interface TimelineGanttProps {
3132
onToggleExpanded: (nodeId: string) => void;
3233
}
3334

34-
const formatRelativeMs = (ms: number): string => {
35-
if (ms >= 1000) return `${(ms / 1000).toFixed(2)} s`;
36-
return `${ms.toFixed(2)} ms`;
37-
};
38-
3935
export const TimelineGantt: React.FC<TimelineGanttProps> = ({
4036
timelineVisibleSpans,
4137
timelineRange,
@@ -173,15 +169,13 @@ export const TimelineGantt: React.FC<TimelineGanttProps> = ({
173169
<div style={{ lineHeight: 1.6 }}>
174170
<div>
175171
<strong>Duration:</strong>{' '}
176-
{span.node.latency || formatRelativeMs(span.durationMs)}
172+
{span.node.latency || formatMs(span.durationMs)}
177173
</div>
178174
<div>
179-
<strong>Start:</strong>{' '}
180-
{formatRelativeMs(span.startMs - timelineRange.minMs)}
175+
<strong>Start:</strong> {formatMs(span.startMs - timelineRange.minMs)}
181176
</div>
182177
<div>
183-
<strong>End:</strong>{' '}
184-
{formatRelativeMs(span.endMs - timelineRange.minMs)}
178+
<strong>End:</strong> {formatMs(span.endMs - timelineRange.minMs)}
185179
</div>
186180
</div>
187181
}

src/plugins/agent_traces/public/application/pages/traces/flyout/trace_details_flyout.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ import {
2222
} from '@elastic/eui';
2323
import { TraceRow } from '../hooks/use_agent_traces';
2424
import { TraceFlowView } from '../flow/trace_flow_view';
25+
import { parseLatencyMs } from '../trace_details/utils/span_timerange_utils';
2526
import {
2627
TreeNode,
2728
buildTreeFromTraceRow,
2829
flattenTree,
2930
countSpans,
3031
sumTokens,
31-
parseLatencyMs,
3232
flattenVisibleNodes,
3333
calculateTimelineRange,
3434
} from './tree_helpers';

src/plugins/agent_traces/public/application/pages/traces/flyout/tree_helpers.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
countSpans,
1111
sumTokens,
1212
parseLatencyMs,
13-
parseRawTimestampMs,
13+
parseTimestampMs,
1414
extractTimestamps,
1515
flattenVisibleNodes,
1616
calculateTimelineRange,
@@ -128,21 +128,21 @@ describe('tree_helpers', () => {
128128
});
129129
});
130130

131-
describe('parseRawTimestampMs', () => {
131+
describe('parseTimestampMs', () => {
132132
it('parses ISO timestamp', () => {
133-
const ms = parseRawTimestampMs('2025-01-01T00:00:00Z');
133+
const ms = parseTimestampMs('2025-01-01T00:00:00Z');
134134
expect(ms).toBe(new Date('2025-01-01T00:00:00Z').getTime());
135135
});
136136

137137
it('normalizes space-separated timestamps', () => {
138-
const ms = parseRawTimestampMs('2025-01-01 12:00:00');
138+
const ms = parseTimestampMs('2025-01-01 12:00:00');
139139
expect(ms).toBeGreaterThan(0);
140140
});
141141

142142
it('returns 0 for invalid input', () => {
143-
expect(parseRawTimestampMs(null)).toBe(0);
144-
expect(parseRawTimestampMs('')).toBe(0);
145-
expect(parseRawTimestampMs(123)).toBe(0);
143+
expect(parseTimestampMs(null)).toBe(0);
144+
expect(parseTimestampMs('')).toBe(0);
145+
expect(parseTimestampMs(123)).toBe(0);
146146
});
147147
});
148148

src/plugins/agent_traces/public/application/pages/traces/flyout/tree_helpers.ts

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
*/
55

66
import { TraceRow } from '../hooks/use_agent_traces';
7+
import { parseLatencyMs, parseTimestampMs } from '../trace_details/utils/span_timerange_utils';
78
import {
89
getSpanCategory,
910
getCategoryMeta,
1011
SpanCategory,
1112
} from '../../../../services/span_categorization';
1213

14+
export { parseLatencyMs, parseTimestampMs };
15+
1316
export interface TreeNode {
1417
label: string;
1518
id: string;
@@ -81,38 +84,18 @@ export const sumTokens = (nodes: TreeNode[]): number => {
8184
return total;
8285
};
8386

84-
export const parseLatencyMs = (latency?: string): number => {
85-
if (!latency || latency === '—') return 0;
86-
if (latency.endsWith('ms')) return parseFloat(latency) || 0;
87-
if (latency.endsWith('s')) return (parseFloat(latency) || 0) * 1000;
88-
return 0;
89-
};
90-
91-
export const parseRawTimestampMs = (ts: unknown): number => {
92-
if (!ts || typeof ts !== 'string') return 0;
93-
let normalized = ts;
94-
if (ts.includes(' ') && !ts.includes('T')) {
95-
normalized = ts.replace(' ', 'T');
96-
if (!normalized.includes('Z') && !normalized.includes('+')) {
97-
normalized += 'Z';
98-
}
99-
}
100-
const ms = new Date(normalized).getTime();
101-
return isNaN(ms) ? 0 : ms;
102-
};
103-
10487
export const extractTimestamps = (node: TreeNode): { startMs: number; endMs: number } => {
10588
const raw = node.traceRow?.rawDocument;
10689
if (raw) {
10790
const rawStart = raw.startTime;
108-
const s = parseRawTimestampMs(rawStart);
91+
const s = parseTimestampMs(rawStart);
10992
if (s > 0) {
11093
// Prefer durationInNanos for sub-ms precision; fall back to endTime.
11194
const durationNanos = (raw.durationInNanos as number) || 0;
11295
if (durationNanos > 0) {
11396
return { startMs: s, endMs: s + durationNanos / 1_000_000 };
11497
}
115-
const e = parseRawTimestampMs(raw.endTime);
98+
const e = parseTimestampMs(raw.endTime);
11699
if (e > 0) return { startMs: s, endMs: e };
117100
}
118101
}

src/plugins/agent_traces/public/application/pages/traces/hooks/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,9 @@ export { useTraceMetrics, useTraceMetricsContext, TraceMetricsContext } from './
99
export type { TraceMetrics } from './use_trace_metrics';
1010
export { formatDuration, traceHitToAgentSpan } from './span_transforms';
1111
export type { AgentSpan } from './span_transforms';
12+
export {
13+
formatMs,
14+
parseLatencyMs,
15+
parseTimestampMs,
16+
} from '../trace_details/utils/span_timerange_utils';
1217
export type { BaseRow, LoadingState } from './tree_utils';

src/plugins/agent_traces/public/application/pages/traces/hooks/tree_utils.test.ts

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import { setLevels, spanToRow, buildFullSpanTree, hitsToAgentSpans, BaseRow } from './tree_utils';
6+
import {
7+
formatTimestamp,
8+
setLevels,
9+
spanToRow,
10+
buildFullSpanTree,
11+
hitsToAgentSpans,
12+
} from './tree_utils';
713
import { AgentSpan } from './span_transforms';
814

915
const makeSpan = (overrides: Partial<AgentSpan> = {}): AgentSpan => ({
@@ -31,11 +37,29 @@ const makeSpan = (overrides: Partial<AgentSpan> = {}): AgentSpan => ({
3137
});
3238

3339
describe('tree_utils', () => {
40+
describe('formatTimestamp', () => {
41+
it('formats UTC timestamp in the given timezone', () => {
42+
const result = formatTimestamp('2025-01-01 12:00:00', 'America/New_York');
43+
expect(result).toBe('01/01/2025, 7:00:00.000 AM');
44+
});
45+
46+
it('returns dash for empty timestamp', () => {
47+
expect(formatTimestamp('', 'UTC')).toBe('—');
48+
});
49+
50+
it('returns dash for invalid timestamp', () => {
51+
expect(formatTimestamp('not-a-date', 'UTC')).toBe('—');
52+
});
53+
54+
it('preserves millisecond precision', () => {
55+
const result = formatTimestamp('2025-06-15 08:30:45.789', 'UTC');
56+
expect(result).toContain('.789');
57+
});
58+
});
59+
3460
describe('setLevels', () => {
3561
it('sets levels recursively', () => {
36-
const rows: Array<{ level?: number; children?: any[] }> = [
37-
{ children: [{ children: [{}] }] },
38-
];
62+
const rows: any[] = [{ children: [{ children: [{}] }] }];
3963
setLevels(rows, 0);
4064
expect(rows[0].level).toBe(0);
4165
expect(rows[0].children![0].level).toBe(1);
@@ -74,7 +98,7 @@ describe('tree_utils', () => {
7498
makeSpan({ spanId: 'root', parentSpanId: null }),
7599
makeSpan({ spanId: 'child', parentSpanId: 'root' }),
76100
];
77-
const tree = buildFullSpanTree<BaseRow>(spans, (ts) => ts);
101+
const tree = buildFullSpanTree(spans, (ts) => ts);
78102

79103
expect(tree).toHaveLength(1);
80104
expect(tree[0].spanId).toBe('root');
@@ -87,11 +111,65 @@ describe('tree_utils', () => {
87111
makeSpan({ spanId: 'root', parentSpanId: null }),
88112
makeSpan({ spanId: 'child', parentSpanId: 'root' }),
89113
];
90-
const tree = buildFullSpanTree<BaseRow>(spans, (ts) => ts);
114+
const tree = buildFullSpanTree(spans, (ts) => ts);
91115

92116
expect(tree[0].level).toBe(0);
93117
expect(tree[0].children![0].level).toBe(1);
94118
});
119+
120+
it('sorts children earliest-first using rawDocument.startTime', () => {
121+
const spans = [
122+
makeSpan({ spanId: 'root', parentSpanId: null }),
123+
makeSpan({
124+
spanId: 'late',
125+
parentSpanId: 'root',
126+
rawDocument: { startTime: '2025-01-01 00:00:10' },
127+
}),
128+
makeSpan({
129+
spanId: 'early',
130+
parentSpanId: 'root',
131+
rawDocument: { startTime: '2025-01-01 00:00:01' },
132+
}),
133+
makeSpan({
134+
spanId: 'mid',
135+
parentSpanId: 'root',
136+
rawDocument: { startTime: '2025-01-01 00:00:05' },
137+
}),
138+
];
139+
const tree = buildFullSpanTree(spans, (ts) => ts);
140+
const childIds = tree[0].children!.map((c) => c.spanId);
141+
expect(childIds).toEqual(['early', 'mid', 'late']);
142+
});
143+
144+
it('handles spans without rawDocument.startTime in sort', () => {
145+
const spans = [
146+
makeSpan({ spanId: 'root', parentSpanId: null }),
147+
makeSpan({
148+
spanId: 'has-time',
149+
parentSpanId: 'root',
150+
rawDocument: { startTime: '2025-01-01 00:00:01' },
151+
}),
152+
makeSpan({
153+
spanId: 'no-time',
154+
parentSpanId: 'root',
155+
rawDocument: {},
156+
}),
157+
];
158+
const tree = buildFullSpanTree(spans, (ts) => ts);
159+
// Should not throw; span without startTime sorts to beginning (time 0)
160+
expect(tree[0].children).toHaveLength(2);
161+
expect(tree[0].children![0].spanId).toBe('no-time');
162+
expect(tree[0].children![1].spanId).toBe('has-time');
163+
});
164+
165+
it('treats orphan spans as roots', () => {
166+
const spans = [
167+
makeSpan({ spanId: 'a', parentSpanId: 'missing' }),
168+
makeSpan({ spanId: 'b', parentSpanId: null }),
169+
];
170+
const tree = buildFullSpanTree(spans, (ts) => ts);
171+
expect(tree).toHaveLength(2);
172+
});
95173
});
96174

97175
describe('hitsToAgentSpans', () => {

0 commit comments

Comments
 (0)