Merge pull request #2843 from BPapp-MS/benpapp/AddFlagsToRushTelemetry

[rush-lib] Add flags for parameters to rush telemetry

Author: iclanton

apps/rush-lib/src/cli/actions/BaseInstallAction.ts

@@ -21,6 +21,7 @@ import { Stopwatch } from '../../utilities/Stopwatch';
 import { VersionMismatchFinder } from '../../logic/versionMismatch/VersionMismatchFinder';
 import { Variants } from '../../api/Variants';
 import { RushConstants } from '../../logic/RushConstants';
+import { SelectionParameterSet } from '../SelectionParameterSet';
 
 const installManagerFactoryModule: typeof import('../../logic/InstallManagerFactory') = Import.lazy(
   '../../logic/InstallManagerFactory',
@@ -39,6 +40,11 @@ export abstract class BaseInstallAction extends BaseRushAction {
   protected _debugPackageManagerParameter!: CommandLineFlagParameter;
   protected _maxInstallAttempts!: CommandLineIntegerParameter;
   protected _ignoreHooksParameter!: CommandLineFlagParameter;
+  /*
+   * Subclasses can initialize the _selectionParameters property in order for
+   * the parameters to be written to the telemetry file
+   */
+  protected _selectionParameters?: SelectionParameterSet;
 
   protected onDefineParameters(): void {
     this._purgeParameter = this.defineFlagParameter({
@@ -181,15 +187,19 @@ export abstract class BaseInstallAction extends BaseRushAction {
     success: boolean
   ): void {
     if (this.parser.telemetry) {
+      const extraData: Record<string, string> = {
+        mode: this.actionName,
+        clean: (!!this._purgeParameter.value).toString(),
+        debug: installManagerOptions.debug.toString(),
+        full: installManagerOptions.fullUpgrade.toString(),
+        ...this.getParameterStringMap(),
+        ...this._selectionParameters?.getTelemetry()
+      };
       this.parser.telemetry.log({
         name: 'install',
         duration: stopwatch.duration,
         result: success ? 'Succeeded' : 'Failed',
-        extraData: {
-          mode: this.actionName,
-          clean: (!!this._purgeParameter.value).toString(),
-          full: installManagerOptions.fullUpgrade.toString()
-        }
+        extraData
       });
     }
   }

apps/rush-lib/src/cli/actions/InstallAction.ts

@@ -7,8 +7,6 @@ import { RushCommandLineParser } from '../RushCommandLineParser';
 import { SelectionParameterSet } from '../SelectionParameterSet';
 
 export class InstallAction extends BaseInstallAction {
-  protected _selectionParameters!: SelectionParameterSet;
-
   public constructor(parser: RushCommandLineParser) {
     super({
       actionName: 'install',
@@ -52,7 +50,7 @@ export class InstallAction extends BaseInstallAction {
       // it is safe to assume that the value is not null
       maxInstallAttempts: this._maxInstallAttempts.value!,
       // These are derived independently of the selection for command line brevity
-      pnpmFilterArguments: this._selectionParameters.getPnpmFilterArguments()
+      pnpmFilterArguments: this._selectionParameters!.getPnpmFilterArguments()
     };
   }
 }

apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts

@@ -5,11 +5,7 @@ import * as os from 'os';
 import colors from 'colors/safe';
 
 import { AlreadyReportedError, ConsoleTerminalProvider, Terminal } from '@rushstack/node-core-library';
-import {
-  CommandLineFlagParameter,
-  CommandLineStringParameter,
-  CommandLineParameterKind
-} from '@rushstack/ts-command-line';
+import { CommandLineFlagParameter, CommandLineStringParameter } from '@rushstack/ts-command-line';
 
 import { Event } from '../../index';
 import { SetupChecks } from '../../logic/SetupChecks';
@@ -381,21 +377,10 @@ export class BulkScriptAction extends BaseScriptAction {
   }
 
   private _collectTelemetry(stopwatch: Stopwatch, success: boolean): void {
-    const extraData: { [key: string]: string } = this._selectionParameters.getTelemetry();
-
-    for (const customParameter of this.customParameters) {
-      switch (customParameter.kind) {
-        case CommandLineParameterKind.Flag:
-        case CommandLineParameterKind.Choice:
-        case CommandLineParameterKind.String:
-        case CommandLineParameterKind.Integer:
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-          extraData[customParameter.longName] = JSON.stringify((customParameter as any).value);
-          break;
-        default:
-          extraData[customParameter.longName] = '?';
-      }
-    }
+    const extraData: Record<string, string> = {
+      ...this._selectionParameters.getTelemetry(),
+      ...this.getParameterStringMap()
+    };
 
     if (this.parser.telemetry) {
       this.parser.telemetry.log({

common/changes/@microsoft/rush/benpapp-AddFlagsToRushTelemetry_2021-08-05-20-52.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Add properties to the extraData section of the telemetry file for parameter usage in the install commands",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "[email protected]"
+}

common/changes/@rushstack/ts-command-line/benpapp-AddFlagsToRushTelemetry_2021-08-17-23-35.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/ts-command-line",
+      "comment": "Add getParameterStringMap to CommandLineParameterProvider, to easily query parameter usage for telemetry",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/ts-command-line",
+  "email": "[email protected]"
+}

common/reviews/api/ts-command-line.api.md

@@ -36,7 +36,7 @@ export class CommandLineChoiceListParameter extends CommandLineParameter {
     // @internal
     _setValue(data: any): void;
     get values(): ReadonlyArray<string>;
-    }
+}
 
 // @public
 export class CommandLineChoiceParameter extends CommandLineParameter {
@@ -53,7 +53,7 @@ export class CommandLineChoiceParameter extends CommandLineParameter {
     // @internal
     _setValue(data: any): void;
     get value(): string | undefined;
-    }
+}
 
 // @public
 export const enum CommandLineConstants {
@@ -70,7 +70,7 @@ export class CommandLineFlagParameter extends CommandLineParameter {
     // @internal
     _setValue(data: any): void;
     get value(): boolean;
-    }
+}
 
 // @public
 export class CommandLineHelper {
@@ -87,7 +87,7 @@ export class CommandLineIntegerListParameter extends CommandLineParameterWithArg
     // @internal
     _setValue(data: any): void;
     get values(): ReadonlyArray<number>;
-    }
+}
 
 // @public
 export class CommandLineIntegerParameter extends CommandLineParameterWithArgument {
@@ -102,7 +102,7 @@ export class CommandLineIntegerParameter extends CommandLineParameterWithArgumen
     // @internal
     _setValue(data: any): void;
     get value(): number | undefined;
-    }
+}
 
 // @public
 export abstract class CommandLineParameter {
@@ -157,6 +157,7 @@ export abstract class CommandLineParameterProvider {
     getFlagParameter(parameterLongName: string): CommandLineFlagParameter;
     getIntegerListParameter(parameterLongName: string): CommandLineIntegerListParameter;
     getIntegerParameter(parameterLongName: string): CommandLineIntegerParameter;
+    getParameterStringMap(): Record<string, string>;
     getStringListParameter(parameterLongName: string): CommandLineStringListParameter;
     getStringParameter(parameterLongName: string): CommandLineStringParameter;
     protected abstract onDefineParameters(): void;
@@ -173,7 +174,7 @@ export abstract class CommandLineParameterWithArgument extends CommandLineParame
     constructor(definition: IBaseCommandLineDefinitionWithArgument);
     readonly argumentName: string;
     readonly completions: (() => Promise<string[]>) | undefined;
-    }
+}
 
 // @public
 export abstract class CommandLineParser extends CommandLineParameterProvider {
@@ -188,7 +189,7 @@ export abstract class CommandLineParser extends CommandLineParameterProvider {
     protected onExecute(): Promise<void>;
     selectedAction: CommandLineAction | undefined;
     tryGetAction(actionName: string): CommandLineAction | undefined;
-    }
+}
 
 // @public
 export class CommandLineRemainder {
@@ -200,7 +201,7 @@ export class CommandLineRemainder {
     // @internal
     _setValue(data: any): void;
     get values(): ReadonlyArray<string>;
-    }
+}
 
 // @public
 export class CommandLineStringListParameter extends CommandLineParameterWithArgument {
@@ -212,7 +213,7 @@ export class CommandLineStringListParameter extends CommandLineParameterWithArgu
     // @internal
     _setValue(data: any): void;
     get values(): ReadonlyArray<string>;
-    }
+}
 
 // @public
 export class CommandLineStringParameter extends CommandLineParameterWithArgument {
@@ -227,7 +228,7 @@ export class CommandLineStringParameter extends CommandLineParameterWithArgument
     // @internal
     _setValue(data: any): void;
     get value(): string | undefined;
-    }
+}
 
 // @public (undocumented)
 export class DynamicCommandLineAction extends CommandLineAction {
@@ -321,5 +322,4 @@ export interface ICommandLineStringDefinition extends IBaseCommandLineDefinition
 export interface ICommandLineStringListDefinition extends IBaseCommandLineDefinitionWithArgument {
 }
 
-
 ```

libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts

@@ -289,6 +289,45 @@ export abstract class CommandLineParameterProvider {
     return this._getArgumentParser().formatHelp();
   }
 
+  /**
+   * Returns a object which maps the long name of each parameter in this.parameters
+   * to the stringified form of its value. This is useful for logging telemetry, but
+   * it is not the proper way of accessing parameters or their values.
+   */
+  public getParameterStringMap(): Record<string, string> {
+    const parameterMap: Record<string, string> = {};
+    for (const parameter of this.parameters) {
+      switch (parameter.kind) {
+        case CommandLineParameterKind.Flag:
+        case CommandLineParameterKind.Choice:
+        case CommandLineParameterKind.String:
+        case CommandLineParameterKind.Integer:
+          parameterMap[parameter.longName] = JSON.stringify(
+            (
+              parameter as
+                | CommandLineFlagParameter
+                | CommandLineIntegerParameter
+                | CommandLineChoiceParameter
+                | CommandLineStringParameter
+            ).value
+          );
+          break;
+        case CommandLineParameterKind.StringList:
+        case CommandLineParameterKind.IntegerList:
+        case CommandLineParameterKind.ChoiceList:
+          const arrayValue: ReadonlyArray<string | number> | undefined = (
+            parameter as
+              | CommandLineIntegerListParameter
+              | CommandLineStringListParameter
+              | CommandLineChoiceListParameter
+          ).values;
+          parameterMap[parameter.longName] = arrayValue ? arrayValue.join(',') : '';
+          break;
+      }
+    }
+    return parameterMap;
+  }
+
   /**
    * The child class should implement this hook to define its command-line parameters,
    * e.g. by calling defineFlagParameter().