From fecbcab18c23ac818ad82b8ffd7c7b007b6a9d3b Mon Sep 17 00:00:00 2001 From: anthogez Date: Wed, 29 May 2024 13:41:22 +0100 Subject: [PATCH] feat: perf enhancement for snyk monitor Rather than have post monitor working 1 by 1, we are trying to achieve an initial and minimal concurrency of 5 monitor requests at once --- src/cli/commands/monitor/index.ts | 126 +++----------------- src/cli/commands/monitor/process-chunks.ts | 130 +++++++++++++++++++++ src/cli/commands/monitor/utils.ts | 36 ++++++ 3 files changed, 181 insertions(+), 111 deletions(-) create mode 100644 src/cli/commands/monitor/process-chunks.ts create mode 100644 src/cli/commands/monitor/utils.ts diff --git a/src/cli/commands/monitor/index.ts b/src/cli/commands/monitor/index.ts index 4a60e28729..460d64c1a4 100644 --- a/src/cli/commands/monitor/index.ts +++ b/src/cli/commands/monitor/index.ts @@ -7,9 +7,6 @@ import { checkOSSPaths } from '../../../lib/check-paths'; import * as theme from '../../../lib/theme'; import { - MonitorOptions, - MonitorMeta, - MonitorResult, Options, Contributor, ProjectAttributes, @@ -18,28 +15,21 @@ import { PROJECT_LIFECYCLE, Tag, } from '../../../lib/types'; -import config from '../../../lib/config'; + import * as detect from '../../../lib/detect'; import { GoodResult, BadResult } from './types'; import { spinner } from '../../../lib/spinner'; import * as analytics from '../../../lib/analytics'; import { MethodArgs } from '../../args'; import { apiOrOAuthTokenExists } from '../../../lib/api-token'; -import { maybePrintDepTree, maybePrintDepGraph } from '../../../lib/print-deps'; -import { monitor as snykMonitor } from '../../../lib/monitor'; import { processJsonMonitorResponse } from './process-json-monitor'; import snyk = require('../../../lib'); // TODO(kyegupov): fix import -import { formatMonitorOutput } from '../../../lib/formatters'; import { getDepsFromPlugin } from '../../../lib/plugins/get-deps-from-plugin'; -import { getExtraProjectCount } from '../../../lib/plugins/get-extra-project-count'; -import { extractPackageManager } from '../../../lib/plugins/extract-package-manager'; import { MultiProjectResultCustom } from '../../../lib/plugins/get-multi-plugin-result'; import { convertMultiResultToMultiCustom } from '../../../lib/plugins/convert-multi-plugin-res-to-multi-custom'; import { convertSingleResultToMultiCustom } from '../../../lib/plugins/convert-single-splugin-res-to-multi-custom'; -import { PluginMetadata } from '@snyk/cli-interface/legacy/plugin'; import { getContributors } from '../../../lib/monitor/dev-count-analysis'; import { - FailedToRunTestError, MonitorError, MissingArgError, ValidationError, @@ -50,6 +40,8 @@ import { getFormattedMonitorOutput } from '../../../lib/ecosystems/monitor'; import { processCommandArgs } from '../process-command-args'; import { hasFeatureFlag } from '../../../lib/feature-flags'; import { PNPM_FEATURE_FLAG } from '../../../lib/package-managers'; +import { promiseOrCleanup } from './utils'; +import { monitorProcessChunksCommand } from './process-chunks'; const SEPARATOR = '\n-------------------------------------------------------\n'; const debug = Debug('snyk'); @@ -59,18 +51,6 @@ commands. If you are using Snyk in a CI pipeline, action may be required. Read https://snyk.io/blog/securing-container-applications-using-the-snyk-cli/ for more info.`; -// This is used instead of `let x; try { x = await ... } catch { cleanup }` to avoid -// declaring the type of x as possibly undefined. -async function promiseOrCleanup( - p: Promise, - cleanup: (x?) => void, -): Promise { - return p.catch((error) => { - cleanup(); - throw error; - }); -} - // Returns an array of Registry responses (one per every sub-project scanned), a single response, // or an error message. export default async function monitor(...args0: MethodArgs): Promise { @@ -265,72 +245,18 @@ export default async function monitor(...args0: MethodArgs): Promise { await spinner(postingMonitorSpinnerLabel); // Post the project dependencies to the Registry - for (const projectDeps of perProjectResult.scannedProjects) { - try { - if (!projectDeps.depGraph && !projectDeps.depTree) { - debug( - 'scannedProject is missing depGraph or depTree, cannot run test/monitor', - ); - throw new FailedToRunTestError( - 'Your monitor request could not be completed. Please email support@snyk.io', - ); - } - const extractedPackageManager = extractPackageManager( - projectDeps, - perProjectResult, - options as MonitorOptions & Options, - ); - - analytics.add('packageManager', extractedPackageManager); - - const projectName = getProjectName(projectDeps); - if (projectDeps.depGraph) { - debug(`Processing ${projectDeps.depGraph.rootPkg?.name}...`); - maybePrintDepGraph(options, projectDeps.depGraph); - } - - if (projectDeps.depTree) { - debug(`Processing ${projectDeps.depTree.name}...`); - maybePrintDepTree(options, projectDeps.depTree); - } - - const tFile = projectDeps.targetFile || targetFile; - const targetFileRelativePath = - projectDeps.plugin.targetFile || - (tFile && pathUtil.join(pathUtil.resolve(path), tFile)) || - ''; - - const res: MonitorResult = await promiseOrCleanup( - snykMonitor( - path, - generateMonitorMeta(options, extractedPackageManager), - projectDeps, - options, - projectDeps.plugin as PluginMetadata, - targetFileRelativePath, - contributors, - generateProjectAttributes(options), - generateTags(options), - ), - spinner.clear(postingMonitorSpinnerLabel), - ); - - res.path = path; - const monOutput = formatMonitorOutput( - extractedPackageManager, - res, - options, - projectName, - await getExtraProjectCount(path, options, inspectResult), - ); - // push a good result - results.push({ ok: true, data: monOutput, path, projectName }); - } catch (err) { - // pushing this error allow this inner loop to keep scanning the projects - // even if 1 in 100 fails - results.push({ ok: false, data: err, path }); - } - } + const monitorResults = await monitorProcessChunksCommand( + path, + inspectResult, + perProjectResult, + contributors, + options, + targetFile, + ); + + monitorResults.forEach((res) => { + results.push(res); + }); } catch (err) { // push this error, the loop continues results.push({ ok: false, data: err, path }); @@ -369,20 +295,6 @@ export default async function monitor(...args0: MethodArgs): Promise { throw new Error(output); } -function generateMonitorMeta(options, packageManager?): MonitorMeta { - return { - method: 'cli', - packageManager, - 'policy-path': options['policy-path'], - 'project-name': options['project-name'] || config.PROJECT_NAME, - isDocker: !!options.docker, - prune: !!options.pruneRepeatedSubdependencies, - 'remote-repo-url': options['remote-repo-url'], - targetReference: options['target-reference'], - assetsProjectName: options['assets-project-name'], - }; -} - /** * Parse an attribute from the CLI into the relevant enum type. * @@ -522,11 +434,3 @@ function validateMonitorPath(path: string, isDocker?: boolean): void { throw new Error('"' + path + '" is not a valid path for "snyk monitor"'); } } - -function getProjectName(projectDeps): string { - return ( - projectDeps.meta?.gradleProjectName || - projectDeps.depGraph?.rootPkg?.name || - projectDeps.depTree?.name - ); -} diff --git a/src/cli/commands/monitor/process-chunks.ts b/src/cli/commands/monitor/process-chunks.ts new file mode 100644 index 0000000000..5cbc89ab0e --- /dev/null +++ b/src/cli/commands/monitor/process-chunks.ts @@ -0,0 +1,130 @@ +import * as Debug from 'debug'; +import * as pathUtil from 'path'; +import * as pMap from 'p-map'; +import * as analytics from '../../../lib/analytics'; +import { FailedToRunTestError } from '../../../lib/errors/failed-to-run-test-error'; +import { extractPackageManager } from '../../../lib/plugins/extract-package-manager'; +import { MultiProjectResultCustom } from '../../../lib/plugins/get-multi-plugin-result'; +import { monitor as snykMonitor } from '../../../lib/monitor'; +import { spinner } from '../../../lib/spinner'; +import { + Options, + MonitorOptions, + MonitorResult, + Contributor, +} from '../../../lib/types'; +import { + generateMonitorMeta, + getSCAProjectName, + promiseOrCleanup, +} from './utils'; +import { maybePrintDepGraph, maybePrintDepTree } from '../../../lib/print-deps'; +import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; +import { PluginMetadata } from '@snyk/cli-interface/legacy/plugin'; +import { generateProjectAttributes, generateTags } from '.'; +import { BadResult, GoodResult } from './types'; +import { formatMonitorOutput } from '../../../lib/formatters'; +import { getExtraProjectCount } from '../../../lib/plugins/get-extra-project-count'; + +const debug = Debug('snyk'); + +export async function monitorProcessChunksCommand( + path: string, + inspectResult: MultiProjectResultCustom | pluginApi.MultiProjectResult, + multiProjectResult: MultiProjectResultCustom, + contributors: Contributor[], + options: MonitorOptions & Options, + targetFile?: string, +): Promise> { + const { scannedProjects } = multiProjectResult; + + const results: Array = []; + const MAX_CONCURRENCY = 5; + + try { + await pMap( + scannedProjects, + async (scannedProject) => { + const { depGraph, depTree } = scannedProject; + + if (!depGraph && !depTree) { + debug( + 'scannedProject is missing depGraph or depTree, cannot run test/monitor', + ); + throw new FailedToRunTestError( + 'Your monitor request could not be completed. Please email support@snyk.io', + ); + } + + const extractedPackageManager = extractPackageManager( + scannedProject, + multiProjectResult, + options as MonitorOptions & Options, + ); + + analytics.add('packageManager', extractedPackageManager); + + const projectName = getSCAProjectName(scannedProject); + + if (depGraph) { + debug(`Processing ${depGraph.rootPkg?.name}...`); + maybePrintDepGraph(options, depGraph); + } + + if (depTree) { + debug(`Processing ${depTree.name}...`); + maybePrintDepTree(options, depTree); + } + + const tFile = scannedProject.targetFile || targetFile; + const targetFileRelativePath = + scannedProject.plugin.targetFile || + (tFile && pathUtil.join(pathUtil.resolve(path), tFile)) || + ''; + + const displayPath = pathUtil.relative( + '.', + pathUtil.join(path, targetFile || ''), + ); + + const postingMonitorSpinnerLabel = + 'Posting monitor snapshot for ' + displayPath + ' ...'; + + const res: MonitorResult = await promiseOrCleanup( + snykMonitor( + path, + generateMonitorMeta(options, extractedPackageManager), + scannedProject, + options, + scannedProject.plugin as PluginMetadata, + targetFileRelativePath, + contributors, + generateProjectAttributes(options), + generateTags(options), + ), + spinner.clear(postingMonitorSpinnerLabel), + ); + + res.path = path; + const monOutput = formatMonitorOutput( + extractedPackageManager, + res, + options, + projectName, + await getExtraProjectCount(path, options, inspectResult), + ); + // push a good result + results.push({ ok: true, data: monOutput, path, projectName }); + }, + { concurrency: MAX_CONCURRENCY }, + ); + } catch (err) { + // pushing this error allow this inner loop to keep scanning the projects + // even if 1 in 100 fails + results.push({ ok: false, data: err, path }); + } finally { + spinner.clearAll(); + } + + return results; +} diff --git a/src/cli/commands/monitor/utils.ts b/src/cli/commands/monitor/utils.ts new file mode 100644 index 0000000000..9f930bcc6a --- /dev/null +++ b/src/cli/commands/monitor/utils.ts @@ -0,0 +1,36 @@ +import { MonitorMeta } from '../../../lib/types'; +import config from '../../../lib/config'; + +export function getSCAProjectName(projectDeps): string { + return ( + projectDeps.meta?.gradleProjectName || + projectDeps.depGraph?.rootPkg?.name || + projectDeps.depTree?.name + ); +} + +// This is used instead of `let x; try { x = await ... } catch { cleanup }` to avoid +// declaring the type of x as possibly undefined. +export async function promiseOrCleanup( + p: Promise, + cleanup: (x?) => void, +): Promise { + return p.catch((error) => { + cleanup(); + throw error; + }); +} + +export function generateMonitorMeta(options, packageManager?): MonitorMeta { + return { + method: 'cli', + packageManager, + 'policy-path': options['policy-path'], + 'project-name': options['project-name'] || config.PROJECT_NAME, + isDocker: !!options.docker, + prune: !!options.pruneRepeatedSubdependencies, + 'remote-repo-url': options['remote-repo-url'], + targetReference: options['target-reference'], + assetsProjectName: options['assets-project-name'], + }; +}