Skip to content

Conversation

@Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented 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.
  • 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.

export type LogLevel = 'error' | 'warn' | 'info' | 'verbose' | 'debug'

export function fromVscodeLogLevel(logLevel: vscode.LogLevel): LogLevel {
if (!vscode.LogLevel) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

minimum version has been bumped since this was written

* Log messages. Use `outputChannel` for application messages.
*/
logOutputChannel: OutputChannel
logOutputChannel: LogOutputChannel
Copy link
Contributor Author

Choose a reason for hiding this comment

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

LogOutputChannel extends OutputChannel and includes the ability to add listeners.

@Hweinstock Hweinstock changed the title feat: sync lsp logging with local configuration. feat(amazonq): sync lsp logging with local configuration. Apr 28, 2025
@Hweinstock Hweinstock closed this Apr 28, 2025
@Hweinstock Hweinstock reopened this Apr 28, 2025
shareCodeWhispererContentWithAWS: !CodeWhispererSettings.instance.isOptoutEnabled(),
},
]
case 'aws.logLevel':
Copy link
Contributor

@justinmk3 justinmk3 Apr 28, 2025

Choose a reason for hiding this comment

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

is this setting being added?

edit: oh, this is something sent to Q LSP, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not being added for us no. This is the section used to detect logging level changes for this notification based on their implementation here.

@Hweinstock Hweinstock marked this pull request as ready for review April 28, 2025 19:46
@Hweinstock Hweinstock requested a review from a team as a code owner April 28, 2025 19:46
// https://github.com/aws/language-server-runtimes/blob/eae85672c345d8adaf4c8cbd741260b8a59750c4/runtimes/runtimes/util/loggingUtil.ts#L4-L10
const validLspLogLevels = ['error', 'warn', 'info', 'log', 'debug'] as const
export type LspLogLevel = (typeof validLspLogLevels)[number]
export const lspLogLevelMapping: Map<vscode.LogLevel, LspLogLevel> = new Map([
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be exported?

Suggested change
export const lspLogLevelMapping: Map<vscode.LogLevel, LspLogLevel> = new Map([
const lspLogLevelMapping: Map<vscode.LogLevel, LspLogLevel> = new Map([

}
}

export function toAmazonQLSPLogLevel(logLevel: vscode.LogLevel): LspLogLevel {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets explain why briefly: "Maps the VS Code log level to an equivalent Amazon Q language server log level, since they are not 1:1"

@Hweinstock
Copy link
Contributor Author

failing test tracked here: #7187

@Hweinstock Hweinstock merged commit db673c9 into aws:feature/hybridChat Apr 28, 2025
35 of 37 checks passed
@Hweinstock Hweinstock deleted the loggingSync branch April 28, 2025 22:11
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