Skip to content

Centralize MSSQL logging and align log levels#22276

Merged
aasimkhan30 merged 12 commits into
mainfrom
aasim/fix/logCleanup
Jun 3, 2026
Merged

Centralize MSSQL logging and align log levels#22276
aasimkhan30 merged 12 commits into
mainfrom
aasim/fix/logCleanup

Conversation

@aasimkhan30
Copy link
Copy Markdown
Contributor

Description

This PR centralizes MSSQL logging contracts and cleans up logging usage across the extension host, webviews, services, utilities, and tests.

  • Added shared logger contracts in sharedInterfaces/logger so logger types and webview log events are defined in one place.
  • Added a webview-side ILogger implementation so webviews can use logger-style APIs like error, warn, debug, piiSanitized, and withPrefix.
  • Added getLogger(prefix?: string) for cleaner prefixed extension-host logging and migrated module-level loggers away from logger as baseLogger alias patterns.
  • Removed unnecessary logging redirects from models/logger, sharedInterfaces/webview, VscodeWrapper, utility helpers, and SqlToolsServiceClient.
  • Replaced application and webview console.* diagnostics with the shared logger where logging should remain, and removed noisy debug-only console output.
  • Recategorized log levels so failures use error, recoverable/unexpected states use warn, and diagnostic lifecycle details use debug.
  • Simplified unit tests by removing assertions that only verified logging side effects, keeping tests focused on behavior, return values, and error handling.

Validation:

  • npm run build -- --target mssql
  • npm run lint -- --target mssql
  • VS Code diagnostics: no errors

Not run:

  • npm run test

Code Changes Checklist

  • New or updated unit tests added
  • All existing tests pass (npm run test)
  • Code follows contributing guidelines
  • Telemetry/logging updated if relevant
  • No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

aasimkhan30 and others added 7 commits June 2, 2026 15:23
…interface for improved clarity and consistency
- Replaced console.error and console.warn calls with logger methods across various files for consistent error handling and logging.
- Updated localizationCache, mainController, webviewPanelController, chatAgentRequestHandler, fabricHelper, connectionInfo, sqlOutputContentProvider, userSurvey, publishProject, and other modules to utilize the new logger.
- Improved error messages by including context and using getErrorMessage utility for better clarity.
- Removed deprecated logDebug function and adjusted logging practices in utils and other components.
- Updated logging messages in AzureController to be more generic.
- Changed error logging in MainController to use the logger directly.
- Removed ILogger from SqlToolsServiceClient and replaced with a base logger.
- Updated various services (AzureBlobService, CopilotService, ExecutionPlanService, etc.) to use the new logger format.
- Refactored error handling in services to provide clearer logging messages.
- Updated WebviewRpc to expose a public log property instead of a private logger.
- Adjusted logging in various webview components to utilize the new logging structure.
- Removed logger assertions from various test cases in `dotnetRuntimeProvider.test.ts`, `fileBrowserService.test.ts`, `httpClient.test.ts`, `metadataService.test.ts`, `objectExplorerService.test.ts`, `objectManagementService.test.ts`, `profilerService.test.ts`, `publishProjectWebViewController.test.ts`, `queryRunner.test.ts`, `saveResults.test.ts`, `scriptingService.test.ts`, `serviceClient.test.ts`, `sqlTasksService.test.ts`, and `tableExplorerService.test.ts`.
- Updated tests to focus on rethrowing errors instead of logging them, enhancing clarity and maintainability.
- Adjusted test descriptions to reflect the new focus on error handling.
- Updated imports for ILogger from "../models/logger" to "../sharedInterfaces/logger" across multiple files.
- Adjusted logger usage in various modules to maintain consistency and improve code organization.
- Ensured all references to logger are correctly aligned with the new import structure.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes MSSQL logging by moving the shared logging contract into sharedInterfaces/logger, adding a webview-side ILogger implementation, and migrating extension-host/webview code away from console.* and ad-hoc logging helpers toward the shared logger APIs.

Changes:

  • Introduces shared ILogger/LogEvent contracts and updates webview RPC logging to send structured log events.
  • Replaces many console.* calls and redirects service/controller logging through getLogger(...) / prefixed loggers.
  • Updates unit tests to remove assertions that only verified logging side effects (while keeping behavior/error assertions).

Reviewed changes

Copilot reviewed 118 out of 118 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
extensions/mssql/test/unit/utils.ts Updates test imports to use shared ILogger contract.
extensions/mssql/test/unit/tableExplorerService.test.ts Removes logger stubbing/assertions; keeps rethrow behavior checks.
extensions/mssql/test/unit/sqlTasksService.test.ts Removes logger-related assertions around handler overwrite logging.
extensions/mssql/test/unit/serviceClient.test.ts Updates SqlToolsServiceClient construction after logger injection removal.
extensions/mssql/test/unit/scriptingService.test.ts Drops logger stubs/assertions; keeps telemetry/behavior assertions.
extensions/mssql/test/unit/queryRunner.test.ts Removes output-channel logging stubs/assertions.
extensions/mssql/test/unit/publishProjectWebViewController.test.ts Removes logger.error assertions from container publish error validation helper.
extensions/mssql/test/unit/profilerService.test.ts Drops logger stubbing/assertions; checks error rethrow.
extensions/mssql/test/unit/objectManagementService.test.ts Removes manual logger injection into SqlToolsServiceClient stub.
extensions/mssql/test/unit/objectExplorerService.test.ts Removes logger stubs/assertions; keeps functional assertions.
extensions/mssql/test/unit/notebooks/notebookConnectionManager.test.ts Switches ILogger import to shared interface.
extensions/mssql/test/unit/msalAzureController.test.ts Switches ILogger import to shared interface.
extensions/mssql/test/unit/metadataService.test.ts Removes logger assertions; updates error-handling tests (currently with gaps).
extensions/mssql/test/unit/httpClient.test.ts Removes request-response logging tests; adjusts proxy-related assertion.
extensions/mssql/test/unit/fileBrowserService.test.ts Removes logger stubs/assertions around error notifications.
extensions/mssql/test/unit/fabricProvisioningHelpers.test.ts Switches ILogger import to shared interface.
extensions/mssql/test/unit/download.test.ts Switches ILogger import to shared interface.
extensions/mssql/test/unit/dotnetRuntimeProvider.test.ts Removes logger assertions; switches ILogger import.
extensions/mssql/test/unit/connectionStore.test.ts Switches ILogger import to shared interface.
extensions/mssql/test/unit/connectionManager.test.ts Switches ILogger import to shared interface.
extensions/mssql/test/unit/connectionDialogWebviewController.test.ts Removes logger assertion for unknown button click handling.
extensions/mssql/test/unit/azureHelpers.test.ts Removes logger assertions for invalid account responses.
extensions/mssql/src/webviews/pages/TableExplorer/TableExplorerPage.tsx Replaces clipboard copy console.error with shared logger call.
extensions/mssql/src/webviews/pages/TableExplorer/TableDataGrid.tsx Removes noisy console.log diagnostics in grid update paths.
extensions/mssql/src/webviews/pages/SchemaDesigner/schemaDesignerStateProvider.tsx Replaces console.error with webview logger.
extensions/mssql/src/webviews/pages/SchemaDesigner/schemaDesignerRpcHandlers.ts Replaces console.warn with webview logger.
extensions/mssql/src/webviews/pages/SchemaDesigner/graph/SchemaDiagramFlow.tsx Replaces string-based logging with structured logger usage.
extensions/mssql/src/webviews/pages/SchemaDesigner/definition/changes/useSchemaDesignerChangeState.ts Replaces console.error with webview logger.
extensions/mssql/src/webviews/pages/SchemaDesigner/dab/dabContext.tsx Replaces multiple console.error sites with webview logger.
extensions/mssql/src/webviews/pages/QueryResult/textView.tsx Uses logger API instead of string+level logging; updates hook deps.
extensions/mssql/src/webviews/pages/QueryResult/table/plugins/rowSelectionModel.plugin.ts Removes leftover console.log subscription.
extensions/mssql/src/webviews/pages/QueryResult/table/plugins/contextMenu.plugin.ts Converts action tracing/warnings to ILogger methods.
extensions/mssql/src/webviews/pages/QueryResult/queryResultPane.tsx Replaces console.error with logger for webview-location fetch.
extensions/mssql/src/webviews/pages/PublishProject/components/sqlPackageCommandDialog.tsx Replaces console.error with logger in fetch/copy flows.
extensions/mssql/src/webviews/pages/DacpacDialog/dacpacDialogForm.tsx Routes errors/validation logging through webview logger.
extensions/mssql/src/webviews/pages/ConnectionDialog/fabricBrowsePage.tsx Switches error logging to ILogger.error.
extensions/mssql/src/webviews/pages/ConnectionDialog/components/fabric/sqlCollectionContentsList.component.tsx Removes console.error for unknown artifact types.
extensions/mssql/src/webviews/pages/ConnectionDialog/components/connectionStringDialog.component.tsx Replaces clipboard console.error with logger calls.
extensions/mssql/src/webviews/common/vscodeWebviewProvider.tsx Uses extensionRpc.log for bootstrap error logging.
extensions/mssql/src/webviews/common/utils.ts Exposes extensionRpc.log directly via CoreRPCs.
extensions/mssql/src/webviews/common/rpc.ts Adds webview-side WebviewLogger implementing shared ILogger.
extensions/mssql/src/webviews/common/forms/form.component.tsx Routes missing-component error through logger instead of console.
extensions/mssql/src/views/statusView.ts Adds prefixed logger and replaces Utils.logDebug usage.
extensions/mssql/src/uriOwnership/uriOwnershipCore.ts Replaces console.error with shared logger + getErrorMessage.
extensions/mssql/src/sharedInterfaces/webview.ts Switches webview context logging to log: ILogger + structured LogEvent.
extensions/mssql/src/sharedInterfaces/tableExplorer.ts Extends TableExplorerContextProps from CoreRPCs for shared logging APIs.
extensions/mssql/src/sharedInterfaces/logger.ts Adds centralized ILogger/LoggerMethod/LogEvent contracts.
extensions/mssql/src/services/tableExplorerService.ts Centralizes error logging via getLogger("TableExplorerService").
extensions/mssql/src/services/tableDesignerService.ts Centralizes error logging via getLogger("TableDesignerService").
extensions/mssql/src/services/sqlTasksService.ts Uses prefixed logger and removes console.warn usage.
extensions/mssql/src/services/schemaDesignerService.ts Uses prefixed logger for service request failures.
extensions/mssql/src/services/profilerService.ts Uses prefixed logger + getErrorMessage for failures.
extensions/mssql/src/services/objectManagementService.ts Uses prefixed logger for backup/restore request failures.
extensions/mssql/src/services/metadataService.ts Uses prefixed logger + getErrorMessage for request failures.
extensions/mssql/src/services/fileBrowserService.ts Uses prefixed logger for notification/close failures.
extensions/mssql/src/services/executionPlanService.ts Uses prefixed logger for request failure logging.
extensions/mssql/src/services/copilotService.ts Uses prefixed logger for SQL Copilot request failures.
extensions/mssql/src/services/azureBlobService.ts Uses prefixed logger for SAS creation failures.
extensions/mssql/src/scripting/scriptingService.ts Switches ILogger import to shared interface.
extensions/mssql/src/schemaDesigner/schemaDesignerWebviewManager.ts Replaces console.error with logger + getErrorMessage on dispose failures.
extensions/mssql/src/schemaCompare/schemaCompareWebViewController.ts Removes redundant console.error after logger.error call.
extensions/mssql/src/schemaCompare/schemaCompareUtils.ts Switches ILogger import to shared interface.
extensions/mssql/src/queryResult/utils.ts Replaces console.warn with logger.warn for invalid requests.
extensions/mssql/src/publishProject/projectUtils.ts Replaces console.warn with logger.warn + getErrorMessage on parse failures.
extensions/mssql/src/profiler/profilerSession.ts Switches ILogger import to shared interface.
extensions/mssql/src/profiler/profilerController.ts Switches ILogger import to shared interface.
extensions/mssql/src/objectExplorer/objectExplorerService.ts Switches ILogger import to shared interface.
extensions/mssql/src/objectExplorer/objectExplorerDragAndDropController.ts Switches ILogger import to shared interface.
extensions/mssql/src/nps/userSurvey.ts Replaces console.error with logger.error + getErrorMessage.
extensions/mssql/src/notebooks/sqlNotebookController.ts Switches ILogger import to shared interface.
extensions/mssql/src/notebooks/notebookQueryExecutor.ts Switches ILogger import to shared interface.
extensions/mssql/src/notebooks/notebookConnectionManager.ts Switches ILogger import to shared interface.
extensions/mssql/src/mssqlProtocolHandler.ts Switches ILogger import to shared interface.
extensions/mssql/src/models/utils.ts Removes old debug helper and routes launch diagnostics through ILogger.
extensions/mssql/src/models/sqlOutputContentProvider.ts Replaces console.log with logger.error for query-run failures.
extensions/mssql/src/models/resultsSerializer.ts Replaces output-channel logging with prefixed logger calls.
extensions/mssql/src/models/logger.ts Moves ILogger contract to shared interfaces and adds getLogger(prefix?).
extensions/mssql/src/models/connectionStore.ts Switches ILogger import to shared interface.
extensions/mssql/src/models/connectionInfo.ts Replaces console.error with prefixed logger + getErrorMessage.
extensions/mssql/src/languageservice/serviceInstallerUtil.ts Switches ILogger import to shared interface.
extensions/mssql/src/languageservice/serviceDownloadProvider.ts Switches ILogger import to shared interface.
extensions/mssql/src/languageservice/serviceclient.ts Removes injected logger; uses getLogger and passes logger to launch-args helper.
extensions/mssql/src/languageservice/interfaces.ts Switches ILogger import to shared interface.
extensions/mssql/src/languageservice/downloadHelper.ts Switches ILogger import to shared interface.
extensions/mssql/src/languageservice/dotnetRuntimeProvider.ts Switches ILogger import to shared interface.
extensions/mssql/src/languageservice/decompressProvider.ts Switches ILogger import to shared interface.
extensions/mssql/src/http/httpClientCore.ts Switches ILogger import to shared interface.
extensions/mssql/src/http/httpClient.ts Switches ILogger import to shared interface.
extensions/mssql/src/fabric/fabricHelper.ts Replaces console.error with prefixed logger usage.
extensions/mssql/src/deployment/fabricProvisioningHelpers.ts Switches ILogger import to shared interface.
extensions/mssql/src/deployment/azureSqlDatabaseHelpers.ts Switches ILogger import to shared interface.
extensions/mssql/src/credentialstore/credentialstore.ts Switches ILogger import to shared interface.
extensions/mssql/src/copilot/chatAgentRequestHandler.ts Replaces console.error with prefixed logger + getErrorMessage.
extensions/mssql/src/controllers/webviewPanelController.ts Replaces console.error with this.logger.error(...) for dispose/restore prompt errors.
extensions/mssql/src/controllers/webviewBaseController.ts Updates webview log notification handling to structured LogEvent routing.
extensions/mssql/src/controllers/vscodeWrapper.ts Removes legacy logToOutputChannel helper.
extensions/mssql/src/controllers/queryRunner.ts Replaces output-channel logging with prefixed logger calls.
extensions/mssql/src/controllers/mainController.ts Removes old debug helper usage; routes warnings/errors via logger.
extensions/mssql/src/controllers/localizationCache.ts Replaces console.error with prefixed logger + getErrorMessage.
extensions/mssql/src/controllers/connectionManager.ts Replaces output-channel logging with prefixed logger calls.
extensions/mssql/src/controllers/azureDataStudioMigrationWebviewController.ts Replaces console.error with controller logger.error.
extensions/mssql/src/controllers/addFirewallRuleWebviewController.ts Replaces console.error with prefixed logger + getErrorMessage.
extensions/mssql/src/connectionSharing/connectionSharingService.ts Switches ILogger import to shared interface.
extensions/mssql/src/connectionconfig/formComponentHelpers.ts Replaces console.error with prefixed logger + getErrorMessage.
extensions/mssql/src/connectionconfig/connectionconfig.ts Switches ILogger import to shared interface.
extensions/mssql/src/connectionconfig/browseProvider.ts Switches ILogger import to shared interface.
extensions/mssql/src/connectionconfig/azureHelpers.ts Replaces console.* with prefixed logger; standardizes thrown error messages.
extensions/mssql/src/codeAnalysis/codeAnalysisWebViewController.ts Routes “rules saved” messaging via controller logger instead of output channel.
extensions/mssql/src/azure/utils.ts Replaces console.error with prefixed logger + getErrorMessage.
extensions/mssql/src/azure/simpleWebServer.ts Replaces console.log/error with prefixed logger and safe error formatting.
extensions/mssql/src/azure/msal/msalCachePlugin.ts Switches ILogger import to shared interface.
extensions/mssql/src/azure/msal/msalAzureDeviceCode.ts Switches ILogger import to shared interface.
extensions/mssql/src/azure/msal/msalAzureController.ts Switches ILogger import to shared interface.
extensions/mssql/src/azure/msal/msalAzureCodeGrant.ts Switches ILogger import to shared interface.
extensions/mssql/src/azure/msal/msalAzureAuth.ts Switches ILogger import to shared interface.
extensions/mssql/src/azure/fileEncryptionHelper.ts Switches ILogger import to shared interface.
extensions/mssql/src/azure/azureController.ts Switches ILogger import to shared interface; reduces account-id logging.
extensions/mssql/src/azure/accountStore.ts Switches ILogger import to shared interface.

Comment thread extensions/mssql/src/views/statusView.ts Outdated
Comment thread extensions/mssql/src/services/sqlTasksService.ts Outdated
Comment thread extensions/mssql/src/controllers/webviewPanelController.ts Outdated
Comment thread extensions/mssql/src/models/utils.ts Outdated
Comment thread extensions/mssql/test/unit/metadataService.test.ts
Comment thread extensions/mssql/test/unit/metadataService.test.ts
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

PR Changes

Category Target Branch PR Branch Difference
vscode-mssql VSIX 78798 KB 78798 KB ⚪ 0 KB ( 0% )
sql-database-projects VSIX 6309 KB 6309 KB ⚪ 0 KB ( 0% )
data-workspace VSIX 535 KB 535 KB ⚪ 0 KB ( 0% )
keymap VSIX 7 KB 7 KB ⚪ 0 KB ( 0% )

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 3, 2026

Codecov Report

❌ Patch coverage is 63.18681% with 201 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.32%. Comparing base (6c82bda) to head (32311cf).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
extensions/mssql/src/webviews/common/rpc.ts 42.66% 43 Missing ⚠️
...ensions/mssql/src/connectionconfig/azureHelpers.ts 20.00% 28 Missing ⚠️
...ons/mssql/src/controllers/webviewBaseController.ts 8.33% 22 Missing ⚠️
extensions/mssql/src/fabric/fabricHelper.ts 9.52% 19 Missing ⚠️
...ensions/mssql/src/languageservice/serviceclient.ts 60.00% 10 Missing ⚠️
extensions/mssql/src/controllers/mainController.ts 20.00% 8 Missing ⚠️
...ssql/src/webviews/common/vscodeWebviewProvider.tsx 0.00% 7 Missing ⚠️
...nsions/mssql/src/services/schemaDesignerService.ts 33.33% 6 Missing ⚠️
...ensions/mssql/src/services/tableDesignerService.ts 40.00% 6 Missing ⚠️
extensions/mssql/src/models/utils.ts 16.66% 5 Missing ⚠️
... and 27 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #22276      +/-   ##
==========================================
- Coverage   74.34%   74.32%   -0.03%     
==========================================
  Files         407      408       +1     
  Lines      128228   128404     +176     
  Branches     7766     7768       +2     
==========================================
+ Hits        95335    95430      +95     
- Misses      32893    32974      +81     
Flag Coverage Δ
data-workspace 77.10% <ø> (ø)
mssql 73.94% <63.05%> (-0.04%) ⬇️
sqlproj 77.56% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
extensions/mssql/src/azure/accountStore.ts 47.72% <100.00%> (+0.29%) ⬆️
extensions/mssql/src/azure/fileEncryptionHelper.ts 35.56% <100.00%> (ø)
extensions/mssql/src/azure/msal/msalAzureAuth.ts 45.68% <100.00%> (ø)
...nsions/mssql/src/azure/msal/msalAzureController.ts 53.51% <100.00%> (ø)
...nsions/mssql/src/azure/msal/msalAzureDeviceCode.ts 38.38% <100.00%> (ø)
extensions/mssql/src/azure/msal/msalCachePlugin.ts 44.37% <100.00%> (ø)
.../src/codeAnalysis/codeAnalysisWebViewController.ts 92.35% <100.00%> (ø)
.../src/connectionSharing/connectionSharingService.ts 98.93% <100.00%> (+<0.01%) ⬆️
...sions/mssql/src/connectionconfig/browseProvider.ts 87.42% <100.00%> (ø)
...ons/mssql/src/connectionconfig/connectionconfig.ts 93.31% <100.00%> (+<0.01%) ⬆️
... and 75 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI review requested due to automatic review settings June 3, 2026 16:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 118 out of 118 changed files in this pull request and generated 6 comments.

Comment on lines 192 to 196
useEffect(() => {
getWebviewLocation().catch((e) => {
console.error(e);
log.error("Failed to get webview location", e);
setWebviewLocation("panel");
});
Comment on lines +154 to +157
this.getFabricLogger().error(
"Error processing Fabric databases",
getErrorMessage(error),
);
Comment on lines +205 to +208
this.getFabricLogger().error(
"Error processing Fabric SQL Endpoints",
getErrorMessage(error),
);
Comment on lines +256 to +259
this.getFabricLogger().error(
"Error processing Fabric Warehouses",
getErrorMessage(error),
);
Comment on lines 282 to +285
} catch (error) {
console.error(`Error fetching server URL for SQL Endpoints: ${getErrorMessage(error)}`);
this.getFabricLogger().error(
`Error fetching server URL for SQL Endpoints: ${getErrorMessage(error)}`,
);
Comment on lines 301 to +305
} catch (err) {
console.error(err);
this.getFabricLogger().error(
"Error fetching roles for Fabric workspace",
getErrorMessage(err),
);
Copilot AI review requested due to automatic review settings June 3, 2026 22:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 120 out of 120 changed files in this pull request and generated 2 comments.

Comment on lines +314 to +316
case LoggerMethod.Dispose:
targetLogger.dispose();
break;

if (!addTableResult.success) {
console.error("Node with position not found for table:", table);
extensionRpc.log.error("Node with position not found for table");
@aasimkhan30 aasimkhan30 merged commit c4983f9 into main Jun 3, 2026
9 of 11 checks passed
@aasimkhan30 aasimkhan30 deleted the aasim/fix/logCleanup branch June 3, 2026 23:32
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.

4 participants