Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use $/logTrace for server trace logs in Zed and VS Code #12564

Merged
merged 4 commits into from
Jul 30, 2024

Conversation

osiewicz
Copy link
Contributor

Summary

This pull request adds support for logging via $/logTrace RPC messages. It also enables that code path for when a client is Zed editor (as there's no way for us to generically tell whether a client prefers $/logTrace over stderr.

Related to: #12523
/cc @dhruvmanila

Test Plan

I've built Ruff from this branch and tested it manually with Zed.

Copy link
Contributor

github-actions bot commented Jul 29, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser requested a review from dhruvmanila July 29, 2024 17:15
crates/ruff_server/src/trace.rs Outdated Show resolved Hide resolved
crates/ruff_server/src/trace.rs Outdated Show resolved Hide resolved
@dhruvmanila dhruvmanila added the server Related to the LSP server label Jul 30, 2024
@dhruvmanila
Copy link
Member

I pushed both the changes and verified in VS Code:

2024-07-30 08:12:09.990 [info] [Trace - 8:12:09 AM]    0.021675875s  INFO ruff:main ruff_server::server: Configuration file watcher successfully registered

@dhruvmanila dhruvmanila changed the title ruff_server: Add support for LogTrace output Use $/logTrace for server trace logs in Zed and VS Code Jul 30, 2024
@dhruvmanila dhruvmanila merged commit fb9f566 into astral-sh:main Jul 30, 2024
20 checks passed
dhruvmanila added a commit that referenced this pull request Jan 8, 2025
## Summary

Refer to the VS Code PR
(astral-sh/ruff-vscode#659) for details on the
change.

This PR changes the following:

1. Add tracing span for both request (request id and method name) and
notification (method name) handler
2. Remove the `RUFF_TRACE` environment variable. This was being used to
turn on / off logging for the server
3. Similarly, remove reading the `trace` value from the initialization
options
4. Remove handling the `$/setTrace` notification
5. Remove the specialized `TraceLogWriter` used for Zed and VS Code
(#12564)

Regarding the (5) for the Zed editor, the reason that was implemented
was because there was no way of looking at the stderr messages in the
editor which has been changed. Now, it captures the stderr as part of
the "Server Logs".
(https://github.com/zed-industries/zed/blob/82492d74a8d0350cba66671c27e282a928fd4c85/crates/language_tools/src/lsp_log.rs#L548-L552)

### Question

Regarding (1), I think having just a simple trace level message should
be good for now as the spans are not hierarchical. This could be tackled
with #12744. The difference between the two:

<details><summary>Using <code>tracing::trace</code></summary>
<p>

```
   0.019243416s DEBUG ThreadId(08) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/playground/ruff/.vscode
   0.026398750s  INFO main ruff_server::session::index: Registering workspace: /Users/dhruv/playground/ruff
   0.026802125s TRACE ruff:main ruff_server::server::api: Received notification "textDocument/didOpen"
   0.026930666s TRACE ruff:main ruff_server::server::api: Received notification "textDocument/didOpen"
   0.026962333s TRACE ruff:main ruff_server::server::api: Received request "textDocument/diagnostic" (1)
   0.027042875s TRACE ruff:main ruff_server::server::api: Received request "textDocument/diagnostic" (2)
   0.027097500s TRACE ruff:main ruff_server::server::api: Received request "textDocument/codeAction" (3)
   0.027107458s DEBUG ruff:worker:0 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
   0.027123541s DEBUG ruff:worker:3 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/organize_imports.py
   0.027514875s  INFO     ruff:main ruff_server::server: Configuration file watcher successfully registered
   0.285689833s TRACE     ruff:main ruff_server::server::api: Received request "textDocument/codeAction" (4)
  45.741101666s TRACE     ruff:main ruff_server::server::api: Received notification "textDocument/didClose"
  47.108745500s TRACE     ruff:main ruff_server::server::api: Received notification "textDocument/didOpen"
  47.109802041s TRACE     ruff:main ruff_server::server::api: Received request "textDocument/diagnostic" (5)
  47.109926958s TRACE     ruff:main ruff_server::server::api: Received request "textDocument/codeAction" (6)
  47.110027791s DEBUG ruff:worker:6 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
  51.863679125s TRACE     ruff:main ruff_server::server::api: Received request "textDocument/hover" (7)
```

</p>
</details> 

<details><summary>Using <code>tracing::trace_span</code></summary>
<p>

Only logging the enter event:

```
   0.018638750s DEBUG ThreadId(11) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/playground/ruff/.vscode
   0.025895791s  INFO main ruff_server::session::index: Registering workspace: /Users/dhruv/playground/ruff
   0.026378791s TRACE ruff:main notification{method="textDocument/didOpen"}: ruff_server::server::api: enter
   0.026531208s TRACE ruff:main notification{method="textDocument/didOpen"}: ruff_server::server::api: enter
   0.026567583s TRACE ruff:main request{id=1 method="textDocument/diagnostic"}: ruff_server::server::api: enter
   0.026652541s TRACE ruff:main request{id=2 method="textDocument/diagnostic"}: ruff_server::server::api: enter
   0.026711041s DEBUG ruff:worker:2 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/organize_imports.py
   0.026729166s DEBUG ruff:worker:1 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
   0.027023083s  INFO     ruff:main ruff_server::server: Configuration file watcher successfully registered
   5.197554750s TRACE     ruff:main notification{method="textDocument/didClose"}: ruff_server::server::api: enter
   6.534458000s TRACE     ruff:main notification{method="textDocument/didOpen"}: ruff_server::server::api: enter
   6.535027958s TRACE     ruff:main request{id=3 method="textDocument/diagnostic"}: ruff_server::server::api: enter
   6.535271166s DEBUG ruff:worker:3 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/organize_imports.py
   6.544240583s TRACE     ruff:main request{id=4 method="textDocument/codeAction"}: ruff_server::server::api: enter
   7.049692458s TRACE     ruff:main request{id=5 method="textDocument/codeAction"}: ruff_server::server::api: enter
   7.508142541s TRACE     ruff:main request{id=6 method="textDocument/hover"}: ruff_server::server::api: enter
   7.872421958s TRACE     ruff:main request{id=7 method="textDocument/hover"}: ruff_server::server::api: enter
   8.024498583s TRACE     ruff:main request{id=8 method="textDocument/codeAction"}: ruff_server::server::api: enter
  13.895063666s TRACE     ruff:main request{id=9 method="textDocument/codeAction"}: ruff_server::server::api: enter
  14.774706083s TRACE     ruff:main request{id=10 method="textDocument/hover"}: ruff_server::server::api: enter
  16.058918958s TRACE     ruff:main notification{method="textDocument/didChange"}: ruff_server::server::api: enter
  16.060562208s TRACE     ruff:main request{id=11 method="textDocument/diagnostic"}: ruff_server::server::api: enter
  16.061109083s DEBUG ruff:worker:8 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
  21.561742875s TRACE     ruff:main notification{method="textDocument/didChange"}: ruff_server::server::api: enter
  21.563573791s TRACE     ruff:main request{id=12 method="textDocument/diagnostic"}: ruff_server::server::api: enter
  21.564206750s DEBUG ruff:worker:4 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
  21.826691375s TRACE     ruff:main request{id=13 method="textDocument/codeAction"}: ruff_server::server::api: enter
  22.091080125s TRACE     ruff:main request{id=14 method="textDocument/codeAction"}: ruff_server::server::api: enter
```

</p>
</details> 


**Todo**

- [x] Update documentation (I'll be adding a troubleshooting section
under "Editors" as a follow-up which is for all editors)
- [x] Check for backwards compatibility. I don't think this should break
backwards compatibility as it's mainly targeted towards improving the
debugging experience.

~**Before I go on to updating the documentation, I'd appreciate initial
review on the chosen approach.**~

resolves: #14959 

## Test Plan

Refer to the test plan in
astral-sh/ruff-vscode#659.

Example logs at `debug` level:

```
   0.010770083s DEBUG ThreadId(15) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/playground/ruff/.vscode
   0.018101916s  INFO main ruff_server::session::index: Registering workspace: /Users/dhruv/playground/ruff
   0.018559916s DEBUG ruff:worker:4 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
   0.018992375s  INFO     ruff:main ruff_server::server: Configuration file watcher successfully registered
  23.408802375s DEBUG ruff:worker:11 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
  24.329127416s DEBUG  ruff:worker:6 ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
```

Example logs at `trace` level:

```
   0.010296375s DEBUG ThreadId(13) ruff_server::session::index::ruff_settings: Ignored path via `exclude`: /Users/dhruv/playground/ruff/.vscode
   0.017422583s  INFO main ruff_server::session::index: Registering workspace: /Users/dhruv/playground/ruff
   0.018034458s TRACE ruff:main notification{method="textDocument/didOpen"}: ruff_server::server::api: enter
   0.018199708s TRACE ruff:worker:0 request{id=1 method="textDocument/diagnostic"}: ruff_server::server::api: enter
   0.018251167s DEBUG ruff:worker:0 request{id=1 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
   0.018528708s  INFO     ruff:main ruff_server::server: Configuration file watcher successfully registered
   1.611798417s TRACE ruff:worker:1 request{id=2 method="textDocument/codeAction"}: ruff_server::server::api: enter
   1.861757542s TRACE ruff:worker:4 request{id=3 method="textDocument/codeAction"}: ruff_server::server::api: enter
   7.027361792s TRACE ruff:worker:2 request{id=4 method="textDocument/codeAction"}: ruff_server::server::api: enter
   7.851361500s TRACE ruff:worker:5 request{id=5 method="textDocument/codeAction"}: ruff_server::server::api: enter
   7.901690875s TRACE     ruff:main notification{method="textDocument/didChange"}: ruff_server::server::api: enter
   7.903063167s TRACE ruff:worker:10 request{id=6 method="textDocument/diagnostic"}: ruff_server::server::api: enter
   7.903183500s DEBUG ruff:worker:10 request{id=6 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
   8.702385292s TRACE      ruff:main notification{method="textDocument/didChange"}: ruff_server::server::api: enter
   8.704106625s TRACE  ruff:worker:3 request{id=7 method="textDocument/diagnostic"}: ruff_server::server::api: enter
   8.704304875s DEBUG  ruff:worker:3 request{id=7 method="textDocument/diagnostic"}: ruff_server::resolve: Included path via `include`: /Users/dhruv/playground/ruff/lsp/play.py
   8.966853458s TRACE  ruff:worker:9 request{id=8 method="textDocument/codeAction"}: ruff_server::server::api: enter
   9.229622792s TRACE  ruff:worker:6 request{id=9 method="textDocument/codeAction"}: ruff_server::server::api: enter
  10.513111583s TRACE  ruff:worker:7 request{id=10 method="textDocument/codeAction"}: ruff_server::server::api: enter
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Related to the LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants