Enhance logger service #171174
Merged
Enhance logger service #171174
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
sandy081
commented
Jan 12, 2023
sandy081
commented
- ILoggerService centrailised place for all loggers
- Provide options to display logger while creating/registering logger
- Listen to logger service and register log output channels accordingly
- Adopt this at all places where loggers are created
- Provide options to display logger while creating/registering logger - Listen to logger service and register log output channels accordingly - Adopt this at all places where loggers are created
sandy081
commented
Jan 12, 2023
| const isVisible = () => supportsTelemetry(productService, environmentService) && logService.getLevel() === LogLevel.Trace; | ||
| this.logger = this._register(loggerService.createLogger(telemetryLogResource, { id: telemetryLogChannelId, name: localize('telemetryLog', "Telemetry{0}", logSuffix), hidden: !isVisible() })); | ||
| this._register(logService.onDidChangeLogLevel(() => loggerService.setVisibility(telemetryLogResource, isVisible()))); | ||
|
|
There was a problem hiding this comment.
CC @lramos15 - You do not need to create output channel now. You can include all necessary info while creating the logger and system will create it whenever you want. For eg., here you can change the visibility of the logger and it will allow system to create/remove the channel.
sandy081
commented
Jan 12, 2023
| @@ -17,7 +18,7 @@ export class EditSessionsLogService extends AbstractLogger implements IEditSessi | |||
| @IEnvironmentService environmentService: IEnvironmentService | |||
| ) { | |||
| super(); | |||
| this.logger = this._register(loggerService.createLogger(environmentService.editSessionsLogResource, { name: 'editsessions' })); | |||
| this.logger = this._register(loggerService.createLogger(environmentService.editSessionsLogResource, { id: editSessionsLogId, name: localize('cloudChangesLog', "Cloud Changes") })); | |||
There was a problem hiding this comment.
@joyceerhl you do not need to create output channel now. You can include all necessary info while creating the logger and system will create it automatically.
sandy081
commented
Jan 12, 2023
| @@ -174,8 +173,7 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable { | |||
| super(themeService); | |||
|
|
|||
| const logsPath = joinPath(environmentService.windowLogsPath, 'notebook.rendering.log'); | |||
| this._renderLogger = this._register(loggerService.createLogger(logsPath, { name: 'notebook.rendering' })); | |||
| registerLogChannel(logChannelId, nls.localize('renderChannelName', "Notebook rendering"), logsPath, fileService, logService); | |||
| this._renderLogger = this._register(loggerService.createLogger(logsPath, { id: logChannelId, name: nls.localize('renderChannelName', "Notebook rendering") })); | |||
sandy081
commented
Jan 12, 2023
| private _registerLogChannel(id: string, label: string, file: URI): void { | ||
| const promise = registerLogChannel(id, label, file, this._fileService, this._logService); | ||
| this._register(toDisposable(() => promise.cancel())); | ||
| } | ||
| } |
There was a problem hiding this comment.
@Tyriar you do not need to create output channel now. You can include all necessary info while creating or registering the logger and system will create it automatically.
sandy081
commented
Jan 12, 2023
| @@ -187,13 +187,13 @@ export class PtyHostService extends Disposable implements IPtyService { | |||
| const logChannel = client.getChannel(TerminalIpcChannels.Log); | |||
| LogLevelChannelClient.setLevel(logChannel, this._logService.getLevel()); | |||
| const ptyHostLogResource = URI.file(join(this._environmentService.logsPath, `${TerminalLogConstants.FileName}.log`)); | |||
| this._loggerService.registerLoggerResource({ id: 'ptyHostLog', name: localize('ptyHost', "Pty Host"), resource: ptyHostLogResource }); | |||
| this._loggerService.registerLogger({ id: 'ptyHostLog', name: localize('ptyHost', "Pty Host"), resource: ptyHostLogResource }); | |||
There was a problem hiding this comment.
@Tyriar In your case you have to register the logger with the main logger service because your logger is created in a different process. Registering shall be sufficient for the system to create output channel for your logger.
aeschli
approved these changes
Jan 13, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.