chore: reduce unused telemetry events

As we've hit telemetry thresholds we disabled several events that were output extremely often but we either weren't using or had questionable value vs the load. This included the following events: - shellInContainer - containerInspect - loadExtension - kubernetesReadNamespacedPod With continued volume on each of these we will never re-enable as-is, and we should not continue sending them in (using up some minor bandwidth) if they will never be used. Instead of removing them entirely, this PR renames each event (so that we can track/filter it separately from the current event) and only sends it when there is a failure. These should be _far_ less frequent and we would still get information on where we have failures. Since the same pattern as kubernetesReadNamespacedPod is used for every Kubernetes object I applied it to all of them. We already track pages to know when someone looks at each kind of object so we don't need to track the underlying API call, and this keeps the code consistent. Fixes podman-desktop#4970. Signed-off-by: Tim deBoer <[email protected]>
axel7083 · Jul 16, 2024 · 938bbc9 · 938bbc9
1 parent 869c1c4
commit 938bbc9
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 47 deletions.
diff --git a/packages/main/src/plugin/container-registry.ts b/packages/main/src/plugin/container-registry.ts
@@ -1697,7 +1697,6 @@ export class ContainerProviderRegistry {
     onError: (error: string) => void,
     onEnd: () => void,
   ): Promise<{ write: (param: string) => void; resize: (w: number, h: number) => void }> {
-    let telemetryOptions = {};
     try {
       const exec = await this.getMatchingContainer(engineId, id).exec({
         AttachStdin: true,
@@ -1741,10 +1740,8 @@ export class ContainerProviderRegistry {
         },
       };
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetryService.track('shellInContainer.error', error);
       throw error;
-    } finally {
-      this.telemetryService.track('shellInContainer', telemetryOptions);
     }
   }
 
@@ -2152,7 +2149,6 @@ export class ContainerProviderRegistry {
   }
 
   async getContainerInspect(engineId: string, id: string): Promise<ContainerInspectInfo> {
-    let telemetryOptions = {};
     try {
       // need to find the container engine of the container
       const provider = this.internalProviders.get(engineId);
@@ -2171,10 +2167,8 @@ export class ContainerProviderRegistry {
         ...containerInspect,
       };
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetryService.track('containerInspect.error', error);
       throw error;
-    } finally {
-      this.telemetryService.track('containerInspect', telemetryOptions);
     }
   }
 

diff --git a/packages/main/src/plugin/extension-loader.spec.ts b/packages/main/src/plugin/extension-loader.spec.ts
@@ -523,7 +523,7 @@ test('Verify extension load', async () => {
   });
 
   expect(telemetry.track).toBeCalledWith(
-    'loadExtension',
+    'loadExtension.error',
     expect.objectContaining({ extensionId: id, extensionVersion: '1.1' }),
   );
 });

diff --git a/packages/main/src/plugin/extension-loader.ts b/packages/main/src/plugin/extension-loader.ts
@@ -762,8 +762,7 @@ export class ExtensionLoader {
       this.extensionState.set(extension.id, 'failed');
       this.extensionStateErrors.set(extension.id, err);
       telemetryOptions['error'] = err;
-    } finally {
-      this.telemetry.track('loadExtension', telemetryOptions);
+      this.telemetry.track('loadExtension.error', telemetryOptions);
     }
   }
 

diff --git a/packages/main/src/plugin/kubernetes-client.ts b/packages/main/src/plugin/kubernetes-client.ts
@@ -830,7 +830,6 @@ export class KubernetesClient {
   }
 
   async readNamespacedPod(name: string, namespace: string): Promise<V1Pod | undefined> {
-    let telemetryOptions = {};
     const k8sApi = this.kubeConfig.makeApiClient(CoreV1Api);
     try {
       const res = await k8sApi.readNamespacedPod(name, namespace);
@@ -839,15 +838,12 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedPod.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedPod', telemetryOptions);
     }
   }
 
   async readNamespacedDeployment(name: string, namespace: string): Promise<V1Deployment | undefined> {
-    let telemetryOptions = {};
     const k8sAppsApi = this.kubeConfig.makeApiClient(AppsV1Api);
     try {
       const res = await k8sAppsApi.readNamespacedDeployment(name, namespace);
@@ -856,18 +852,15 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedDeployment.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedDeployment', telemetryOptions);
     }
   }
 
   async readNamespacedPersistentVolumeClaim(
     name: string,
     namespace: string,
   ): Promise<V1PersistentVolumeClaim | undefined> {
-    let telemetryOptions = {};
     const k8sApi = this.kubeConfig.makeApiClient(CoreV1Api);
     try {
       const res = await k8sApi.readNamespacedPersistentVolumeClaim(name, namespace);
@@ -876,15 +869,12 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedPersistentVolumeClaim.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedPersistentVolumeClaim', telemetryOptions);
     }
   }
 
   async readNode(name: string): Promise<V1Node | undefined> {
-    let telemetryOptions = {};
     const k8sApi = this.kubeConfig.makeApiClient(CoreV1Api);
     try {
       const res = await k8sApi.readNode(name);
@@ -893,15 +883,12 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNode.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNode', telemetryOptions);
     }
   }
 
   async readNamespacedIngress(name: string, namespace: string): Promise<V1Ingress | undefined> {
-    let telemetryOptions = {};
     const k8sNetworkingApi = this.kubeConfig.makeApiClient(NetworkingV1Api);
     try {
       const res = await k8sNetworkingApi.readNamespacedIngress(name, namespace);
@@ -910,15 +897,12 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedIngress.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedIngress', telemetryOptions);
     }
   }
 
   async readNamespacedRoute(name: string, namespace: string): Promise<V1Route | undefined> {
-    let telemetryOptions = {};
     const k8sCustomObjectsApi = this.kubeConfig.makeApiClient(CustomObjectsApi);
     try {
       const res = await k8sCustomObjectsApi.getNamespacedCustomObject(
@@ -934,15 +918,12 @@ export class KubernetesClient {
       }
       return route;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedRoute.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedRoute', telemetryOptions);
     }
   }
 
   async readNamespacedService(name: string, namespace: string): Promise<V1Service | undefined> {
-    let telemetryOptions = {};
     const k8sApi = this.kubeConfig.makeApiClient(CoreV1Api);
     try {
       const res = await k8sApi.readNamespacedService(name, namespace);
@@ -951,15 +932,12 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedService.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedService', telemetryOptions);
     }
   }
 
   async readNamespacedConfigMap(name: string, namespace: string): Promise<V1ConfigMap | undefined> {
-    let telemetryOptions = {};
     const k8sApi = this.kubeConfig.makeApiClient(CoreV1Api);
     try {
       const res = await k8sApi.readNamespacedConfigMap(name, namespace);
@@ -968,15 +946,12 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedConfigMap.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedConfigMap', telemetryOptions);
     }
   }
 
   async readNamespacedSecret(name: string, namespace: string): Promise<V1Secret | undefined> {
-    let telemetryOptions = {};
     const k8sApi = this.kubeConfig.makeApiClient(CoreV1Api);
     try {
       const res = await k8sApi.readNamespacedSecret(name, namespace);
@@ -985,10 +960,8 @@ export class KubernetesClient {
       }
       return res?.body;
     } catch (error) {
-      telemetryOptions = { error: error };
+      this.telemetry.track('kubernetesReadNamespacedSecret.error', error);
       throw this.wrapK8sClientError(error);
-    } finally {
-      this.telemetry.track('kubernetesReadNamespacedSecret', telemetryOptions);
     }
   }