Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15,492 changes: 0 additions & 15,492 deletions package-lock.json

This file was deleted.

16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",
"dependencies": {
"@oclif/core": "3.27.0",
"@salesforce/code-analyzer-core": "0.26.0",
"@salesforce/code-analyzer-engine-api": "0.21.0",
"@salesforce/code-analyzer-eslint-engine": "0.21.0",
"@salesforce/code-analyzer-flow-engine": "0.19.0",
"@salesforce/code-analyzer-pmd-engine": "0.22.0",
"@salesforce/code-analyzer-regex-engine": "0.19.0",
"@salesforce/code-analyzer-retirejs-engine": "0.19.0",
"@salesforce/code-analyzer-sfge-engine": "0.2.0",
"@salesforce/code-analyzer-core": "0.27.0",
"@salesforce/code-analyzer-engine-api": "0.22.0",
"@salesforce/code-analyzer-eslint-engine": "0.22.0",
"@salesforce/code-analyzer-flow-engine": "0.20.0",
"@salesforce/code-analyzer-pmd-engine": "0.23.0",
"@salesforce/code-analyzer-regex-engine": "0.20.0",
"@salesforce/code-analyzer-retirejs-engine": "0.20.0",
"@salesforce/code-analyzer-sfge-engine": "0.3.1",
"@salesforce/core": "6.7.6",
"@salesforce/sf-plugins-core": "5.0.13",
"@salesforce/ts-types": "^2.0.12",
Expand Down
7 changes: 7 additions & 0 deletions src/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,10 @@ export enum View {
DETAIL = 'detail',
TABLE = 'table'
}

export const TelemetryEventName = 'plugin-code-analyzer';

export const CliTelemetryEvents = {
ENGINE_SELECTION: 'engine_selection',
ENGINE_EXECUTION: 'engine_execution'
}
2 changes: 2 additions & 0 deletions src/commands/code-analyzer/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {BundleName, getMessage, getMessages} from '../../lib/messages';
import {LogEventDisplayer} from '../../lib/listeners/LogEventListener';
import {EngineRunProgressSpinner, RuleSelectionProgressSpinner} from '../../lib/listeners/ProgressEventListener';
import {Displayable, UxDisplay} from '../../lib/Display';
import {SfCliTelemetryEmitter} from "../../lib/Telemetry";

export default class RunCommand extends SfCommand<void> implements Displayable {
// We don't need the `--json` output for this command.
Expand Down Expand Up @@ -96,6 +97,7 @@ export default class RunCommand extends SfCommand<void> implements Displayable {
configFactory: new CodeAnalyzerConfigFactoryImpl(),
pluginsFactory: new EnginePluginsFactoryImpl(),
writer: CompositeResultsWriter.fromFiles(outputFiles),
telemetryEmitter: new SfCliTelemetryEmitter(),
logEventListeners: [new LogEventDisplayer(uxDisplay)],
progressListeners: [new EngineRunProgressSpinner(uxDisplay), new RuleSelectionProgressSpinner(uxDisplay)],
resultsViewer,
Expand Down
17 changes: 17 additions & 0 deletions src/lib/Telemetry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import {TelemetryData} from '@salesforce/code-analyzer-core';
import {Lifecycle} from "@salesforce/core";

export interface TelemetryEmitter {
emitTelemetry(source: string, eventName: string, data: TelemetryData): void;
}

export class SfCliTelemetryEmitter implements TelemetryEmitter {
// istanbul ignore next - No sense in covering SF-CLI core code.
public emitTelemetry(source: string, eventName: string, data: TelemetryData): void {
return void Lifecycle.getInstance().emitTelemetry({
...data,
source,
eventName
});
}
}
28 changes: 28 additions & 0 deletions src/lib/actions/RunAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,16 @@ import {LogFileWriter} from '../writers/LogWriter';
import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener';
import {ProgressEventListener} from '../listeners/ProgressEventListener';
import {BundleName, getMessage} from '../messages';
import {TelemetryEmitter} from "../Telemetry";
import {TelemetryEventListener} from "../listeners/TelemetryEventListener";
import * as Constants from '../../Constants';

export type RunDependencies = {
configFactory: CodeAnalyzerConfigFactory;
pluginsFactory: EnginePluginsFactory;
logEventListeners: LogEventListener[];
progressListeners: ProgressEventListener[];
telemetryEmitter: TelemetryEmitter;
writer: ResultsWriter;
resultsViewer: ResultsViewer;
actionSummaryViewer: RunActionSummaryViewer;
Expand Down Expand Up @@ -56,6 +60,8 @@ export class RunAction {
// LogEventListeners should start listening as soon as the Core is instantiated, since Core can start emitting
// events they listen for basically immediately.
this.dependencies.logEventListeners.forEach(listener => listener.listen(core));
const telemetryListener: TelemetryEventListener = new TelemetryEventListener(this.dependencies.telemetryEmitter);
telemetryListener.listen(core);
const enginePlugins = this.dependencies.pluginsFactory.create();
const enginePluginModules = config.getCustomEnginePluginModules();
const addEnginePromises: Promise<void>[] = [
Expand All @@ -71,10 +77,12 @@ export class RunAction {
const ruleSelection: RuleSelection = await core.selectRules(input['rule-selector'], {workspace});
const runOptions: RunOptions = {workspace};
const results: RunResults = await core.run(ruleSelection, runOptions);
this.emitEngineTelemetry(ruleSelection, results, enginePlugins.flatMap(p => p.getAvailableEngineNames()));
// After Core is done running, the listeners need to be told to stop, since some of them have persistent UI elements
// or file handlers that must be gracefully ended.
this.dependencies.progressListeners.forEach(listener => listener.stopListening());
this.dependencies.logEventListeners.forEach(listener => listener.stopListening());
telemetryListener.stopListening();
this.dependencies.writer.write(results);
this.dependencies.resultsViewer.view(results);
this.dependencies.actionSummaryViewer.viewPostExecutionSummary(results, logWriter.getLogDestination(), input['output-file']);
Expand All @@ -88,6 +96,26 @@ export class RunAction {
public static createAction(dependencies: RunDependencies): RunAction {
return new RunAction(dependencies);
}

private emitEngineTelemetry(ruleSelection: RuleSelection, results: RunResults, coreEngineNames: string[]): void {
const selectedEngineNames: Set<string> = new Set(ruleSelection.getEngineNames());
for (const coreEngineName of coreEngineNames) {
if (!selectedEngineNames.has(coreEngineName)) {
continue;
}
Comment on lines +103 to +105
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need to pass in coreEngineNames. Let's just loop over selectedEngineNames. Then back on line 81... we don't need to get the engine names from the plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to use coreEngineNames in order to limit ourselves to the plugins that we ourselves ship. Otherwise, we're collecting telemetry on possibly-user-created engines, which is apparently a no-no.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh you are saying that enginePlugins which you get from the this.dependencies.pluginsFactory.create(); doesn't include the dynamic engines. OK makes sense.

this.dependencies.telemetryEmitter.emitTelemetry('RunAction', Constants.TelemetryEventName, {
sfcaEvent: Constants.CliTelemetryEvents.ENGINE_SELECTION,
engine: coreEngineName,
ruleCount: ruleSelection.getRulesFor(coreEngineName).length
});

this.dependencies.telemetryEmitter.emitTelemetry('RunAction', Constants.TelemetryEventName, {
sfcaEvent: Constants.CliTelemetryEvents.ENGINE_EXECUTION,
engine: coreEngineName,
violationCount: results.getEngineRunResults(coreEngineName).getViolationCount()
});
}
}
}

function throwErrorIfSevThresholdExceeded(threshold: SeverityLevel, results: RunResults): void {
Expand Down
35 changes: 35 additions & 0 deletions src/lib/listeners/TelemetryEventListener.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {
CodeAnalyzer,
EngineTelemetryEvent,
EventType,
TelemetryEvent
} from "@salesforce/code-analyzer-core";
import {TelemetryEmitter} from '../Telemetry';
import * as constants from '../../Constants';

export class TelemetryEventListener {
private telemetryEmitter: TelemetryEmitter;

public constructor(telemetryEmitter: TelemetryEmitter) {
this.telemetryEmitter = telemetryEmitter;
}

public listen(codeAnalyzer: CodeAnalyzer): void {
// Set up listeners.
codeAnalyzer.onEvent(EventType.TelemetryEvent, (e: TelemetryEvent) => this.handleEvent('Core', e));
codeAnalyzer.onEvent(EventType.EngineTelemetryEvent, (e: EngineTelemetryEvent) => this.handleEvent(e.engineName, e));
}

public stopListening(): void {
// Intentional no-op, because no cleanup is required.
}

private handleEvent(source: string, event: TelemetryEvent|EngineTelemetryEvent): void {
return this.telemetryEmitter.emitTelemetry(source, constants.TelemetryEventName, {
...event.data,
sfcaEvent: event.eventName,
timestamp: event.timestamp.getTime(),
uuid: event.uuid
});
}
}
22 changes: 22 additions & 0 deletions test/commands/code-analyzer/run.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
import {stubSfCommandUx} from '@salesforce/sf-plugins-core';
import {TelemetryData} from '@salesforce/code-analyzer-core';
import {TestContext} from '@salesforce/core/lib/testSetup';
import RunCommand from '../../../src/commands/code-analyzer/run';
import {RunAction, RunDependencies, RunInput} from '../../../src/lib/actions/RunAction';
import {CompositeResultsWriter} from '../../../src/lib/writers/ResultsWriter';
import {SfCliTelemetryEmitter} from "../../../src/lib/Telemetry";

type TelemetryEmission = {
source: string,
eventName: string,
data: TelemetryData
};

describe('`code-analyzer run` tests', () => {
const $$ = new TestContext();

let executeSpy: jest.SpyInstance;
let createActionSpy: jest.SpyInstance;
let receivedTelemetryEmissions: TelemetryEmission[];
let receivedActionInput: RunInput;
let receivedActionDependencies: RunDependencies;
let fromFilesSpy: jest.SpyInstance;
Expand All @@ -19,6 +28,11 @@ describe('`code-analyzer run` tests', () => {
receivedActionInput = input;
return Promise.resolve();
});
receivedTelemetryEmissions = [];
jest.spyOn(SfCliTelemetryEmitter.prototype, 'emitTelemetry').mockImplementation((source, eventName, data) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know this isn't the preferred way of doing this sort of test, but it was the best option available.

receivedTelemetryEmissions.push({source, eventName, data});
return Promise.resolve();
});
const originalCreateAction = RunAction.createAction;
createActionSpy = jest.spyOn(RunAction, 'createAction').mockImplementation((dependencies) => {
receivedActionDependencies = dependencies;
Expand Down Expand Up @@ -325,6 +339,14 @@ describe('`code-analyzer run` tests', () => {
});
});

describe('Telemetry emission', () => {
it('Passes telemetry emitter through into Action layer', async () => {
await RunCommand.run([]);
expect(createActionSpy).toHaveBeenCalled();
expect(receivedActionDependencies.telemetryEmitter!.constructor.name).toEqual('SfCliTelemetryEmitter');
});
});

describe('Flag interactions', () => {
describe('--output-file and --view', () => {
it('When --output-file and --view are both present, both are used', async () => {
Expand Down
44 changes: 44 additions & 0 deletions test/lib/actions/RunAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
StubEnginePluginsFactory_withPreconfiguredStubEngines,
StubEnginePluginsFactory_withThrowingStubPlugin
} from '../../stubs/StubEnginePluginsFactories';
import {SpyTelemetryEmitter} from "../../stubs/SpyTelemetryEmitter";

const PATH_TO_FILE_A = path.resolve('test', 'sample-code', 'fileA.cls');
const PATH_TO_GOLDFILES = path.join(__dirname, '..', '..', 'fixtures', 'comparison-files', 'lib', 'actions', 'RunAction.test.ts');
Expand Down Expand Up @@ -49,6 +50,7 @@ describe('RunAction tests', () => {
pluginsFactory: pluginsFactory,
logEventListeners: [],
progressListeners: [],
telemetryEmitter: new SpyTelemetryEmitter(),
writer,
resultsViewer,
actionSummaryViewer
Expand Down Expand Up @@ -230,6 +232,7 @@ describe('RunAction tests', () => {
pluginsFactory: new StubEnginePluginsFactory_withThrowingStubPlugin(),
logEventListeners: [],
progressListeners: [],
telemetryEmitter: new SpyTelemetryEmitter(),
writer,
resultsViewer,
actionSummaryViewer
Expand Down Expand Up @@ -363,6 +366,47 @@ describe('RunAction tests', () => {
expect(displayedLogEvents).toContain(goldfileContents);
});
});

describe('Telemetry Emission', () => {
it('When a telemetry emitter is provided, it is used', async () => {
// ==== SETUP ====
// Create a telemetry emitter and set it to be used.
const spyTelemetryEmitter: SpyTelemetryEmitter = new SpyTelemetryEmitter();
dependencies.telemetryEmitter = spyTelemetryEmitter;
// Create the input.
const input: RunInput = {
// Select all rules.
'rule-selector': ['all'],
// Use the current directory, for convenience.
'workspace': ['.'],
// Outfiles can just be an empty list.
'output-file': []
};
// ==== TESTED BEHAVIOR ====
await action.execute(input);

// ==== ASSERTIONS ====
expect(spyTelemetryEmitter.getCapturedTelemetry()).toHaveLength(4);

expect(spyTelemetryEmitter.getCapturedTelemetry()[0].eventName).toEqual('plugin-code-analyzer');
expect(spyTelemetryEmitter.getCapturedTelemetry()[0].source).toEqual('stubEngine1');
expect(spyTelemetryEmitter.getCapturedTelemetry()[0].data.sfcaEvent).toEqual('engine1DescribeTelemetry');

expect(spyTelemetryEmitter.getCapturedTelemetry()[1].eventName).toEqual('plugin-code-analyzer');
expect(spyTelemetryEmitter.getCapturedTelemetry()[1].source).toEqual('stubEngine1');
expect(spyTelemetryEmitter.getCapturedTelemetry()[1].data.sfcaEvent).toEqual('engine1ExecuteTelemetry');

expect(spyTelemetryEmitter.getCapturedTelemetry()[2].eventName).toEqual('plugin-code-analyzer');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].source).toEqual('RunAction');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].data.sfcaEvent).toEqual('engine_selection');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].data.ruleCount).toEqual(5);

expect(spyTelemetryEmitter.getCapturedTelemetry()[3].eventName).toEqual('plugin-code-analyzer');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].source).toEqual('RunAction');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].data.sfcaEvent).toEqual('engine_execution');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].data.violationCount).toEqual(0);
});
})
});

// TODO: Whenever we decide to document the custom_engine_plugin_modules flag in our configuration file, then we'll want
Expand Down
20 changes: 20 additions & 0 deletions test/stubs/SpyTelemetryEmitter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {TelemetryData} from '@salesforce/code-analyzer-core';
import {TelemetryEmitter} from '../../src/lib/Telemetry';

export type CapturedTelemetryEmission = {
source: string,
eventName: string,
data: TelemetryData
};

export class SpyTelemetryEmitter implements TelemetryEmitter {
private capturedTelemetry: CapturedTelemetryEmission[] = [];

public emitTelemetry(source: string, eventName: string, data: TelemetryData): void {
this.capturedTelemetry.push({source, eventName, data});
}

public getCapturedTelemetry(): CapturedTelemetryEmission[] {
return this.capturedTelemetry;
}
}
6 changes: 6 additions & 0 deletions test/stubs/StubEnginePlugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ export class StubEngine1 extends EngineApi.Engine {
type: EngineApi.EventType.DescribeRulesProgressEvent,
percentComplete: 0
});
this.emitTelemetryEvent('engine1DescribeTelemetry', {
someArg: true
});
this.emitEvent<EngineApi.LogEvent>({
type: EngineApi.EventType.LogEvent,
message: "someMiscFineMessageFromStubEngine1",
Expand Down Expand Up @@ -138,6 +141,9 @@ export class StubEngine1 extends EngineApi.Engine {
type: EngineApi.EventType.RunRulesProgressEvent,
percentComplete: 0
});
this.emitTelemetryEvent('engine1ExecuteTelemetry', {
someArg: true
});
this.emitEvent<EngineApi.LogEvent>({
type: EngineApi.EventType.LogEvent,
message: "someMiscFineMessageFromStubEngine1",
Expand Down