Do not send any LSP logs by default to collab clients (#37163)

Kirill Bulatov created

Follow-up https://github.com/zed-industries/zed/pull/37083

Noisy RPC LSP logs were functioning this way already, but to keep Collab
loaded even less, do not send any kind of logs to the client if the
client has a corresponding log tab not opened.

This change is pretty raw and does not fully cover scenarious with
multiple clients: if one client has a log tab open and another opens tab
with another kind of log, the 2nd kind of logs will be streamed only.
Also, it should be possible to forward the host logs to the client on
enabling — that is not done to keep the change smaller.

Release Notes:

- N/A

Change summary

crates/language_tools/src/language_tools.rs     |  2 
crates/language_tools/src/lsp_log_view.rs       | 96 ++++++++++++++----
crates/language_tools/src/lsp_log_view_tests.rs |  2 
crates/project/src/lsp_store/log_store.rs       | 66 ++++++++----
crates/project/src/project.rs                   | 12 ++
crates/remote_server/src/headless_project.rs    |  6 
6 files changed, 135 insertions(+), 49 deletions(-)

Detailed changes

crates/language_tools/src/language_tools.rs 🔗

@@ -14,7 +14,7 @@ use ui::{Context, Window};
 use workspace::{Item, ItemHandle, SplitDirection, Workspace};
 
 pub fn init(cx: &mut App) {
-    lsp_log_view::init(true, cx);
+    lsp_log_view::init(false, cx);
     syntax_tree_view::init(cx);
     key_context_view::init(cx);
 }

crates/language_tools/src/lsp_log_view.rs 🔗

@@ -16,6 +16,7 @@ use project::{
     lsp_store::log_store::{self, Event, LanguageServerKind, LogKind, LogStore, Message},
     search::SearchQuery,
 };
+use proto::toggle_lsp_logs::LogType;
 use std::{any::TypeId, borrow::Cow, sync::Arc};
 use ui::{Button, Checkbox, ContextMenu, Label, PopoverMenu, ToggleState, prelude::*};
 use util::ResultExt as _;
@@ -111,8 +112,8 @@ actions!(
     ]
 );
 
-pub fn init(store_logs: bool, cx: &mut App) {
-    let log_store = log_store::init(store_logs, cx);
+pub fn init(on_headless_host: bool, cx: &mut App) {
+    let log_store = log_store::init(on_headless_host, cx);
 
     log_store.update(cx, |_, cx| {
         Copilot::global(cx).map(|copilot| {
@@ -266,6 +267,19 @@ impl LspLogView {
             window.focus(&log_view.editor.focus_handle(cx));
         });
 
+        cx.on_release(|log_view, cx| {
+            log_view.log_store.update(cx, |log_store, cx| {
+                for (server_id, state) in &log_store.language_servers {
+                    if let Some(log_kind) = state.toggled_log_kind {
+                        if let Some(log_type) = log_type(log_kind) {
+                            send_toggle_log_message(state, *server_id, false, log_type, cx);
+                        }
+                    }
+                }
+            });
+        })
+        .detach();
+
         let mut lsp_log_view = Self {
             focus_handle,
             editor,
@@ -436,6 +450,12 @@ impl LspLogView {
             cx.notify();
         }
         self.editor.read(cx).focus_handle(cx).focus(window);
+        self.log_store.update(cx, |log_store, cx| {
+            let state = log_store.get_language_server_state(server_id)?;
+            state.toggled_log_kind = Some(LogKind::Logs);
+            send_toggle_log_message(state, server_id, true, LogType::Log, cx);
+            Some(())
+        });
     }
 
     fn update_log_level(
@@ -472,8 +492,8 @@ impl LspLogView {
     ) {
         let trace_level = self
             .log_store
-            .update(cx, |this, _| {
-                Some(this.get_language_server_state(server_id)?.trace_level)
+            .update(cx, |log_store, _| {
+                Some(log_store.get_language_server_state(server_id)?.trace_level)
             })
             .unwrap_or(TraceValue::Messages);
         let log_contents = self
@@ -487,6 +507,12 @@ impl LspLogView {
             let (editor, editor_subscriptions) = Self::editor_for_logs(log_contents, window, cx);
             self.editor = editor;
             self.editor_subscriptions = editor_subscriptions;
+            self.log_store.update(cx, |log_store, cx| {
+                let state = log_store.get_language_server_state(server_id)?;
+                state.toggled_log_kind = Some(LogKind::Trace);
+                send_toggle_log_message(state, server_id, true, LogType::Trace, cx);
+                Some(())
+            });
             cx.notify();
         }
         self.editor.read(cx).focus_handle(cx).focus(window);
@@ -551,24 +577,7 @@ impl LspLogView {
             }
 
             if let Some(server_state) = log_store.language_servers.get(&server_id) {
-                if let LanguageServerKind::Remote { project } = &server_state.kind {
-                    project
-                        .update(cx, |project, cx| {
-                            if let Some((client, project_id)) =
-                                project.lsp_store().read(cx).upstream_client()
-                            {
-                                client
-                                    .send(proto::ToggleLspLogs {
-                                        project_id,
-                                        log_type: proto::toggle_lsp_logs::LogType::Rpc as i32,
-                                        server_id: server_id.to_proto(),
-                                        enabled,
-                                    })
-                                    .log_err();
-                            }
-                        })
-                        .ok();
-                }
+                send_toggle_log_message(server_state, server_id, enabled, LogType::Rpc, cx);
             };
         });
         if !enabled && Some(server_id) == self.current_server_id {
@@ -644,6 +653,49 @@ impl LspLogView {
         self.editor_subscriptions = editor_subscriptions;
         cx.notify();
         self.editor.read(cx).focus_handle(cx).focus(window);
+        self.log_store.update(cx, |log_store, cx| {
+            let state = log_store.get_language_server_state(server_id)?;
+            if let Some(log_kind) = state.toggled_log_kind.take() {
+                if let Some(log_type) = log_type(log_kind) {
+                    send_toggle_log_message(state, server_id, false, log_type, cx);
+                }
+            };
+            Some(())
+        });
+    }
+}
+
+fn log_type(log_kind: LogKind) -> Option<LogType> {
+    match log_kind {
+        LogKind::Rpc => Some(LogType::Rpc),
+        LogKind::Trace => Some(LogType::Trace),
+        LogKind::Logs => Some(LogType::Log),
+        LogKind::ServerInfo => None,
+    }
+}
+
+fn send_toggle_log_message(
+    server_state: &log_store::LanguageServerState,
+    server_id: LanguageServerId,
+    enabled: bool,
+    log_type: LogType,
+    cx: &mut App,
+) {
+    if let LanguageServerKind::Remote { project } = &server_state.kind {
+        project
+            .update(cx, |project, cx| {
+                if let Some((client, project_id)) = project.lsp_store().read(cx).upstream_client() {
+                    client
+                        .send(proto::ToggleLspLogs {
+                            project_id,
+                            log_type: log_type as i32,
+                            server_id: server_id.to_proto(),
+                            enabled,
+                        })
+                        .log_err();
+                }
+            })
+            .ok();
     }
 }
 

crates/language_tools/src/lsp_log_view_tests.rs 🔗

@@ -53,7 +53,7 @@ async fn test_lsp_log_view(cx: &mut TestAppContext) {
         },
     );
 
-    let log_store = cx.new(|cx| LogStore::new(true, cx));
+    let log_store = cx.new(|cx| LogStore::new(false, cx));
     log_store.update(cx, |store, cx| store.add_project(&project, cx));
 
     let _rust_buffer = project

crates/project/src/lsp_store/log_store.rs 🔗

@@ -21,8 +21,8 @@ const SERVER_LOGS: &str = "Server Logs";
 const SERVER_TRACE: &str = "Server Trace";
 const SERVER_INFO: &str = "Server Info";
 
-pub fn init(store_logs: bool, cx: &mut App) -> Entity<LogStore> {
-    let log_store = cx.new(|cx| LogStore::new(store_logs, cx));
+pub fn init(on_headless_host: bool, cx: &mut App) -> Entity<LogStore> {
+    let log_store = cx.new(|cx| LogStore::new(on_headless_host, cx));
     cx.set_global(GlobalLogStore(log_store.clone()));
     log_store
 }
@@ -43,7 +43,7 @@ pub enum Event {
 impl EventEmitter<Event> for LogStore {}
 
 pub struct LogStore {
-    store_logs: bool,
+    on_headless_host: bool,
     projects: HashMap<WeakEntity<Project>, ProjectState>,
     pub copilot_log_subscription: Option<lsp::Subscription>,
     pub language_servers: HashMap<LanguageServerId, LanguageServerState>,
@@ -138,6 +138,7 @@ pub struct LanguageServerState {
     pub trace_level: TraceValue,
     pub log_level: MessageType,
     io_logs_subscription: Option<lsp::Subscription>,
+    pub toggled_log_kind: Option<LogKind>,
 }
 
 impl std::fmt::Debug for LanguageServerState {
@@ -151,6 +152,7 @@ impl std::fmt::Debug for LanguageServerState {
             .field("rpc_state", &self.rpc_state)
             .field("trace_level", &self.trace_level)
             .field("log_level", &self.log_level)
+            .field("toggled_log_kind", &self.toggled_log_kind)
             .finish_non_exhaustive()
     }
 }
@@ -226,14 +228,14 @@ impl LogKind {
 }
 
 impl LogStore {
-    pub fn new(store_logs: bool, cx: &mut Context<Self>) -> Self {
+    pub fn new(on_headless_host: bool, cx: &mut Context<Self>) -> Self {
         let (io_tx, mut io_rx) = mpsc::unbounded();
 
         let log_store = Self {
             projects: HashMap::default(),
             language_servers: HashMap::default(),
             copilot_log_subscription: None,
-            store_logs,
+            on_headless_host,
             io_tx,
         };
         cx.spawn(async move |log_store, cx| {
@@ -351,12 +353,26 @@ impl LogStore {
                                     }
                                 }
                             }
-                            crate::Event::ToggleLspLogs { server_id, enabled } => {
-                                // we do not support any other log toggling yet
-                                if *enabled {
-                                    log_store.enable_rpc_trace_for_language_server(*server_id);
-                                } else {
-                                    log_store.disable_rpc_trace_for_language_server(*server_id);
+                            crate::Event::ToggleLspLogs {
+                                server_id,
+                                enabled,
+                                toggled_log_kind,
+                            } => {
+                                if let Some(server_state) =
+                                    log_store.get_language_server_state(*server_id)
+                                {
+                                    if *enabled {
+                                        server_state.toggled_log_kind = Some(*toggled_log_kind);
+                                    } else {
+                                        server_state.toggled_log_kind = None;
+                                    }
+                                }
+                                if LogKind::Rpc == *toggled_log_kind {
+                                    if *enabled {
+                                        log_store.enable_rpc_trace_for_language_server(*server_id);
+                                    } else {
+                                        log_store.disable_rpc_trace_for_language_server(*server_id);
+                                    }
                                 }
                             }
                             _ => {}
@@ -395,6 +411,7 @@ impl LogStore {
                 trace_level: TraceValue::Off,
                 log_level: MessageType::LOG,
                 io_logs_subscription: None,
+                toggled_log_kind: None,
             }
         });
 
@@ -425,7 +442,7 @@ impl LogStore {
         message: &str,
         cx: &mut Context<Self>,
     ) -> Option<()> {
-        let store_logs = self.store_logs;
+        let store_logs = !self.on_headless_host;
         let language_server_state = self.get_language_server_state(id)?;
 
         let log_lines = &mut language_server_state.log_messages;
@@ -464,7 +481,7 @@ impl LogStore {
         verbose_info: Option<String>,
         cx: &mut Context<Self>,
     ) -> Option<()> {
-        let store_logs = self.store_logs;
+        let store_logs = !self.on_headless_host;
         let language_server_state = self.get_language_server_state(id)?;
 
         let log_lines = &mut language_server_state.trace_messages;
@@ -530,7 +547,7 @@ impl LogStore {
         message: &str,
         cx: &mut Context<'_, Self>,
     ) {
-        let store_logs = self.store_logs;
+        let store_logs = !self.on_headless_host;
         let Some(state) = self
             .get_language_server_state(language_server_id)
             .and_then(|state| state.rpc_state.as_mut())
@@ -673,6 +690,7 @@ impl LogStore {
     }
 
     fn emit_event(&mut self, e: Event, cx: &mut Context<Self>) {
+        let on_headless_host = self.on_headless_host;
         match &e {
             Event::NewServerLogEntry { id, kind, text } => {
                 if let Some(state) = self.get_language_server_state(*id) {
@@ -686,14 +704,18 @@ impl LogStore {
                     }
                     .and_then(|lsp_store| lsp_store.read(cx).downstream_client());
                     if let Some((client, project_id)) = downstream_client {
-                        client
-                            .send(proto::LanguageServerLog {
-                                project_id,
-                                language_server_id: id.to_proto(),
-                                message: text.clone(),
-                                log_type: Some(kind.to_proto()),
-                            })
-                            .ok();
+                        if on_headless_host
+                            || Some(LogKind::from_server_log_type(kind)) == state.toggled_log_kind
+                        {
+                            client
+                                .send(proto::LanguageServerLog {
+                                    project_id,
+                                    language_server_id: id.to_proto(),
+                                    message: text.clone(),
+                                    log_type: Some(kind.to_proto()),
+                                })
+                                .ok();
+                        }
                     }
                 }
             }

crates/project/src/project.rs 🔗

@@ -33,7 +33,7 @@ mod yarn;
 
 use dap::inline_value::{InlineValueLocation, VariableLookupKind, VariableScope};
 
-use crate::git_store::GitStore;
+use crate::{git_store::GitStore, lsp_store::log_store::LogKind};
 pub use git_store::{
     ConflictRegion, ConflictSet, ConflictSetSnapshot, ConflictSetUpdate,
     git_traversal::{ChildEntriesGitIter, GitEntry, GitEntryRef, GitTraversal},
@@ -285,6 +285,7 @@ pub enum Event {
     ToggleLspLogs {
         server_id: LanguageServerId,
         enabled: bool,
+        toggled_log_kind: LogKind,
     },
     Toast {
         notification_id: SharedString,
@@ -4719,10 +4720,19 @@ impl Project {
         envelope: TypedEnvelope<proto::ToggleLspLogs>,
         mut cx: AsyncApp,
     ) -> Result<()> {
+        let toggled_log_kind =
+            match proto::toggle_lsp_logs::LogType::from_i32(envelope.payload.log_type)
+                .context("invalid log type")?
+            {
+                proto::toggle_lsp_logs::LogType::Log => LogKind::Logs,
+                proto::toggle_lsp_logs::LogType::Trace => LogKind::Trace,
+                proto::toggle_lsp_logs::LogType::Rpc => LogKind::Rpc,
+            };
         project.update(&mut cx, |_, cx| {
             cx.emit(Event::ToggleLspLogs {
                 server_id: LanguageServerId::from_proto(envelope.payload.server_id),
                 enabled: envelope.payload.enabled,
+                toggled_log_kind,
             })
         })?;
         Ok(())

crates/remote_server/src/headless_project.rs 🔗

@@ -67,7 +67,7 @@ impl HeadlessProject {
         settings::init(cx);
         language::init(cx);
         project::Project::init_settings(cx);
-        log_store::init(false, cx);
+        log_store::init(true, cx);
     }
 
     pub fn new(
@@ -546,7 +546,9 @@ impl HeadlessProject {
             .context("lsp logs store is missing")?;
 
         lsp_logs.update(&mut cx, |lsp_logs, _| {
-            // we do not support any other log toggling yet
+            // RPC logs are very noisy and we need to toggle it on the headless server too.
+            // The rest of the logs for the ssh project are very important to have toggled always,
+            // to e.g. send language server error logs to the client before anything is toggled.
             if envelope.payload.enabled {
                 lsp_logs.enable_rpc_trace_for_language_server(server_id);
             } else {