fix: remove amplitude experiment dependency for Code scans [HEAD-316] (…

…#359)
snyk · May 18, 2023 · fd720fb · fd720fb
1 parent 73df773
commit fd720fb
Show file tree

Hide file tree

Showing 18 changed files with 53 additions and 475 deletions.
diff --git a/src/snyk/base/modules/baseSnykModule.ts b/src/snyk/base/modules/baseSnykModule.ts
@@ -6,7 +6,6 @@ import { CommandController } from '../../common/commands/commandController';
 import { configuration } from '../../common/configuration/instance';
 import { IWorkspaceTrust, WorkspaceTrust } from '../../common/configuration/trustedFolders';
 import { ExperimentService } from '../../common/experiment/services/experimentService';
-import { CodeScanOrchestrator } from '../../common/languageServer/experiments/codeScanOrchestrator';
 import { ILanguageServer } from '../../common/languageServer/languageServer';
 import { CodeIssueData, IacIssueData } from '../../common/languageServer/types';
 import { Logger } from '../../common/logger/logger';
@@ -65,7 +64,6 @@ export default abstract class BaseSnykModule implements IBaseSnykModule {
   snykCodeOld: ISnykCodeServiceOld;
   snykCode: IProductService<CodeIssueData>;
   protected codeSettings: ICodeSettings;
-  protected codeScanOrchestrator: CodeScanOrchestrator;
 
   iacService: IProductService<IacIssueData>;
 
@@ -102,7 +100,5 @@ export default abstract class BaseSnykModule implements IBaseSnykModule {
 
   abstract runScan(): Promise<void>;
 
-  abstract runCodeScan(): Promise<void>;
-
   abstract runOssScan(): Promise<void>;
 }
diff --git a/src/snyk/base/modules/interfaces.ts b/src/snyk/base/modules/interfaces.ts
@@ -19,7 +19,6 @@ export interface IBaseSnykModule {
 
   // Abstract methods
   runScan(): Promise<void>;
-  runCodeScan(manual?: boolean): Promise<void>;
   runOssScan(manual?: boolean): Promise<void>;
 }
 

diff --git a/src/snyk/base/modules/snykLib.ts b/src/snyk/base/modules/snykLib.ts
@@ -6,15 +6,15 @@ import { configuration } from '../../common/configuration/instance';
 import { DEFAULT_SCAN_DEBOUNCE_INTERVAL, IDE_NAME, OSS_SCAN_DEBOUNCE_INTERVAL } from '../../common/constants/general';
 import { SNYK_CONTEXT } from '../../common/constants/views';
 import { ErrorHandler } from '../../common/error/errorHandler';
-import { ExperimentKey } from '../../common/experiment/services/experimentService';
 import { Logger } from '../../common/logger/logger';
+import { vsCodeCommands } from '../../common/vscode/commands';
 import { vsCodeWorkspace } from '../../common/vscode/workspace';
 import BaseSnykModule from './baseSnykModule';
 import { ISnykLib } from './interfaces';
-import { vsCodeCommands } from '../../common/vscode/commands';
 
 export default class SnykLib extends BaseSnykModule implements ISnykLib {
   private async runFullScan_(manual = false): Promise<void> {
+    // Only starts OSS scan. Code & IaC scans are managed by LS
     Logger.info('Starting full scan');
 
     await this.contextService.setContext(SNYK_CONTEXT.ERROR, false);
@@ -44,9 +44,7 @@ export default class SnykLib extends BaseSnykModule implements ISnykLib {
 
       if (workspacePaths.length) {
         this.logFullAnalysisIsTriggered(manual);
-
         void this.startOssAnalysis(manual, false);
-        await this.startSnykCodeAnalysis(workspacePaths, manual, false); // mark void, handle errors inside of startSnykCodeAnalysis()
       }
     } catch (err) {
       await ErrorHandler.handleGlobal(err, Logger, this.contextService, this.loadingBadge);
@@ -55,59 +53,15 @@ export default class SnykLib extends BaseSnykModule implements ISnykLib {
 
   // This function is called by commands, error handlers, etc.
   // We should avoid having duplicate parallel executions.
+  // Only starts OSS scan. Code & IaC scans are managed by LS
   public runScan = _.debounce(this.runFullScan_.bind(this), DEFAULT_SCAN_DEBOUNCE_INTERVAL, { leading: true });
 
-  public runCodeScan = _.debounce(this.startSnykCodeAnalysis.bind(this), DEFAULT_SCAN_DEBOUNCE_INTERVAL, {
-    leading: true,
-  });
-
   public runOssScan = _.debounce(this.startOssAnalysis.bind(this), OSS_SCAN_DEBOUNCE_INTERVAL, { leading: true });
 
   async enableCode(): Promise<void> {
+    Logger.info('Enabling Snyk Code');
     const wasEnabled = await this.codeSettings.enable();
-    if (wasEnabled) {
-      await this.codeSettings.checkCodeEnabled();
-
-      Logger.info('Snyk Code was enabled.');
-      try {
-        await this.startSnykCodeAnalysis();
-      } catch (err) {
-        ErrorHandler.handle(err, Logger);
-      }
-    }
-  }
-
-  async startSnykCodeAnalysis(paths: string[] = [], manual = false, reportTriggeredEvent = true): Promise<void> {
-    // If the execution is suspended, we only allow user-triggered Snyk Code analyses.
-    if (this.isSnykCodeAutoscanSuspended(manual)) {
-      return;
-    }
-
-    const codeEnabled = await this.codeSettings.checkCodeEnabled();
-    if (!codeEnabled) {
-      return;
-    }
-
-    // if LS is used to scan, don't proceed
-    const codeScansViaLs = await this.experimentService.isUserPartOfExperiment(
-      ExperimentKey.CodeScansViaLanguageServer,
-    );
-    if (codeScansViaLs) {
-      return;
-    }
-
-    if (
-      !configuration.getFeaturesConfiguration()?.codeSecurityEnabled &&
-      !configuration.getFeaturesConfiguration()?.codeQualityEnabled
-    ) {
-      return;
-    }
-
-    if (!paths.length) {
-      paths = vsCodeWorkspace.getWorkspaceFolders();
-    }
-
-    await this.snykCodeOld.startAnalysis(paths, manual, reportTriggeredEvent);
+    Logger.info(wasEnabled ? 'Snyk Code was enabled' : 'Failed to enable Snyk Code');
   }
 
   async onDidChangeWelcomeViewVisibility(visible: boolean): Promise<void> {

diff --git a/src/snyk/common/experiment/services/experimentService.ts b/src/snyk/common/experiment/services/experimentService.ts
@@ -7,7 +7,6 @@ import { User } from '../../user';
 export enum ExperimentKey {
   // to be populated with running experiment keys
   TestExperiment = 'vscode-test-experiment',
-  CodeScansViaLanguageServer = 'snyk-code-via-ls-in-vs-code-integration',
 }
 
 export class ExperimentService {

diff --git a/src/snyk/common/languageServer/experiments/codeScanOrchestrator.ts b/src/snyk/common/languageServer/experiments/codeScanOrchestrator.ts
diff --git a/src/snyk/common/languageServer/languageServer.ts b/src/snyk/common/languageServer/languageServer.ts
@@ -12,7 +12,6 @@ import {
 } from '../constants/languageServer';
 import { CONFIGURATION_IDENTIFIER } from '../constants/settings';
 import { ErrorHandler } from '../error/errorHandler';
-import { ExperimentService } from '../experiment/services/experimentService';
 import { ILog } from '../logger/interfaces';
 import { getProxyEnvVariable, getProxyOptions } from '../proxy';
 import { DownloadService } from '../services/downloadService';
@@ -49,7 +48,6 @@ export class LanguageServer implements ILanguageServer {
     private authenticationService: IAuthenticationService,
     private readonly logger: ILog,
     private downloadService: DownloadService,
-    private experimentService: ExperimentService,
   ) {
     this.downloadService = downloadService;
   }
@@ -97,7 +95,7 @@ export class LanguageServer implements ILanguageServer {
       synchronize: {
         configurationSection: CONFIGURATION_IDENTIFIER,
       },
-      middleware: new LanguageClientMiddleware(this.configuration, this.experimentService),
+      middleware: new LanguageClientMiddleware(this.configuration),
       /**
        * We reuse the output channel here as it's not properly disposed of by the language client ([email protected])
        * See: https://github.com/microsoft/vscode-languageserver-node/blob/cdf4d6fdaefe329ce417621cf0f8b14e0b9bb39d/client/src/common/client.ts#L2789
@@ -107,16 +105,17 @@ export class LanguageServer implements ILanguageServer {
 
     // Create the language client and start the client.
     this.client = this.languageClientAdapter.create('Snyk LS', SNYK_LANGUAGE_SERVER_NAME, serverOptions, clientOptions);
-    this.client
-      .onReady()
-      .then(() => {
-        this.registerListeners(this.client);
-      })
-      .catch((error: Error) => ErrorHandler.handle(error, this.logger, error.message));
 
     // Start the client. This will also launch the server
     this.client.start();
     this.logger.info('Snyk Language Server started');
+
+    try {
+      await this.client.onReady();
+      this.registerListeners(this.client);
+    } catch (error) {
+      return ErrorHandler.handle(error, this.logger, error instanceof Error ? error.message : 'An error occurred');
+    }
   }
 
   private registerListeners(client: LanguageClient): void {
@@ -165,7 +164,7 @@ export class LanguageServer implements ILanguageServer {
   // Initialization options are not semantically equal to server settings, thus separated here
   // https://github.com/microsoft/language-server-protocol/issues/567
   async getInitializationOptions(): Promise<InitializationOptions> {
-    const settings = await LanguageServerSettings.fromConfiguration(this.configuration, this.experimentService);
+    const settings = await LanguageServerSettings.fromConfiguration(this.configuration);
 
     return {
       ...settings,

diff --git a/src/snyk/common/languageServer/middleware.ts b/src/snyk/common/languageServer/middleware.ts
@@ -1,5 +1,4 @@
 import { IConfiguration } from '../configuration/configuration';
-import { ExperimentService } from '../experiment/services/experimentService';
 import {
   CancellationToken,
   ConfigurationParams,
@@ -19,7 +18,7 @@ export type LanguageClientWorkspaceMiddleware = Partial<WorkspaceMiddleware> & {
 };
 
 export class LanguageClientMiddleware implements Middleware {
-  constructor(private configuration: IConfiguration, private experimentService: ExperimentService) {}
+  constructor(private configuration: IConfiguration) {}
 
   workspace: LanguageClientWorkspaceMiddleware = {
     configuration: async (
@@ -40,7 +39,7 @@ export class LanguageClientMiddleware implements Middleware {
         return [];
       }
 
-      const serverSettings = await LanguageServerSettings.fromConfiguration(this.configuration, this.experimentService);
+      const serverSettings = await LanguageServerSettings.fromConfiguration(this.configuration);
       return [serverSettings];
     },
   };

diff --git a/src/snyk/common/languageServer/settings.ts b/src/snyk/common/languageServer/settings.ts
@@ -1,6 +1,5 @@
 import _ from 'lodash';
 import { IConfiguration, SeverityFilter } from '../configuration/configuration';
-import { ExperimentKey, ExperimentService } from '../experiment/services/experimentService';
 
 export type InitializationOptions = ServerSettings & {
   integrationName?: string;
@@ -31,26 +30,17 @@ export type ServerSettings = {
 };
 
 export class LanguageServerSettings {
-  static async fromConfiguration(
-    configuration: IConfiguration,
-    experimentService: ExperimentService,
-  ): Promise<ServerSettings> {
+  static async fromConfiguration(configuration: IConfiguration): Promise<ServerSettings> {
     const featuresConfiguration = configuration.getFeaturesConfiguration();
 
     const iacEnabled = _.isUndefined(featuresConfiguration.iacEnabled) ? true : featuresConfiguration.iacEnabled;
-    let codeSecurityEnabled = _.isUndefined(featuresConfiguration.codeSecurityEnabled)
+    const codeSecurityEnabled = _.isUndefined(featuresConfiguration.codeSecurityEnabled)
       ? true
       : featuresConfiguration.codeSecurityEnabled;
-    let codeQualityEnabled = _.isUndefined(featuresConfiguration.codeQualityEnabled)
+    const codeQualityEnabled = _.isUndefined(featuresConfiguration.codeQualityEnabled)
       ? true
       : featuresConfiguration.codeQualityEnabled;
 
-    const codeScansViaLs = await experimentService.isUserPartOfExperiment(ExperimentKey.CodeScansViaLanguageServer);
-    if (!codeScansViaLs) {
-      codeSecurityEnabled = false;
-      codeQualityEnabled = false;
-    }
-
     return {
       activateSnykCodeSecurity: `${codeSecurityEnabled}`,
       activateSnykCodeQuality: `${codeQualityEnabled}`,

diff --git a/src/snyk/extension.ts b/src/snyk/extension.ts
@@ -45,8 +45,7 @@ import {
 } from './common/constants/views';
 import { ErrorHandler } from './common/error/errorHandler';
 import { ErrorReporter } from './common/error/errorReporter';
-import { ExperimentKey, ExperimentService } from './common/experiment/services/experimentService';
-import { CodeScanOrchestrator } from './common/languageServer/experiments/codeScanOrchestrator';
+import { ExperimentService } from './common/experiment/services/experimentService';
 import { LanguageServer } from './common/languageServer/languageServer';
 import { StaticLsApi } from './common/languageServer/staticLsApi';
 import { Logger } from './common/logger/logger';
@@ -194,7 +193,6 @@ class SnykExtension extends SnykLib implements IExtension {
       this.authService,
       Logger,
       this.downloadService,
-      this.experimentService,
     );
 
     const codeSuggestionProvider = new CodeSuggestionWebviewProvider(
@@ -441,39 +439,24 @@ class SnykExtension extends SnykLib implements IExtension {
       configuration,
     );
 
-    const codeScansViaLs = await this.experimentService.isUserPartOfExperiment(
-      ExperimentKey.CodeScansViaLanguageServer,
-    );
-    if (codeScansViaLs) {
-      await this.contextService.setContext(SNYK_CONTEXT.LS_CODE_PREVIEW, true);
-      Logger.info('Code scans via language server enabled.');
-    } else {
-      await this.contextService.setContext(SNYK_CONTEXT.LS_CODE_PREVIEW, false);
-      Logger.info('Code scans are not using Language Server.');
-    }
-
-    await this.languageServer.start();
-
-    this.codeScanOrchestrator = new CodeScanOrchestrator(
-      this.experimentService,
-      this.languageServer,
-      Logger,
-      this.contextService,
-      this,
-    );
+    await this.contextService.setContext(SNYK_CONTEXT.LS_CODE_PREVIEW, true);
 
     // noinspection ES6MissingAwait
     void this.advisorScoreDisposable.activate();
 
     // Actually start analysis
     this.runScan();
+
+    // Wait for LS startup to finish before updating the codeEnabled context
+    // The codeEnabled context depends on an LS command
+    await this.languageServer.start();
+    await this.codeSettings.updateIsCodeEnabled();
   }
 
   public async deactivate(): Promise<void> {
     this.snykCodeOld.dispose();
     this.ossVulnerabilityCountService.dispose();
     await this.languageServer.stop();
-    this.codeScanOrchestrator.dispose();
     await this.analytics.flush();
     await ErrorReporter.flush();
   }