Skip to content

Conversation

@Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Apr 25, 2025

Problem

We don't pass log level to the LSP, meaning it always defaults to info. This results in debug logs never showing up in the client side logs.

These settings are also missing from the UI.

Solution

  • add a setting amazonQ.lsp.logLevel to allow this to be configured. The allowed options are listed here
  • rename the trace server setting to amazonQ.lsp.traceChannel.
  • This means there are two related but different settings:
    • ...lsp.traceChannel sends language server logs to a separate channel (and includes the lsp req/rsp tracing).
    • ...lsp.logLevel determines which level these should be logged at.
  • expose these settings to the user in the UI.
  • sync logLevel setting with the lsp based on instructions here. AFAIK this can't currently be done with the outputChannel setting without some runtimes changes.

related: #7114

Notes

  • While testing this, I noticed that debug logs from the LSP show up as info. I don't see this as blocking, but worth tracking. My assumption is this happens because our log levels and Flare's don't perfectly match-up.
  • Kind of a nit, but we use the lsp prefix to describe the language server, but this isn't technically correct since lsp is the protocol. I kept my changes consistent with this, but in the future we may want to change this to avoid potentially confusing people.
  • The settings sync logic is non-trivial (send a notification, flare then sends a request for the config section which we provide) and it'd be great to have tests for this, but would require some decent refactoring. Tracking this as a follow-up.

Settings UI

image
  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@Hweinstock Hweinstock changed the title feat: add setting to configure lsp log level (WIP) feat(amazonq): add setting to configure lsp log level (WIP) Apr 25, 2025
@Hweinstock Hweinstock closed this Apr 25, 2025
@Hweinstock Hweinstock reopened this Apr 25, 2025
@Hweinstock Hweinstock changed the title feat(amazonq): add setting to configure lsp log level (WIP) feat(amazonq): add setting to configure lsp log level Apr 26, 2025
@Hweinstock
Copy link
Contributor Author

failing test tracked in #7183

@Hweinstock Hweinstock marked this pull request as ready for review April 28, 2025 13:00
@Hweinstock Hweinstock requested a review from a team as a code owner April 28, 2025 13:00
"type": "string"
}
},
"amazonQ.lsp.traceChannel": {
Copy link
Contributor

@nkomonen-amazon nkomonen-amazon Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be explained somewhere in the documentation? Also I don't see traceChannel being used specifically in the code anywhere

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't amazonq.lsp.trace defined here? Does it not need to be, or we want to hide from users?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed amazonq.lsp.trace to amazonQ.lsp.traceChannel however didn't update the reference in the docs. Good catch, should be updated now.

docs/lsp.md Outdated
4. Uncomment the `__AMAZONQLSP_PATH` and `__AMAZONQLSP_UI` variables in the `amazonq/.vscode/launch.json` extension configuration
5. Use the `Launch LSP with Debugging` configuration and set breakpoints in VSCode or the language server
6. (Optional): Enable `"amazonq.trace.server": "on"` or `"amazonq.trace.server": "verbose"` in your VSCode settings to view detailed log messages sent to/from the language server. These log messages will show up in the "Amazon Q Language Server" output channel
6. (Optional): Enable `"amazonq.lsp.trace": "on"` in your VSCode settings to view detailed log messages sent to/from the language server. These log messages will show up in the "Amazon Q Language Server" output channel. `"amazonq.lsp.logLevel"` can be used to configure the log level for the language server.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be helpful to create a dedicated section to enabling logs here instead of as a single bullet point. Then you can link to it. It will also be useful to link to it from the CONTRIBUTING.md loggin section (search ### Logging) as a subsection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these settings are exposed to the user through the UI, I was treating those descriptions as the documentation. However, I agree that this is useful information to have in CONTRIBUTING.md so I've duplicated it there as where.

@Hweinstock
Copy link
Contributor Author

Going to look into mapping the VSC log level to the Flare one instead.

@Hweinstock Hweinstock closed this Apr 28, 2025
Hweinstock added a commit that referenced this pull request Apr 28, 2025
## Problem
Same problem as #7172

## Solution
- listen to logLevel change events, and forward them to LSP. 
- pass initial logLevel to the LSP as well. 
- This requires mapping the local levels to the Flare Levels. This is
done with the following:
```
export const lspLogLevelMapping: Map<vscode.LogLevel, LspLogLevel> = new Map([
    [vscode.LogLevel.Error, 'error'],
    [vscode.LogLevel.Warning, 'warn'],
    [vscode.LogLevel.Info, 'info'],
    [vscode.LogLevel.Debug, 'log'],
    [vscode.LogLevel.Trace, 'debug'],
    [vscode.LogLevel.Off, 'error'], // TODO: once the language server supports a no-log setting, we can map to that.
])
```
## Notes
- Because of this mapping, it means that to enabled 'debug' logs in
Flare, we set to 'trace' in VSC which can be confusing for contributors.
---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants