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
6 changes: 6 additions & 0 deletions src/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ export const CliTelemetryEvents = {
ENGINE_SELECTION: 'engine_selection',
ENGINE_EXECUTION: 'engine_execution'
}

export const CliCommands = {
RUN: 'run',
RULES: 'rules',
CONFIG: 'config'
}
2 changes: 2 additions & 0 deletions src/commands/code-analyzer/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {BundleName, getMessage, getMessages} from '../../lib/messages.js';
import {LogEventDisplayer} from '../../lib/listeners/LogEventListener.js';
import {RuleSelectionProgressSpinner} from '../../lib/listeners/ProgressEventListener.js';
import {Displayable, UxDisplay} from '../../lib/Display.js';
import {SfCliTelemetryEmitter} from '../../lib/Telemetry.js';

export default class ConfigCommand extends SfCommand<void> implements Displayable {
// We don't need the `--json` output for this command.
Expand Down Expand Up @@ -76,6 +77,7 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
const dependencies: ConfigDependencies = {
configFactory: new CodeAnalyzerConfigFactoryImpl(),
pluginsFactory: new EnginePluginsFactoryImpl(),
telemetryEmitter: new SfCliTelemetryEmitter(),
logEventListeners: [new LogEventDisplayer(uxDisplay)],
progressEventListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
actionSummaryViewer: new ConfigActionSummaryViewer(uxDisplay),
Expand Down
26 changes: 26 additions & 0 deletions src/lib/actions/ConfigAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@ import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener.js
import {ProgressEventListener} from '../listeners/ProgressEventListener.js';
import {ConfigActionSummaryViewer} from '../viewers/ActionSummaryViewer.js';
import {AnnotatedConfigModel, ConfigModel} from '../models/ConfigModel.js';
import {TelemetryEmitter} from '../Telemetry.js';
import {TelemetryEventListener} from '../listeners/TelemetryEventListener.js';
import * as Constants from '../../Constants.js';

export type ConfigDependencies = {
configFactory: CodeAnalyzerConfigFactory;
pluginsFactory: EnginePluginsFactory;
logEventListeners: LogEventListener[];
progressEventListeners: ProgressEventListener[];
telemetryEmitter: TelemetryEmitter;
writer?: ConfigWriter;
actionSummaryViewer: ConfigActionSummaryViewer;
viewer: ConfigViewer;
Expand Down Expand Up @@ -54,6 +58,8 @@ export class ConfigAction {
// LogEventListeners should start listening as soon as the User Core is instantiated, since it can start emitting
// relevant events basically immediately.
this.dependencies.logEventListeners.forEach(listener => listener.listen(userCore));
const telemetryListener: TelemetryEventListener = new TelemetryEventListener(this.dependencies.telemetryEmitter);
telemetryListener.listen(userCore);

const enginePlugins: EnginePlugin[] = this.dependencies.pluginsFactory.create();
const enginePluginModules: string[] = userConfig.getCustomEnginePluginModules();
Expand Down Expand Up @@ -128,7 +134,10 @@ export class ConfigAction {
// elements or file handlers that must be gracefully ended.
this.dependencies.progressEventListeners.forEach(listener => listener.stopListening());
this.dependencies.logEventListeners.forEach(listener => listener.stopListening());
telemetryListener.stopListening();

// ==== EMIT TELEMETRY ==========================================================================================
this.emitEngineTelemetry(enginePlugins, userCore, userRules);

// ==== CREATE AND WRITE CONFIG YAML ===========================================================================

Expand All @@ -154,6 +163,23 @@ export class ConfigAction {
return Promise.resolve();
}

private emitEngineTelemetry(enginePlugins: EnginePlugin[], core: CodeAnalyzer, ruleSelection: {getEngineNames(): string[], getRulesFor(engine: string): unknown[]}): void {
const coreEngineNames: string[] = enginePlugins.flatMap(enginePlugin => enginePlugin.getAvailableEngineNames());
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,
command: Constants.CliCommands.CONFIG,
engine: coreEngineName,
ruleCount: ruleSelection.getRulesFor(coreEngineName).length
});
Comment on lines +174 to +179
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code flow does not seems to be in the path of the config cli command.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in my understanding

This code is definitely in the execution path of the config CLI command. The ConfigAction.execute() method is the main entry point when running sf code-analyzer config, and emitEngineTelemetry() is called directly from within execute() after the rule selection
completes but before the method returns.

The config command performs rule selection internally (via userCore.selectRules()) to determine which rules will be included in the generated config file - this is the core functionality of the command. After the selection completes, we have the actual data about
which engines were selected and how many rules from each engine, so we emit telemetry at that point. This follows the same pattern as the run and rules commands - emit telemetry after you have the selection results.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense, sorry I got confused here with engine selection.

}
}

public static createAction(dependencies: ConfigDependencies): ConfigAction {
return new ConfigAction(dependencies)
}
Expand Down
1 change: 1 addition & 0 deletions src/lib/actions/RulesAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export class RulesAction {
}
this.dependencies.telemetryEmitter.emitTelemetry(Constants.TelemetrySource, Constants.TelemetryEventName, {
sfcaEvent: Constants.CliTelemetryEvents.ENGINE_SELECTION,
command: Constants.CliCommands.RULES,
engine: coreEngineName,
ruleCount: ruleSelection.getRulesFor(coreEngineName).length
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/actions/RunAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,14 @@ export class RunAction {
}
this.dependencies.telemetryEmitter.emitTelemetry(Constants.TelemetrySource, Constants.TelemetryEventName, {
sfcaEvent: Constants.CliTelemetryEvents.ENGINE_SELECTION,
command: Constants.CliCommands.RUN,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will we get 2 run command telemetry events for ENGINE_SELECTION and ENGINE_EXECUTION? If only the command is reflected in the pft events we will see 2 run metrics for one execution of the command and and since no sfcaEvent is there we will not be able to distinguish b/w the one for ENGINE_SELECTION and the one for ENGINE_EXECUTION. Is this expected behaviour?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this is not a problem. The concern is based on a misunderstanding.

What's actually emitted:

For a single run command execution, you get 2 telemetry events per engine, and both events include BOTH the sfcaEvent and command fields:

Event 1 - ENGINE_SELECTION:
{
sfcaEvent: 'engine_selection', // ← Identifies this as selection event
command: 'run', // ← Identifies which command
engine: 'pmd',
ruleCount: 50
}

Event 2 - ENGINE_EXECUTION:
{
sfcaEvent: 'engine_execution', // ← Identifies this as execution event
command: 'run', // ← Identifies which command
engine: 'pmd',
violationCount: 10
}

Why you CAN distinguish them:

  • The sfcaEvent field tells you whether it's engine_selection or engine_execution
  • The command field tells you whether it came from run, rules, or config

This is expected and correct behavior:

  • run command: emits ENGINE_SELECTION + ENGINE_EXECUTION (2 events per engine)
  • rules command: emits ENGINE_SELECTION only (1 event per engine)
  • config command: emits ENGINE_SELECTION only (1 event per engine)

The sfcaEvent field has always been there to distinguish between selection and execution. The new command field just adds another dimension to identify which CLI command was used.

engine: coreEngineName,
ruleCount: ruleSelection.getRulesFor(coreEngineName).length
});

this.dependencies.telemetryEmitter.emitTelemetry(Constants.TelemetrySource, Constants.TelemetryEventName, {
sfcaEvent: Constants.CliTelemetryEvents.ENGINE_EXECUTION,
command: Constants.CliCommands.RUN,
engine: coreEngineName,
violationCount: results.getEngineRunResults(coreEngineName).getViolationCount()
});
Expand Down
22 changes: 15 additions & 7 deletions test/lib/actions/ConfigAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {SpyConfigWriter} from '../../stubs/SpyConfigWriter.js';
import {SpyConfigViewer} from '../../stubs/SpyConfigViewer.js';
import {DisplayEvent, DisplayEventType, SpyDisplay} from '../../stubs/SpyDisplay.js';
import {LogEventDisplayer} from '../../../src/lib/listeners/LogEventListener.js';
import {SpyTelemetryEmitter} from '../../stubs/SpyTelemetryEmitter.js';

const PATH_TO_FIXTURES = path.join(import.meta.dirname, '..', '..', 'fixtures');

Expand All @@ -38,7 +39,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new StubCodeAnalyzerConfigFactory(),
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
pluginsFactory: new StubEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
};
});

Expand Down Expand Up @@ -177,7 +179,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: stubConfigFactory,
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
pluginsFactory: new StubEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
};
});

Expand Down Expand Up @@ -447,7 +450,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: configFactory,
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
pluginsFactory: new StubEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
};
});

Expand Down Expand Up @@ -531,7 +535,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new StubCodeAnalyzerConfigFactory(),
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
pluginsFactory: new StubEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
};
});

Expand Down Expand Up @@ -687,7 +692,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new StubCodeAnalyzerConfigFactory(),
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new WorkspaceAwareEnginePluginFactory()
pluginsFactory: new WorkspaceAwareEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
};

// ==== TESTED BEHAVIOR ====
Expand All @@ -708,7 +714,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new StubCodeAnalyzerConfigFactory(),
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
pluginsFactory: new StubEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
};
});

Expand Down Expand Up @@ -756,7 +763,8 @@ describe('ConfigAction tests', () => {
viewer: new ConfigStyledYamlViewer(spyDisplay),
configFactory: new StubCodeAnalyzerConfigFactory(),
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
pluginsFactory: new StubEnginePluginFactory()
pluginsFactory: new StubEnginePluginFactory(),
telemetryEmitter: new SpyTelemetryEmitter()
}
});

Expand Down
10 changes: 6 additions & 4 deletions test/lib/actions/RulesAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,18 +271,20 @@ describe('RulesAction tests', () => {
expect(ruleSelectionTelemEvents).toHaveLength(2);
expect(ruleSelectionTelemEvents[0]).toEqual({
"data": {
"sfcaEvent": "engine_selection",
"command": "rules",
"engine": "stubEngine1",
"ruleCount": 5,
"sfcaEvent": "engine_selection"
"ruleCount": 5
},
"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": {
"sfcaEvent": "engine_selection",
"command": "rules",
"engine": "stubEngine2",
"ruleCount": 3,
"sfcaEvent": "engine_selection"
"ruleCount": 3
},
"eventName": "plugin-code-analyzer",
"source": "CLI" // NOTE: We might move these events to Core in the future instead of the CLI
Expand Down
2 changes: 2 additions & 0 deletions test/lib/actions/RunAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,13 @@ describe('RunAction tests', () => {
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].eventName).toEqual('plugin-code-analyzer');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].source).toEqual('CLI');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].data.sfcaEvent).toEqual('engine_selection');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].data.command).toEqual('run');
expect(spyTelemetryEmitter.getCapturedTelemetry()[2].data.ruleCount).toEqual(5);

expect(spyTelemetryEmitter.getCapturedTelemetry()[3].eventName).toEqual('plugin-code-analyzer');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].source).toEqual('CLI');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].data.sfcaEvent).toEqual('engine_execution');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].data.command).toEqual('run');
expect(spyTelemetryEmitter.getCapturedTelemetry()[3].data.violationCount).toEqual(0);
});
})
Expand Down
Loading