Skip to content

FIX: @W-19149943@: Add in missing telemetry when rules command runs #1862

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

Merged
merged 2 commits into from
Jul 28, 2025
Merged

FIX: @W-19149943@: Add in missing telemetry when rules command runs #1862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fba0b4e
FIX: @W-19149943@: Add in missing telemetry when rules command runs
stephen-carter-at-sf Jul 28, 2025
1dc141c
Lock in custom engine telementry events
stephen-carter-at-sf Jul 28, 2025
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
2 changes: 2 additions & 0 deletions src/commands/code-analyzer/rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {Displayable, UxDisplay} from '../../lib/Display';
import {LogEventDisplayer} from '../../lib/listeners/LogEventListener';
import {RuleSelectionProgressSpinner} from '../../lib/listeners/ProgressEventListener';
import {CompositeRulesWriter} from '../../lib/writers/RulesWriter';
import { SfCliTelemetryEmitter } from '../../lib/Telemetry';

export default class RulesCommand extends SfCommand<void> implements Displayable {
// We don't need the `--json` output for this command.
Expand Down Expand Up @@ -86,6 +87,7 @@ export default class RulesCommand extends SfCommand<void> implements Displayable
pluginsFactory: new EnginePluginsFactoryImpl(),
logEventListeners: [new LogEventDisplayer(uxDisplay)],
progressListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
telemetryEmitter: new SfCliTelemetryEmitter(),
actionSummaryViewer: new RulesActionSummaryViewer(uxDisplay),
viewer: this.createRulesViewer(view, outputFiles, uxDisplay),
writer: CompositeRulesWriter.fromFiles(outputFiles)
Expand Down
22 changes: 22 additions & 0 deletions src/lib/actions/RulesAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@ import { RulesActionSummaryViewer } from '../viewers/ActionSummaryViewer';
import { RuleViewer } from '../viewers/RuleViewer';
import { LogFileWriter } from '../writers/LogWriter';
import { RulesWriter } from '../writers/RulesWriter';
import { TelemetryEmitter } from '../Telemetry';
import { TelemetryEventListener } from '../listeners/TelemetryEventListener';
import * as Constants from '../../Constants';

export type RulesDependencies = {
configFactory: CodeAnalyzerConfigFactory;
pluginsFactory: EnginePluginsFactory;
logEventListeners: LogEventListener[];
progressListeners: ProgressEventListener[];
telemetryEmitter: TelemetryEmitter;
actionSummaryViewer: RulesActionSummaryViewer,
viewer: RuleViewer;
writer: RulesWriter;
Expand Down Expand Up @@ -45,6 +49,8 @@ export class RulesAction {
// 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 @@ -60,10 +66,12 @@ export class RulesAction {
// that's when progress events can start being emitted.
this.dependencies.progressListeners.forEach(listener => listener.listen(core));
const ruleSelection: RuleSelection = await core.selectRules(input["rule-selector"], selectOptions);
this.emitEngineTelemetry(ruleSelection, 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();
const rules: Rule[] = core.getEngineNames().flatMap(name => ruleSelection.getRulesFor(name));

this.dependencies.writer.write(ruleSelection)
Expand All @@ -79,4 +87,18 @@ export class RulesAction {
public static createAction(dependencies: RulesDependencies): RulesAction {
return new RulesAction(dependencies);
}

private emitEngineTelemetry(ruleSelection: RuleSelection, coreEngineNames: string[]): void {
const selectedEngineNames: Set<string> = new Set(ruleSelection.getEngineNames());
for (const coreEngineName of coreEngineNames) {
if (!selectedEngineNames.has(coreEngineName)) {
continue;
}
this.dependencies.telemetryEmitter.emitTelemetry(Constants.TelemetrySource, Constants.TelemetryEventName, {
sfcaEvent: Constants.CliTelemetryEvents.ENGINE_SELECTION,
engine: coreEngineName,
ruleCount: ruleSelection.getRulesFor(coreEngineName).length
});
}
}
}
90 changes: 65 additions & 25 deletions test/lib/actions/RulesAction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import { SpyRuleViewer } from '../../stubs/SpyRuleViewer';
import { SpyRuleWriter } from '../../stubs/SpyRuleWriter';
import { StubDefaultConfigFactory } from '../../stubs/StubCodeAnalyzerConfigFactories';
import * as StubEnginePluginFactories from '../../stubs/StubEnginePluginsFactories';
import { CapturedTelemetryEmission, SpyTelemetryEmitter } from '../../stubs/SpyTelemetryEmitter';

const PATH_TO_GOLDFILES = path.join(__dirname, '..', '..', 'fixtures', 'comparison-files', 'lib', 'actions', 'RulesAction.test.ts');

describe('RulesAction tests', () => {
let viewer: SpyRuleViewer;
let writer: SpyRuleWriter;
let spyDisplay: SpyDisplay;
let spyTelemetryEmitter: SpyTelemetryEmitter;
let actionSummaryViewer: RulesActionSummaryViewer;
let defaultDependencies: RulesDependencies;

Expand All @@ -23,11 +25,13 @@ describe('RulesAction tests', () => {
writer = new SpyRuleWriter();
spyDisplay = new SpyDisplay();
actionSummaryViewer = new RulesActionSummaryViewer(spyDisplay);
spyTelemetryEmitter = new SpyTelemetryEmitter();
defaultDependencies = {
configFactory: new StubDefaultConfigFactory(),
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withFunctionalStubEngine(),
logEventListeners: [],
progressListeners: [],
telemetryEmitter: spyTelemetryEmitter,
actionSummaryViewer,
viewer,
writer
Expand Down Expand Up @@ -123,14 +127,8 @@ describe('RulesAction tests', () => {
}
])('When $case, only the relevant rules are returned', async ({workspace, target}) => {
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
// The engine we're using here will synthesize one rule per target.
...defaultDependencies,
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withTargetDependentStubEngine(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer,
writer
};
const action = RulesAction.createAction(dependencies);
const input: RulesInput = {
Expand Down Expand Up @@ -161,13 +159,8 @@ describe('RulesAction tests', () => {
*/
it('When no engines are registered, empty results are displayed', async () => {
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
...defaultDependencies,
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withNoPlugins(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer,
writer
};
const action = RulesAction.createAction(dependencies);
const input: RulesInput = {
Expand All @@ -183,13 +176,8 @@ describe('RulesAction tests', () => {

it('Throws an error when an engine throws an error', async () => {
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
...defaultDependencies,
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withThrowingStubPlugin(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer,
writer
};
const action = RulesAction.createAction(dependencies);
const input: RulesInput = {
Expand All @@ -214,13 +202,9 @@ describe('RulesAction tests', () => {
])('When $quantifier rules are returned, $expectation', async ({selector, goldfile}) => {
const goldfilePath: string = path.join(PATH_TO_GOLDFILES, 'action-summaries', goldfile);
const dependencies: RulesDependencies = {
configFactory: new StubDefaultConfigFactory(),
...defaultDependencies,
pluginsFactory: new StubEnginePluginFactories.StubEnginePluginsFactory_withFunctionalStubEngine(),
logEventListeners: [],
progressListeners: [],
actionSummaryViewer,
viewer,
writer
viewer
};
const action = RulesAction.createAction(dependencies);
const input: RulesInput = {
Expand Down Expand Up @@ -266,6 +250,62 @@ describe('RulesAction tests', () => {
expect(displayedLogEvents).toContain(goldfileContents);
});
});

describe('Telemetry Emission', () => {
it('When a telemetry emitter is provided, it is used', async () => {

defaultDependencies.pluginsFactory = new StubEnginePluginFactories.StubEnginePluginsFactory_withFunctionalStubEngine();
const action = RulesAction.createAction(defaultDependencies);
// Create the input.
const input: RulesInput = {
'rule-selector': ['all']
};
// ==== TESTED BEHAVIOR ====
await action.execute(input);

// ==== ASSERTIONS ====
const allTelemEvents: CapturedTelemetryEmission[] = spyTelemetryEmitter.getCapturedTelemetry();
const ruleSelectionTelemEvents: CapturedTelemetryEmission[] = allTelemEvents.filter(
e => e.data.sfcaEvent === 'engine_selection');

expect(ruleSelectionTelemEvents).toHaveLength(2);
expect(ruleSelectionTelemEvents[0]).toEqual({
"data": {
"engine": "stubEngine1",
"ruleCount": 5,
"sfcaEvent": "engine_selection"
},
"eventName": "plugin-code-analyzer",
"source": "CLI" // NOTE: We might move these events to Core in the future instead of the CLI
});
expect(ruleSelectionTelemEvents[1]).toEqual({
"data": {
"engine": "stubEngine2",
"ruleCount": 3,
"sfcaEvent": "engine_selection"
},
"eventName": "plugin-code-analyzer",
"source": "CLI" // NOTE: We might move these events to Core in the future instead of the CLI
});

const engineSpecificTelemEvents: CapturedTelemetryEmission[] = allTelemEvents.filter(
e => e.data.sfcaEvent === 'engine1DescribeTelemetry');
expect(engineSpecificTelemEvents).toHaveLength(1);
const customEvent: CapturedTelemetryEmission = engineSpecificTelemEvents[0];
customEvent.data["timestamp"] = 0; // fix for deterministic testing
customEvent.data["uuid"] = "someUUID"; // fix for deterministic testing
expect(customEvent).toEqual({
"data": {
"someArg": true, // argument set by engine
"sfcaEvent": "engine1DescribeTelemetry",
"timestamp": 0,
"uuid": "someUUID"
},
"eventName": "plugin-code-analyzer",
"source": "stubEngine1"
});
});
})
});

// TODO: Whenever we decide to document the custom_engine_plugin_modules flag in our configuration file, then we'll want
Expand Down
Loading