moved telemetry to update, added dryrun

elastic · Oct 25, 2021 · c77d7f6 · c77d7f6
1 parent 7c7a8d8
commit c77d7f6
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 35 deletions.
diff --git a/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
@@ -146,7 +146,8 @@ export const updatePackagePolicyHandler: RequestHandler<
       esClient,
       request.params.packagePolicyId,
       { ...newData, package: pkg, inputs },
-      { user }
+      { user },
+      packagePolicy.package?.version
     );
     return response.ok({
       body: { item: updatedPackagePolicy },

diff --git a/x-pack/plugins/fleet/server/services/package_policy.test.ts b/x-pack/plugins/fleet/server/services/package_policy.test.ts
@@ -134,6 +134,12 @@ jest.mock('./epm/packages/cleanup', () => {
   };
 });
 
+jest.mock('./upgrade_usage', () => {
+  return {
+    sendTelemetryEvents: jest.fn(),
+  };
+});
+
 const mockedFetchInfo = fetchInfo as jest.Mock<ReturnType<typeof fetchInfo>>;
 
 type CombinedExternalCallback = PutPackagePolicyUpdateCallback | PostPackagePolicyCreateCallback;
@@ -578,6 +584,12 @@ describe('Package policy service', () => {
   });
 
   describe('update', () => {
+    beforeEach(() => {
+      appContextService.start(createAppContextStartContractMock());
+    });
+    afterEach(() => {
+      appContextService.stop();
+    });
     it('should fail to update on version conflict', async () => {
       const savedObjectsClient = savedObjectsClientMock.create();
       savedObjectsClient.get.mockResolvedValue({
@@ -601,7 +613,8 @@ describe('Package policy service', () => {
           savedObjectsClient,
           elasticsearchClient,
           'the-package-policy-id',
-          createPackagePolicyMock()
+          createPackagePolicyMock(),
+          'current-version'
         )
       ).rejects.toThrow('Saved object [abc/123] conflict');
     });
@@ -721,7 +734,8 @@ describe('Package policy service', () => {
         savedObjectsClient,
         elasticsearchClient,
         'the-package-policy-id',
-        { ...mockPackagePolicy, inputs: inputsUpdate }
+        { ...mockPackagePolicy, inputs: inputsUpdate },
+        'current-version'
       );
 
       const [modifiedInput] = result.inputs;
@@ -844,7 +858,8 @@ describe('Package policy service', () => {
         savedObjectsClient,
         elasticsearchClient,
         'the-package-policy-id',
-        { ...mockPackagePolicy, inputs: inputsUpdate }
+        { ...mockPackagePolicy, inputs: inputsUpdate },
+        'current-version'
       );
 
       const [modifiedInput] = result.inputs;
@@ -903,7 +918,8 @@ describe('Package policy service', () => {
         savedObjectsClient,
         elasticsearchClient,
         'the-package-policy-id',
-        { ...mockPackagePolicy, inputs: [] }
+        { ...mockPackagePolicy, inputs: [] },
+        'current-version'
       );
 
       expect(result.elasticsearch).toMatchObject({ privileges: { cluster: ['monitor'] } });

diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts
@@ -359,7 +359,8 @@ class PackagePolicyService {
     esClient: ElasticsearchClient,
     id: string,
     packagePolicy: UpdatePackagePolicy,
-    options?: { user?: AuthenticatedUser }
+    options?: { user?: AuthenticatedUser },
+    currentVersion?: string
   ): Promise<PackagePolicy> {
     const oldPackagePolicy = await this.get(soClient, id);
     const { version, ...restOfPackagePolicy } = packagePolicy;
@@ -433,6 +434,20 @@ class PackagePolicyService {
         pkgName: packagePolicy.package.name,
         currentVersion: packagePolicy.package.version,
       });
+
+      if (packagePolicy.package.version !== currentVersion) {
+        sendTelemetryEvents(
+          appContextService.getLogger(),
+          appContextService.getTelemetryEventsSender(),
+          {
+            package_name: packagePolicy.package.name,
+            current_version: currentVersion || 'unknown',
+            new_version: packagePolicy.package.version,
+            status: 'success',
+            dryRun: false,
+          }
+        );
+      }
     }
 
     return newPolicy;
@@ -601,25 +616,19 @@ class PackagePolicyService {
         );
         updatePackagePolicy.elasticsearch = registryPkgInfo.elasticsearch;
 
-        await this.update(soClient, esClient, id, updatePackagePolicy, options);
+        await this.update(
+          soClient,
+          esClient,
+          id,
+          updatePackagePolicy,
+          options,
+          packagePolicy.package.version
+        );
         result.push({
           id,
           name: packagePolicy.name,
           success: true,
         });
-
-        if (packagePolicy.package.version !== packageInfo.version) {
-          sendTelemetryEvents(
-            appContextService.getLogger(),
-            appContextService.getTelemetryEventsSender(),
-            {
-              package_name: packageInfo.name,
-              current_version: packagePolicy.package.version,
-              new_version: packageInfo.version,
-              status: 'success',
-            }
-          );
-        }
       } catch (error) {
         // We only want to specifically handle validation errors for the new package policy. If a more severe or
         // general error is thrown elsewhere during the upgrade process, we want to surface that directly in
@@ -676,19 +685,18 @@ class PackagePolicyService {
       const hasErrors = 'errors' in updatedPackagePolicy;
 
       if (packagePolicy.package.version !== packageInfo.version) {
-        if (hasErrors) {
-          sendTelemetryEvents(
-            appContextService.getLogger(),
-            appContextService.getTelemetryEventsSender(),
-            {
-              package_name: packageInfo.name,
-              current_version: packagePolicy.package.version,
-              new_version: packageInfo.version,
-              status: hasErrors ? 'failure' : 'success',
-              error: updatedPackagePolicy.errors,
-            }
-          );
-        }
+        sendTelemetryEvents(
+          appContextService.getLogger(),
+          appContextService.getTelemetryEventsSender(),
+          {
+            package_name: packageInfo.name,
+            current_version: packagePolicy.package.version,
+            new_version: packageInfo.version,
+            status: hasErrors ? 'failure' : 'success',
+            error: hasErrors ? updatedPackagePolicy.errors : undefined,
+            dryRun: true,
+          }
+        );
       }
 
       return {

diff --git a/x-pack/plugins/fleet/server/services/upgrade_usage.test.ts b/x-pack/plugins/fleet/server/services/upgrade_usage.test.ts
@@ -33,14 +33,15 @@ describe('sendTelemetryEvents', () => {
         { key: 'fieldX', message: ['Field X is required'] },
         { key: 'fieldX', message: 'Invalid format' },
       ],
+      dryRun: true,
     };
 
     sendTelemetryEvents(loggerMock, eventsTelemetryMock, upgardeMessage);
 
     expect(eventsTelemetryMock.queueTelemetryEvents).toHaveBeenCalledWith(
       [
         {
-          id: 'aws_0.6.1_1.3.0_failure',
+          id: 'aws_0.6.1_1.3.0_failure_true',
           package_policy_upgrade: {
             current_version: '0.6.1',
             error: [
@@ -56,6 +57,7 @@ describe('sendTelemetryEvents', () => {
             new_version: '1.3.0',
             package_name: 'aws',
             status: 'failure',
+            dryRun: true,
           },
         },
       ],

diff --git a/x-pack/plugins/fleet/server/services/upgrade_usage.ts b/x-pack/plugins/fleet/server/services/upgrade_usage.ts
@@ -15,6 +15,7 @@ export interface PackagePolicyUpgradeUsage {
   new_version: string;
   status: 'success' | 'failure';
   error?: UpgradeError[];
+  dryRun?: boolean;
 }
 
 export interface UpgradeError {
@@ -44,7 +45,7 @@ export function sendTelemetryEvents(
               ? makeErrorGeneric(capErrorSize(upgradeUsage.error, MAX_ERROR_SIZE))
               : undefined,
           },
-          id: `${upgradeUsage.package_name}_${upgradeUsage.current_version}_${upgradeUsage.new_version}_${upgradeUsage.status}`,
+          id: `${upgradeUsage.package_name}_${upgradeUsage.current_version}_${upgradeUsage.new_version}_${upgradeUsage.status}_${upgradeUsage.dryRun}`,
         },
       ],
       FLEET_UPGRADES_CHANNEL_NAME