Skip to content

Commit a7fb3b8

Browse files
authored
chore: add diagnostic logging to JSONRPC handlers (Fixes #360) (#361)
Add diagnostic logging to JSONRPC handlers that were missing visibility, particularly `handle_configure` which can hang during glob expansion. - **handle_configure**: `info!` on entry/exit with elapsed time, `trace!` for each glob pattern being expanded, `warn!` if glob expansion exceeds 5 seconds (client timeout is 30s). Also moved glob expansion outside the write lock to avoid blocking configuration readers during slow filesystem traversal. - **handle_find**: `trace!` on entry/exit with elapsed time and result count - **handle_conda_telemetry**: `trace!` on entry/exit This addresses the diagnostic gap where PET produces zero output during a configure request that times out (e.g., when `environmentDirectories` contains recursive patterns like `**/.venv` on large directory trees). Fixes #360 Related: #357
1 parent 3def20f commit a7fb3b8

File tree

1 file changed

+55
-12
lines changed

1 file changed

+55
-12
lines changed

crates/pet/src/jsonrpc.rs

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::find::identify_python_executables_using_locators;
77
use crate::find::SearchScope;
88
use crate::locators::create_locators;
99
use lazy_static::lazy_static;
10-
use log::{error, info, trace};
10+
use log::{error, info, trace, warn};
1111
use pet::initialize_tracing;
1212
use pet::resolve::resolve_environment;
1313
use pet_conda::Conda;
@@ -46,7 +46,7 @@ use std::{
4646
path::PathBuf,
4747
sync::{Arc, RwLock},
4848
thread,
49-
time::SystemTime,
49+
time::{Instant, SystemTime},
5050
};
5151
use tracing::info_span;
5252

@@ -110,28 +110,60 @@ pub struct ConfigureOptions {
110110
pub cache_directory: Option<PathBuf>,
111111
}
112112

113+
/// Threshold for glob expansion duration before emitting a warning.
114+
/// The client has a 30-second timeout for configure requests.
115+
const GLOB_EXPANSION_WARN_THRESHOLD: Duration = Duration::from_secs(5);
116+
113117
pub fn handle_configure(context: Arc<Context>, id: u32, params: Value) {
114118
match serde_json::from_value::<ConfigureOptions>(params.clone()) {
115119
Ok(configure_options) => {
120+
info!("Received configure request");
116121
// Start in a new thread, we can have multiple requests.
117122
thread::spawn(move || {
118-
let mut cfg = context.configuration.write().unwrap();
119-
// Expand glob patterns in workspace_directories
120-
cfg.workspace_directories = configure_options.workspace_directories.map(|dirs| {
121-
expand_glob_patterns(&dirs)
123+
let now = Instant::now();
124+
125+
// Expand glob patterns before acquiring the write lock so we
126+
// don't block readers/writers while traversing the filesystem.
127+
let workspace_directories = configure_options.workspace_directories.map(|dirs| {
128+
let start = Instant::now();
129+
let result: Vec<PathBuf> = expand_glob_patterns(&dirs)
122130
.into_iter()
123131
.filter(|p| p.is_dir())
124-
.collect()
132+
.collect();
133+
trace!(
134+
"Expanded workspace directory patterns ({:?}) in {:?}",
135+
dirs,
136+
start.elapsed()
137+
);
138+
result
125139
});
126-
cfg.conda_executable = configure_options.conda_executable;
127-
// Expand glob patterns in environment_directories
128-
cfg.environment_directories =
140+
let environment_directories =
129141
configure_options.environment_directories.map(|dirs| {
130-
expand_glob_patterns(&dirs)
142+
let start = Instant::now();
143+
let result: Vec<PathBuf> = expand_glob_patterns(&dirs)
131144
.into_iter()
132145
.filter(|p| p.is_dir())
133-
.collect()
146+
.collect();
147+
trace!(
148+
"Expanded environment directory patterns ({:?}) in {:?}",
149+
dirs,
150+
start.elapsed()
151+
);
152+
result
134153
});
154+
let glob_elapsed = now.elapsed();
155+
trace!("Glob expansion completed in {:?}", glob_elapsed);
156+
if glob_elapsed >= GLOB_EXPANSION_WARN_THRESHOLD {
157+
warn!(
158+
"Glob expansion took {:?}, this may cause client timeouts",
159+
glob_elapsed
160+
);
161+
}
162+
163+
let mut cfg = context.configuration.write().unwrap();
164+
cfg.workspace_directories = workspace_directories;
165+
cfg.conda_executable = configure_options.conda_executable;
166+
cfg.environment_directories = environment_directories;
135167
cfg.pipenv_executable = configure_options.pipenv_executable;
136168
cfg.poetry_executable = configure_options.poetry_executable;
137169
// We will not support changing the cache directories once set.
@@ -146,6 +178,7 @@ pub fn handle_configure(context: Arc<Context>, id: u32, params: Value) {
146178
for locator in context.locators.iter() {
147179
locator.configure(&config);
148180
}
181+
info!("Configure completed in {:?}", now.elapsed());
149182
send_reply(id, None::<()>);
150183
});
151184
}
@@ -390,6 +423,8 @@ pub fn handle_find(context: Arc<Context>, id: u32, params: Value) {
390423
thread::spawn(
391424
move || match serde_json::from_value::<FindOptions>(params.clone()) {
392425
Ok(find_options) => {
426+
let now = Instant::now();
427+
trace!("Finding environments in {:?}", find_options.search_path);
393428
let global_env_search_paths: Vec<PathBuf> =
394429
get_search_paths_from_env_variables(context.os_environment.as_ref());
395430

@@ -424,6 +459,12 @@ pub fn handle_find(context: Arc<Context>, id: u32, params: Value) {
424459
.lock()
425460
.expect("environments mutex poisoned")
426461
.clone();
462+
trace!(
463+
"Find completed in {:?}, found {} environments in {:?}",
464+
now.elapsed(),
465+
envs.len(),
466+
find_options.search_path
467+
);
427468
if envs.is_empty() {
428469
send_reply(id, None::<Vec<PythonEnvironment>>);
429470
} else {
@@ -444,6 +485,7 @@ pub fn handle_find(context: Arc<Context>, id: u32, params: Value) {
444485

445486
pub fn handle_conda_telemetry(context: Arc<Context>, id: u32, _params: Value) {
446487
thread::spawn(move || {
488+
trace!("Gathering conda telemetry");
447489
let conda_locator = context.conda_locator.clone();
448490
let conda_executable = context
449491
.configuration
@@ -452,6 +494,7 @@ pub fn handle_conda_telemetry(context: Arc<Context>, id: u32, _params: Value) {
452494
.conda_executable
453495
.clone();
454496
let info = conda_locator.get_info_for_telemetry(conda_executable);
497+
trace!("Conda telemetry complete");
455498
send_reply(id, info.into());
456499
});
457500
}

0 commit comments

Comments
 (0)