Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: perf enhancement for snyk monitor #5272

Closed
wants to merge 1 commit into from
Closed

feat: perf enhancement for snyk monitor #5272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 15 additions & 111 deletions src/cli/commands/monitor/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import { checkOSSPaths } from '../../../lib/check-paths';
import * as theme from '../../../lib/theme';

import {
MonitorOptions,
MonitorMeta,
MonitorResult,
Options,
Contributor,
ProjectAttributes,
Expand All @@ -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,
Expand All @@ -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');
Expand All @@ -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<T>(
p: Promise<T>,
cleanup: (x?) => void,
): Promise<T> {
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<any> {
Expand Down Expand Up @@ -265,72 +245,18 @@ export default async function monitor(...args0: MethodArgs): Promise<any> {
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 [email protected]',
);
}
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 });
Expand Down Expand Up @@ -369,20 +295,6 @@ export default async function monitor(...args0: MethodArgs): Promise<any> {
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.
*
Expand Down Expand Up @@ -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
);
}
130 changes: 130 additions & 0 deletions src/cli/commands/monitor/process-chunks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<Array<GoodResult | BadResult>> {
const { scannedProjects } = multiProjectResult;

const results: Array<GoodResult | BadResult> = [];
const MAX_CONCURRENCY = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: Let's be a bit more pessimistic to not increase the load too much and set it to a lower value first. Let's start with 2.


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 [email protected]',
);
}

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;
}
36 changes: 36 additions & 0 deletions src/cli/commands/monitor/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<T>(
p: Promise<T>,
cleanup: (x?) => void,
): Promise<T> {
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'],
};
}
Loading