Skip to content

Commit 87e10b0

Browse files
jmoseleyCopilotSteveSandersonMS
authored
Add StartupTimings per-phase breakdown to Client::start (#2066)
* Add StartupTimings breakdown to Client::start Introduce a `StartupTimings` struct that decomposes the CLI spawn + handshake cost into per-phase millisecond fields, so hosts can attribute "time to first token" startup latency to a specific phase instead of reconstructing it from scattered debug lines. Phases captured: - program_resolve_ms: NEW timer around bundled-CLI resolution/extraction (`resolve::copilot_binary_with_extract_dir`), previously untimed and the prime suspect for cold-start cost on Windows. - process_spawn_ms: subprocess `command.spawn()`. - port_wait_ms: TCP port-announcement wait (tcp transport only). - handshake_ms: `verify_protocol_version` connect round-trip. - session_fs_ms / llm_handler_ms: post-handshake provider-registration RPCs. - total_ms: full Client::start wall-clock. Surfacing is non-breaking: timings are stored on ClientInner via a `OnceLock` and exposed through a new `Client::startup_timings()` getter, plus a single structured `debug!` event. `Client::start`'s return type is unchanged. Existing per-phase debug logs are retained. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95834bd3-b7ac-40bc-8bf4-60102600d41a * Harden startup timing observability Emit numeric structured fields, cover all transport setup, tighten always-present timing values, and exercise startup timings in Rust E2E tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify optional startup timing fields Record each optional timing as a numeric span field when present and the string None otherwise, without companion presence flags. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Steve Sanderson <1101362+SteveSandersonMS@users.noreply.github.com> Copilot-Session: 95834bd3-b7ac-40bc-8bf4-60102600d41a
1 parent 8d3b493 commit 87e10b0

5 files changed

Lines changed: 235 additions & 12 deletions

File tree

‎rust/README.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,20 @@ let pong = client.ping("hello").await?;
7070
client.stop().await?;
7171
```
7272

73+
After `Client::start` succeeds, inspect its startup cost without parsing logs:
74+
75+
```rust,ignore
76+
let timings = client.startup_timings().expect("started by Client::start");
77+
println!(
78+
"startup={}ms transport={}ms handshake={}ms",
79+
timings.total_ms, timings.transport_setup_ms, timings.handshake_ms
80+
);
81+
```
82+
83+
Transport-specific phases are optional. For example, `port_wait_ms` is present
84+
only for TCP and `process_spawn_ms` is absent for external and in-process
85+
transports.
86+
7387
**`ClientOptions`:**
7488

7589
| Field | Type | Description |

‎rust/src/lib.rs‎

Lines changed: 90 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ pub mod session;
4040
/// Custom session filesystem provider (virtualizable filesystem layer).
4141
pub mod session_fs;
4242
mod session_fs_dispatch;
43+
/// Per-phase timing breakdown for [`Client::start`].
44+
pub mod startup_timings;
4345
/// Event subscription handles returned by `subscribe()` methods.
4446
pub mod subscription;
4547
/// Typed tool definition framework and dispatch router.
@@ -106,12 +108,24 @@ pub use types::*;
106108

107109
mod sdk_protocol_version;
108110
pub use sdk_protocol_version::{SDK_PROTOCOL_VERSION, get_sdk_protocol_version};
111+
pub use startup_timings::StartupTimings;
109112
pub use subscription::{EventSubscription, LifecycleSubscription};
110113

111114
/// Minimum protocol version this SDK can communicate with.
112115
const MIN_PROTOCOL_VERSION: u32 = 3;
113116
const RUNTIME_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10);
114117

118+
fn record_optional_millis(span: &tracing::Span, field: &'static str, value: Option<u64>) {
119+
match value {
120+
Some(value) => {
121+
span.record(field, value);
122+
}
123+
None => {
124+
span.record(field, "None");
125+
}
126+
}
127+
}
128+
115129
/// How the SDK communicates with the CLI server.
116130
#[derive(Debug, Default)]
117131
#[non_exhaustive]
@@ -1007,6 +1021,10 @@ struct ClientInner {
10071021
/// SDK [`ClientMode`] captured at start time. Drives empty-mode safe
10081022
/// defaults inside `create_session` / `resume_session`.
10091023
pub(crate) mode: ClientMode,
1024+
/// Per-phase startup timing breakdown, populated once at the end of
1025+
/// [`Client::start`]. Empty for clients built via [`Client::from_streams`]
1026+
/// or [`Client::from_transport`] directly.
1027+
startup_timings: OnceLock<StartupTimings>,
10101028
}
10111029

10121030
impl Client {
@@ -1024,6 +1042,7 @@ impl Client {
10241042
/// backend.
10251043
pub async fn start(options: ClientOptions) -> Result<Self> {
10261044
let start_time = Instant::now();
1045+
let mut timings = StartupTimings::default();
10271046
let mut options = options;
10281047
if matches!(options.transport, Transport::Default) {
10291048
options.transport = resolve_default_transport(&options)?;
@@ -1119,9 +1138,16 @@ impl Client {
11191138
path.clone()
11201139
}
11211140
CliProgram::Resolve => {
1141+
let resolve_start = Instant::now();
11221142
let resolved = resolve::copilot_binary_with_extract_dir(
11231143
options.bundled_cli_extract_dir.as_deref(),
11241144
)?;
1145+
let resolve_elapsed = resolve_start.elapsed();
1146+
timings.program_resolve_ms = Some(StartupTimings::millis(resolve_elapsed));
1147+
debug!(
1148+
elapsed_ms = resolve_elapsed.as_millis(),
1149+
"Client::start CLI program resolution complete"
1150+
);
11251151
info!(path = %resolved.display(), "resolved copilot CLI");
11261152
#[cfg(windows)]
11271153
{
@@ -1148,6 +1174,7 @@ impl Client {
11481174
}
11491175
};
11501176

1177+
let transport_setup_start = Instant::now();
11511178
let client = match options.transport {
11521179
Transport::Default => unreachable!("default transport resolved above"),
11531180
Transport::External {
@@ -1183,8 +1210,10 @@ impl Client {
11831210
port,
11841211
connection_token: _,
11851212
} => {
1186-
let (mut child, actual_port) =
1213+
let (mut child, actual_port, spawn_elapsed, port_wait_elapsed) =
11871214
Self::spawn_tcp(&program, &options, &working_directory, port).await?;
1215+
timings.process_spawn_ms = Some(StartupTimings::millis(spawn_elapsed));
1216+
timings.port_wait_ms = Some(StartupTimings::millis(port_wait_elapsed));
11881217
let connect_start = Instant::now();
11891218
let stream = TcpStream::connect(("127.0.0.1", actual_port)).await?;
11901219
debug!(
@@ -1209,7 +1238,9 @@ impl Client {
12091238
)?
12101239
}
12111240
Transport::Stdio => {
1212-
let mut child = Self::spawn_stdio(&program, &options, &working_directory)?;
1241+
let (mut child, spawn_elapsed) =
1242+
Self::spawn_stdio(&program, &options, &working_directory)?;
1243+
timings.process_spawn_ms = Some(StartupTimings::millis(spawn_elapsed));
12131244
let stdin = child.stdin.take().expect("stdin is piped");
12141245
let stdout = child.stdout.take().expect("stdout is piped");
12151246
Self::drain_stderr(&mut child);
@@ -1290,11 +1321,14 @@ impl Client {
12901321
unreachable!("in-process feature validation returned above")
12911322
}
12921323
};
1324+
timings.transport_setup_ms = StartupTimings::millis(transport_setup_start.elapsed());
12931325
debug!(
12941326
elapsed_ms = start_time.elapsed().as_millis(),
12951327
"Client::start transport setup complete"
12961328
);
1329+
let handshake_start = Instant::now();
12971330
client.verify_protocol_version().await?;
1331+
timings.handshake_ms = StartupTimings::millis(handshake_start.elapsed());
12981332
debug!(
12991333
elapsed_ms = start_time.elapsed().as_millis(),
13001334
"Client::start protocol verification complete"
@@ -1313,8 +1347,10 @@ impl Client {
13131347
session_state_path: cfg.session_state_path,
13141348
};
13151349
client.rpc().session_fs().set_provider(request).await?;
1350+
let session_fs_elapsed = session_fs_start.elapsed();
1351+
timings.session_fs_ms = Some(StartupTimings::millis(session_fs_elapsed));
13161352
debug!(
1317-
elapsed_ms = session_fs_start.elapsed().as_millis(),
1353+
elapsed_ms = session_fs_elapsed.as_millis(),
13181354
"Client::start session filesystem setup complete"
13191355
);
13201356
}
@@ -1334,11 +1370,38 @@ impl Client {
13341370
client.inner.on_github_telemetry.clone(),
13351371
);
13361372
client.rpc().llm_inference().set_provider().await?;
1373+
let llm_inference_elapsed = llm_inference_start.elapsed();
1374+
timings.llm_handler_ms = Some(StartupTimings::millis(llm_inference_elapsed));
13371375
debug!(
1338-
elapsed_ms = llm_inference_start.elapsed().as_millis(),
1376+
elapsed_ms = llm_inference_elapsed.as_millis(),
13391377
"Client::start Copilot request handler registration complete"
13401378
);
13411379
}
1380+
timings.total_ms = StartupTimings::millis(start_time.elapsed());
1381+
// A span allows optional fields to retain their numeric type when
1382+
// present while recording an explicit "None" when a phase did not run.
1383+
let timings_span = tracing::debug_span!(
1384+
"Client::start timings",
1385+
program_resolve_ms = tracing::field::Empty,
1386+
process_spawn_ms = tracing::field::Empty,
1387+
port_wait_ms = tracing::field::Empty,
1388+
transport_setup_ms = timings.transport_setup_ms,
1389+
handshake_ms = timings.handshake_ms,
1390+
session_fs_ms = tracing::field::Empty,
1391+
llm_handler_ms = tracing::field::Empty,
1392+
total_ms = timings.total_ms,
1393+
);
1394+
record_optional_millis(
1395+
&timings_span,
1396+
"program_resolve_ms",
1397+
timings.program_resolve_ms,
1398+
);
1399+
record_optional_millis(&timings_span, "process_spawn_ms", timings.process_spawn_ms);
1400+
record_optional_millis(&timings_span, "port_wait_ms", timings.port_wait_ms);
1401+
record_optional_millis(&timings_span, "session_fs_ms", timings.session_fs_ms);
1402+
record_optional_millis(&timings_span, "llm_handler_ms", timings.llm_handler_ms);
1403+
timings_span.in_scope(|| debug!("Client::start timings"));
1404+
let _ = client.inner.startup_timings.set(timings);
13421405
debug!(
13431406
elapsed_ms = start_time.elapsed().as_millis(),
13441407
"Client::start complete"
@@ -1507,6 +1570,7 @@ impl Client {
15071570
on_get_trace_context,
15081571
effective_connection_token,
15091572
mode,
1573+
startup_timings: OnceLock::new(),
15101574
}),
15111575
};
15121576
client.spawn_lifecycle_dispatcher();
@@ -1683,7 +1747,7 @@ impl Client {
16831747
program: &Path,
16841748
options: &ClientOptions,
16851749
working_directory: &Path,
1686-
) -> Result<Child> {
1750+
) -> Result<(Child, Duration)> {
16871751
info!(cwd = ?working_directory, program = %program.display(), "spawning copilot CLI (stdio)");
16881752
let mut command = Self::build_command(program, options, working_directory);
16891753
command
@@ -1696,19 +1760,20 @@ impl Client {
16961760
.stdin(Stdio::piped());
16971761
let spawn_start = Instant::now();
16981762
let child = command.spawn()?;
1763+
let spawn_elapsed = spawn_start.elapsed();
16991764
debug!(
1700-
elapsed_ms = spawn_start.elapsed().as_millis(),
1765+
elapsed_ms = spawn_elapsed.as_millis(),
17011766
"Client::spawn_stdio subprocess spawned"
17021767
);
1703-
Ok(child)
1768+
Ok((child, spawn_elapsed))
17041769
}
17051770

17061771
async fn spawn_tcp(
17071772
program: &Path,
17081773
options: &ClientOptions,
17091774
working_directory: &Path,
17101775
port: u16,
1711-
) -> Result<(Child, u16)> {
1776+
) -> Result<(Child, u16, Duration, Duration)> {
17121777
info!(cwd = ?working_directory, program = %program.display(), port = %port, "spawning copilot CLI (tcp)");
17131778
let mut command = Self::build_command(program, options, working_directory);
17141779
command
@@ -1721,8 +1786,9 @@ impl Client {
17211786
.stdin(Stdio::null());
17221787
let spawn_start = Instant::now();
17231788
let mut child = command.spawn()?;
1789+
let spawn_elapsed = spawn_start.elapsed();
17241790
debug!(
1725-
elapsed_ms = spawn_start.elapsed().as_millis(),
1791+
elapsed_ms = spawn_elapsed.as_millis(),
17261792
"Client::spawn_tcp subprocess spawned"
17271793
);
17281794
let stdout = child.stdout.take().expect("stdout is piped");
@@ -1759,13 +1825,14 @@ impl Client {
17591825
.map_err(|_| Error::from(ErrorKind::Protocol(ProtocolErrorKind::CliStartupTimeout)))?
17601826
.map_err(|_| Error::from(ErrorKind::Protocol(ProtocolErrorKind::CliStartupFailed)))?;
17611827

1828+
let port_wait_elapsed = port_wait_start.elapsed();
17621829
debug!(
1763-
elapsed_ms = port_wait_start.elapsed().as_millis(),
1830+
elapsed_ms = port_wait_elapsed.as_millis(),
17641831
port = actual_port,
17651832
"Client::spawn_tcp TCP port wait complete"
17661833
);
17671834
info!(port = %actual_port, "CLI server listening");
1768-
Ok((child, actual_port))
1835+
Ok((child, actual_port, spawn_elapsed, port_wait_elapsed))
17691836
}
17701837

17711838
fn drain_stderr(child: &mut Child) {
@@ -1942,6 +2009,16 @@ impl Client {
19422009
self.inner.negotiated_protocol_version.get().copied()
19432010
}
19442011

2012+
/// Returns the per-phase [`StartupTimings`] breakdown captured during
2013+
/// [`start`](Self::start), if available.
2014+
///
2015+
/// Returns `None` for clients created via
2016+
/// [`from_streams`](Self::from_streams), which bypasses the timed startup
2017+
/// sequence.
2018+
pub fn startup_timings(&self) -> Option<StartupTimings> {
2019+
self.inner.startup_timings.get().cloned()
2020+
}
2021+
19452022
/// Verify the CLI server's protocol version is within the supported range.
19462023
///
19472024
/// Called automatically by [`start`](Self::start). Call manually after
@@ -3065,6 +3142,7 @@ mod tests {
30653142
let (client_write, _server_read) = tokio::io::duplex(8192);
30663143
let (_server_write, client_read) = tokio::io::duplex(8192);
30673144
let client = Client::from_streams(client_read, client_write, std::env::temp_dir()).unwrap();
3145+
assert!(client.startup_timings().is_none());
30683146
let session_id = SessionId::new("resume-cancel-test");
30693147
let handle = tokio::spawn({
30703148
let client = client.clone();
@@ -3112,6 +3190,7 @@ mod tests {
31123190
on_get_trace_context: None,
31133191
effective_connection_token: None,
31143192
mode: ClientMode::default(),
3193+
startup_timings: OnceLock::new(),
31153194
}),
31163195
}
31173196
}

‎rust/src/startup_timings.rs‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
//! Per-phase timing breakdown for [`Client::start`](crate::Client::start).
2+
//!
3+
//! `Client::start` performs several sequential phases between "spawn the CLI"
4+
//! and "client is ready to create sessions": resolving (and possibly
5+
//! extracting) the CLI binary, spawning the subprocess, waiting for the TCP
6+
//! port announcement, the `connect` protocol handshake, and the optional
7+
//! `sessionFs.setProvider` / `llmInference.setProvider` registration RPCs.
8+
//!
9+
//! Each phase is already measured internally with an [`Instant`] and logged at
10+
//! `debug`. [`StartupTimings`] aggregates those durations into a single value
11+
//! so a host can attribute total startup latency ("time to first token"
12+
//! groundwork) to a specific phase — e.g. separating "process exec cost" from
13+
//! "handshake/negotiation cost" — instead of reconstructing it from scattered
14+
//! log lines.
15+
//!
16+
//! Retrieve it after start via
17+
//! [`Client::startup_timings`](crate::Client::startup_timings).
18+
//!
19+
//! [`Instant`]: std::time::Instant
20+
21+
use std::time::Duration;
22+
23+
/// Millisecond breakdown of the phases of [`Client::start`](crate::Client::start).
24+
///
25+
/// Optional fields represent phases that do not run for every configuration:
26+
/// `program_resolve_ms` is `None` when the caller supplies an explicit CLI path
27+
/// (no resolution/extraction), `port_wait_ms` is `Some` only for the TCP
28+
/// transport, and `session_fs_ms` / `llm_handler_ms` are `Some` only when the
29+
/// corresponding option is configured. `process_spawn_ms` is `None` for
30+
/// transports that do not spawn a subprocess (external server, in-process FFI
31+
/// runtime). `transport_setup_ms`, `handshake_ms`, and `total_ms` are always
32+
/// populated for a value returned by
33+
/// [`Client::startup_timings`](crate::Client::startup_timings).
34+
///
35+
/// Durations are whole milliseconds, matching the existing `elapsed_ms`
36+
/// tracing fields.
37+
#[derive(Debug, Clone, Default, PartialEq, Eq)]
38+
#[non_exhaustive]
39+
pub struct StartupTimings {
40+
/// Time spent in `resolve::copilot_binary_with_extract_dir` locating (and,
41+
/// for a bundled CLI, extracting) the copilot binary. `None` when the
42+
/// caller passes an explicit [`CliProgram::Path`](crate::CliProgram::Path).
43+
pub program_resolve_ms: Option<u64>,
44+
/// Time spent spawning the CLI subprocess (`command.spawn()`). `None` for
45+
/// the external-server and in-process transports, which do not spawn a
46+
/// child.
47+
pub process_spawn_ms: Option<u64>,
48+
/// Time spent waiting for the TCP server to announce its listening port on
49+
/// stdout. `Some` only for the TCP transport.
50+
pub port_wait_ms: Option<u64>,
51+
/// Total transport setup time. This includes spawning and connecting to a
52+
/// subprocess, connecting to an external server, or starting the in-process
53+
/// FFI runtime. `process_spawn_ms` and `port_wait_ms` provide nested detail
54+
/// for spawned transports.
55+
pub transport_setup_ms: u64,
56+
/// Time spent on the `connect` protocol handshake in
57+
/// [`Client::verify_protocol_version`](crate::Client::verify_protocol_version),
58+
/// including the fallback to the legacy `ping` RPC.
59+
pub handshake_ms: u64,
60+
/// Time spent registering the filesystem provider via
61+
/// `sessionFs.setProvider`. `Some` only when
62+
/// [`ClientOptions::session_fs`](crate::ClientOptions::session_fs) is set.
63+
pub session_fs_ms: Option<u64>,
64+
/// Time spent registering the LLM inference provider via
65+
/// `llmInference.setProvider`. `Some` only when
66+
/// [`ClientOptions::request_handler`](crate::ClientOptions::request_handler)
67+
/// is set.
68+
pub llm_handler_ms: Option<u64>,
69+
/// Total wall-clock time for [`Client::start`](crate::Client::start), from
70+
/// entry to the client being ready. Always present.
71+
pub total_ms: u64,
72+
}
73+
74+
impl StartupTimings {
75+
/// Whole milliseconds of `duration`, saturating at [`u64::MAX`].
76+
pub(crate) fn millis(duration: Duration) -> u64 {
77+
u64::try_from(duration.as_millis()).unwrap_or(u64::MAX)
78+
}
79+
}
80+
81+
#[cfg(test)]
82+
mod tests {
83+
use super::*;
84+
85+
#[test]
86+
fn millis_truncates_to_whole_milliseconds() {
87+
assert_eq!(StartupTimings::millis(Duration::from_micros(1_999)), 1);
88+
assert_eq!(StartupTimings::millis(Duration::from_millis(250)), 250);
89+
assert_eq!(StartupTimings::millis(Duration::ZERO), 0);
90+
}
91+
92+
#[test]
93+
fn default_leaves_every_phase_unset() {
94+
let timings = StartupTimings::default();
95+
assert_eq!(timings, StartupTimings::default());
96+
assert!(timings.program_resolve_ms.is_none());
97+
assert!(timings.process_spawn_ms.is_none());
98+
assert!(timings.port_wait_ms.is_none());
99+
assert_eq!(timings.transport_setup_ms, 0);
100+
assert_eq!(timings.handshake_ms, 0);
101+
assert!(timings.session_fs_ms.is_none());
102+
assert!(timings.llm_handler_ms.is_none());
103+
assert_eq!(timings.total_ms, 0);
104+
}
105+
}

0 commit comments

Comments
 (0)