Spike - Excessive output #9611

Activity

1. Karthik Nadig (karthiknadig)

   added

   needs proposalNeed to make some design decisionsNeed to make some design decisions

   bugIssue identified by VS Code Team member as probable bugIssue identified by VS Code Team member as probable bug

   area-diagnosticsLogging/Features to facilitate diagnosing user problemsLogging/Features to facilitate diagnosing user problems

   on Jan 15, 2020
2. Luciana Abud (luabud)

   added this to the 2020 January Sprint milestone on Jan 15, 2020
3. Kim-Adeline Miguel (kimadeline)

   added

   in progress

   on Jan 20, 2020
4. Kim-Adeline Miguel (kimadeline)

   self-assigned this

   on Jan 20, 2020
5. kimadeline commented on Jan 20, 2020

   

   Kim-Adeline Miguel (kimadeline)

   on Jan 20, 2020

   Last edited by kimadeline

   More actions

   To be done:

   • What is being displayed in the dev tools, and where it comes from
   • If it is still necessary or not
   • Possible solutions:
     • move all this output to a Trace level (setting to enable/disable it) and put it in the Python output channel
     • move to a separate output channel
6. kimadeline commented on Jan 20, 2020

   

   Kim-Adeline Miguel (kimadeline)

   on Jan 20, 2020

   Last edited by kimadeline

   More actions

   📣 There are 4 5 ways to log content in the extension at the moment, all defined in logger.ts:

   • use @traceDecorators.[verbose|error|info|warn] decorators
   • use the static methods from the Logger class directly;
   • use the exported trace[Verbose|Error|Info|Warning] functions:
     • traceVerbose is used a lot for shell detection and interpreter auto selection rules (and some Jupyter kernel functions);
     • traceInfo seems to be used a lot in DS code (and a little bit in the core extension);
     • traceError is used everywhere;
     • traceWarning is mostly used in DS code (and a little bit in the core extension);
   • since we register a Logger instance in our service registry, it can be passed around as part of DI and its (static) methods are called via this.logger.[verbose|error|info|warn];
   • direct calls to (window.)console.[log|warn|error] methods, listed below: some of them are from DS overriding them, but non-DS calls should either use logging methods or be removed

   💡 Conclusions:

   • consolidate the way we do logging
   • remove verbose output 🚮
   • move all logging to an output channel (either the exiting Python one or a separate one), and we can add a trace level flag (None, Error, Warning, Info, and optionally Verbose if we don't want to remove all these logs)
   • do we want CI-specific logging?

   ---

   I started listing all uses cases but when I got to bullet point 3 (trace... functions) there were too many 🙅

   traceDecorators uses

   • activation/activationManager.ts:
     • error level - Failed to activate workspace
   • activation/extensionSurvey.ts:
     • error level - Failed to check whether to display prompt for extension survey
     • error level - Failed to display prompt for extension survey
   • languageServer/activator.ts:
     • error level - Failed to activate language server
     • error level - Failed to ensure language server is available
   • languageServer/analysisOptions.ts:
     • error level - Failed to get analysis options
     • verbose level - Changes in python settings detected in analysis options
   • languageServer/languageServerFolderService.ts:
     • verbose level - Get language server folder name
     • verbose level - Retrieve latest LS package version info
   • languageServer/languageServerPackageService.ts:
     • verbose level - Retrieve latest LS package version info (called in languageServerFolderService.ts)
   • languageServer/languageServerProxy.ts:
     • verbose level - Stopping LS
     • error level - Failed to start LS
     • error level - Failed to load LS extension
   • languageServer/manager.ts:
     • error level - Failed to start LS (calls the function with the verbose 'Starting LS' trace in the same file)
     • error level - Failed to restart LS
     • verbose level - Restarting LS
     • verbose level - Starting LS (calls languageServerProxy..ts)
   • node/activator.ts:
     • error level - Failed to activate LS
   • node/languageServerProxy.ts:
     • verbose level - Stopping LS
     • error level - Failed to start LS
   • node/manager.ts:
     • error level - Failed to start LS (calls the function with the verbose 'Starting LS' trace in the same file)
     • error level - Failed to restart LS
     • verbose level - Restarting LS
     • verbose level - Starting LS (calls languageServerProxy..ts)
   • services/linuxCompatibilityService.ts: verbose level - Checking if 64bit Linux for .NET support (for LS)
   • services/unknownOsCompatibilityService.ts: info level - Unable to determine .NET support (for LS)
   • common/experiments.ts:
     • error level - Failed to identify if user is in experiment (calls sendTelemetryIfInExperiment which also has an error level decorator)
     • error level - Failed to populate user experiments (calls sendTelemetryIfInExperiment which also has an error level decorator, and theisUserInRange method can throw)
     • error level - Failed to send telemetry when user is in experiment
     • error level - Failed to initialize experiments (calls downloadAndStoreExperiments which also has its own error level trace decorator)
     • error level - Failed to download and store experiments
   • insidersBuild/downloadChannelRules.ts:
     • error level - Failed to check if we should look for a daily Insiders build (from accessing or modifying a global persistent state variable)
     • error level - Failed to check if we should look for a weekly Insiders build (from accessing or modifying a global persistent state variable)
   • insidersBuild/downloadChannelService.ts:
     • error level - Failed to update the Insiders channel setting
   • insidersBuild/insidersExtensionPrompt/ts:
     • error level - Error when prompting to install Insiders
     • error level - Error when prompting to reload after installing Insiders
   • insidersBuild/insidersExtensionService.ts:
     • error level - Failed when handling the Insiders channel (calls the checks if we should look for Insiders builds, install Insiders and prompt to reload)
   • installer/extensionBuildInstaller.ts:
     • error level - Failed to install the stable build of the extension (where is it used?)
     • error level - Failed to install the insiders build of the extension
   • nuget/azureBlobStoreNugetRepository.ts:
     • verbose level - List Nuget packages (LS logic)
   • process/pythonDaemonPool.ts:
     • error level - Failed to create Python daemon
     • error level - Failed to test whether the daemon is usable or not (by pinging it)
   • terminal/helper.ts:
     • error level - Failed to send terminal type telemetry
   • kernels/kernelService.ts:
     • error level - DS - Failed to register an interpreter as a kernel
     • error level - Failed to parse kernel creation stdout
   • activation/service.ts:
     • verbose level - Get activated environment variables
     • error level - Failed to parse environment output (to get env vars)
     • verbose level - Parse environment output
   • rules/BaseRule.ts:
     • verbose level - Auto-select interpreter (PR to add more logging for interpreter selection Add more logging for auto selection of interpreters #3903)
     • verbose level - Set global interpreter (Add more logging for auto selection of interpreters #3903)
   • display/progressDisplay.ts:
     • verbose level - Display interpreter locator refreshing progress
     • verbose level - Hide intepreter locator refreshing progress
   • locators/index.ts:
     • verbose level - Get interpreters
   • locators/progressService.ts:
     • verbose level - Handle interpreter refresh progress
     • verbose level - Notify all locators have completed locating interpreters
     • verbose level - Notify locators are still locating (refresh progress)
     • verbose level - Checking if locators have completed locating
   • services/cacheableLocatorService.ts:
     • verbose level - Get interpreters in CacheableLocatorService
   • services/condaService.ts:
     • verbose level - Get conda environments
     • verbose level - Get conda executable path from an interpreter's path
   • services/interpreterWatcherBuilder.ts:
     • verbose level - Build the workspace virtual env interpreter watcher
   • services/windowsStoreInterpreter.ts:
     • error level - Failed getting the hash of the Python interpreter installed from the Windows store
   • services/workspaceVirtualEnvWatcherService.ts:
     • verbose level - Register an interpreter watcher for the workspace
     • verbose level - Notify when interpreter was changed (to be one from a virtual environment)
   • virtualEnvs/condaInheritEnvPrompt.ts:
     • error level - Failed to initialize prompt to flip inheritEnv setting for conda activation issues
     • error level - Failed to display inheritEnv prompt or update the setting
     • error level - Failed to check if the inheritEnv prompt should be displayed or not
   • virtualEnvs/virtualEnvPrompt.ts:
     - error level - Error in the event handler when detecting a virtual env in the workspace and notifying user
   • common/updateTestSettings.ts:
     • error level - Failed to update test-related settings
   • explorer/commandHandlers.ts:
     • error level - Failed to run a test node
     • error level - Failed to debug a test node
     • error level - Failed to open a test node in the editor

   Direct calls to Logger methods and all that

   • checks/powerShellActivation.ts:
     • error level - Failed to switch from Powershell to Command Prompt
   • services/cacheableLocatorService.ts:
     • verbose level - Log when interpreter watcher was created
   • services/condaService.ts:
     • warning level - Unable to parse version of conda
     • warning level - Failed to find the path to the conda executable in default locations
   • services/workspaceVirtualEnvWatcherService.ts:
     • verbose level - Created file system watcher
   • providers/jediProxy.ts:
     • error level - Log jedi error
     • error level - Log spawnProcess.end

   Direct uses of console.

   • DS - create an HTTP server to serve up React UI - Rewires console. calls to post messages to the web panel
     • datascience-ui/react-common/postOffice.ts - console. calls overriden here
     • webPanel.ts - console.log
     • webPanelServer.ts- console.log
     • datascience-ui/react-common/logger.ts - console.log
   • src/test/:
     • exitCIAfterTestReporter.ts: for tests (CI/AzDO)
     • utils/ts: for functional tests
     • debuggerTest.ts: for debugger tests
     • datascience/: DS tests
   • featureDeprecationManager.ts: notify deprecation (console.error) ➡️ can be moved to an output channel
   • socketServer.ts: error in socket server (console.error) ➡️ can be deleted
   • debugStreamProvider.ts: DA logging for third-party extension devs (console.error) ➡️ can be moved to a debug output channel
   • LocalDebugClient.ts: debugging error logging (console.error) ➡️ can be moved to a debug output channel
   • intellisenseDocument.ts: DS autocompletion (console.warn)
   • container.ts: for tests (console.warn) ➡️ use a logging function instead

   ---

   Todo/Questions:

   • List uses of @tracedecorators.func
   • List direct uses of traceError, traceInfo and friends (🙅 too many)
   • List uses of static Logger.[verbose|error]
   • List uses of logger in DI (🙅 too many)
   • Direct console. uses: which level?
7. Kim-Adeline Miguel (kimadeline)

   added

   in review

   and removed

   in progress

   on Jan 22, 2020
8. kimadeline commented on Jan 22, 2020

   

   Kim-Adeline Miguel (kimadeline)

   on Jan 22, 2020

   Last edited by kimadeline

   More actions

   Discussion output

   • Get rid of windows.console Logging consolidation - Remove window.console and console. calls #9721
   • Get rid of the Logger class and DI logger instances Logging consolidation - Remove use of the Logger class #9722
   • See if the decorators have value wherever they are used or if we can replace them with the exported trace functions (and revisit/dev design the useful ones) Spike - Logging consolidation - Do our trace decorators bring any value? #9724
9. Kim-Adeline Miguel (kimadeline)

   closed this as completedon Jan 22, 2020
10. ghost removed

    needs proposalNeed to make some design decisionsNeed to make some design decisions

    on Jan 22, 2020
11. lock

    locked as resolved and limited conversation to collaborators on Jan 30, 2020