Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improve logging system using separate output channels (#659)
## Summary Related: astral-sh/ruff#15232 The basic idea is that there will be two channels where the logs will go: 1. Ruff: Here, all the logs for the extension will go 2. Ruff Language Server: Here, all the server log messages will go and are captured from the stderr. It will also contain the trace communications between the client and the server when it's enabled. 3. Ruff Language Server Trace: Here, all the communication messages between the client and the server will go. **This channel is created lazily when the value of `ruff.trace.server` is changed to "verbose" or "messages" for the first time.** (2) is controlled by the `ruff.logLevel` configuration which defaults to `info`. (3) is controlled by the `ruff.trace.server` configuration which defaults to `off` and can be changed to `messages` and `verbose`. The difference between them is that the former will only log the method name for both the request and response while the latter will also log the request parameters and the response result. ### Red Knot The end goal is to merge the tracing system with Red knot. Currently, Red knot's tracing system utilizes the `-v`, `-vv` and `-vvv` flags from the command-line and `RED_KNOT_LOG` environment variable for configuration. For the red knot server specifically, we will need to provide an additional configuration parameter like `ruff.server.extraEnv` which allows user to fine tune the logging using the mentioned environment variable. The `ruff.logLevel` will by default pass `RED_KNOT_LOG=<logLevel>` to allow for easy configuration. ### Why not `window/logMessage` ? The `window/logMessage` request can be used by the server to notify the client to log a message at a certain log level. There are a few disadvantages of using this as oppose to just using stderr: * It needs to go through the RPC which requires serializing and de-serializing the messages * The output format depends on how client handles the message * The trace channel will get populated with "Received notification 'window/logMessage'" lines * There's no 'debug' and 'trace' message type the LSP protocol although the next version is going to include debug type Additionally, putting the message onto stderr also allows us to use a custom formatter which is what is being done in red knot and would prove to be beneficial when merging the tracing setup. <details><summary>For posterity, here's the patch to use <code>window/logMessage</code>:</summary> <p> ```diff diff --git a/crates/ruff_server/src/trace.rs b/crates/ruff_server/src/trace.rs index 7b367c780..ceedeff21 100644 --- a/crates/ruff_server/src/trace.rs +++ b/crates/ruff_server/src/trace.rs @@ -15,6 +15,9 @@ //! Tracing will write to `stderr` by default, which should appear in the logs for most LSP clients. //! A `logFile` path can also be specified in the settings, and output will be directed there instead. use core::str; +use lsp_server::{Message, Notification}; +use lsp_types::notification::{LogMessage, Notification as _}; +use lsp_types::{LogMessageParams, MessageType}; use serde::Deserialize; use std::{ path::PathBuf, @@ -22,6 +25,7 @@ use std::{ sync::{Arc, OnceLock}, }; use tracing::level_filters::LevelFilter; +use tracing_subscriber::fmt::MakeWriter; use tracing_subscriber::{ fmt::{time::Uptime, writer::BoxMakeWriter}, layer::SubscriberExt, @@ -73,7 +77,7 @@ pub(crate) fn init_tracing( let logger = match log_file { Some(file) => BoxMakeWriter::new(Arc::new(file)), - None => BoxMakeWriter::new(std::io::stderr), + None => BoxMakeWriter::new(LogMessageMakeWriter), }; let subscriber = tracing_subscriber::Registry::default().with( tracing_subscriber::fmt::layer() @@ -135,3 +139,61 @@ impl<S> tracing_subscriber::layer::Filter<S> for LogLevelFilter { Some(LevelFilter::from_level(self.filter.trace_level())) } } + +struct LogMessageMakeWriter; + +impl<'a> MakeWriter<'a> for LogMessageMakeWriter { + type Writer = LogMessageWriter; + + fn make_writer(&'a self) -> Self::Writer { + LogMessageWriter { + level: tracing::Level::INFO, + } + } + + fn make_writer_for(&'a self, meta: &tracing::Metadata<'_>) -> Self::Writer { + LogMessageWriter { + level: *meta.level(), + } + } +} + +struct LogMessageWriter { + level: tracing::Level, +} + +impl LogMessageWriter { + const fn message_type(&self) -> MessageType { + match self.level { + tracing::Level::ERROR => MessageType::ERROR, + tracing::Level::WARN => MessageType::WARNING, + tracing::Level::INFO => MessageType::INFO, + tracing::Level::DEBUG => MessageType::LOG, + tracing::Level::TRACE => MessageType::LOG, + } + } +} + +impl std::io::Write for LogMessageWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> { + let message = str::from_utf8(buf) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))? + .to_owned(); + LOGGING_SENDER + .get() + .expect("logging sender should be initialized at this point") + .send(Message::Notification(Notification { + method: LogMessage::METHOD.to_owned(), + params: serde_json::to_value(LogMessageParams { + typ: self.message_type(), + message, + })?, + })) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?; + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} ``` </p> </details> ## Test Plan Using the following **VS Code** settings: ```json { "ruff.nativeServer": "on", "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"], "ruff.trace.server": "verbose", "ruff.logLevel": "debug" } ``` The following video showcases the three channels in VS Code: https://github.com/user-attachments/assets/53396774-9dfb-4c33-9145-67eaf2395936 Using the following **Zed** settings: ```json { "lsp": { "ruff": { "binary": { "path": "/Users/dhruv/work/astral/ruff/target/debug/ruff", "arguments": [ "server" ] }, "initialization_options": { "settings": { "logLevel": "debug" } } } } } ``` The following video showcases that the stderr messages are captured in the "Server Logs" window and the RPC window provides the raw request and response messages: https://github.com/user-attachments/assets/ee36eb4a-133f-46d3-abb9-366c48793f32 For **Neovim**, there's an open issue (neovim/neovim#16807) to send stderr messages to a separate file but currently it gets logged as "ERROR" in the main LSP log file. There doesn't seem to be any way to hook into the stderr of the Neovim LSP client for us to provide a solution for Ruff users.
- Loading branch information